Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 40 additions & 33 deletions compiler/rustc_trait_selection/src/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,9 @@ pub fn is_vtable_safe_method(tcx: TyCtxt<'_>, trait_def_id: DefId, method: ty::A
return false;
}

match virtual_call_violation_for_method(tcx, trait_def_id, method) {
None | Some(MethodViolationCode::WhereClauseReferencesSelf) => true,
Some(_) => false,
}
virtual_call_violation_for_method(tcx, trait_def_id, method)
.iter()
.all(|v| matches!(v, MethodViolationCode::WhereClauseReferencesSelf))
}

fn object_safety_violations_for_trait(
Expand All @@ -123,7 +122,7 @@ fn object_safety_violations_for_trait(
let mut violations: Vec<_> = tcx
.associated_items(trait_def_id)
.in_definition_order()
.filter_map(|&item| object_safety_violation_for_assoc_item(tcx, trait_def_id, item))
.flat_map(|&item| object_safety_violation_for_assoc_item(tcx, trait_def_id, item))
.collect();

// Check the trait itself.
Expand Down Expand Up @@ -365,45 +364,48 @@ fn object_safety_violation_for_assoc_item(
tcx: TyCtxt<'_>,
trait_def_id: DefId,
item: ty::AssocItem,
) -> Option<ObjectSafetyViolation> {
) -> Vec<ObjectSafetyViolation> {
// Any item that has a `Self : Sized` requisite is otherwise
// exempt from the regulations.
if tcx.generics_require_sized_self(item.def_id) {
return None;
return Vec::new();
}

match item.kind {
// Associated consts are never object safe, as they can't have `where` bounds yet at all,
// and associated const bounds in trait objects aren't a thing yet either.
ty::AssocKind::Const => {
Some(ObjectSafetyViolation::AssocConst(item.name, item.ident(tcx).span))
vec![ObjectSafetyViolation::AssocConst(item.name, item.ident(tcx).span)]
}
ty::AssocKind::Fn => virtual_call_violation_for_method(tcx, trait_def_id, item).map(|v| {
let node = tcx.hir().get_if_local(item.def_id);
// Get an accurate span depending on the violation.
let span = match (&v, node) {
(MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
(MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
(MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span,
(MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
node.fn_decl().map_or(item.ident(tcx).span, |decl| decl.output.span())
}
_ => item.ident(tcx).span,
};
ty::AssocKind::Fn => virtual_call_violation_for_method(tcx, trait_def_id, item)
.into_iter()
.map(|v| {
let node = tcx.hir().get_if_local(item.def_id);
// Get an accurate span depending on the violation.
let span = match (&v, node) {
(MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
(MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
(MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span,
(MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
node.fn_decl().map_or(item.ident(tcx).span, |decl| decl.output.span())
}
_ => item.ident(tcx).span,
};

ObjectSafetyViolation::Method(item.name, v, span)
}),
ObjectSafetyViolation::Method(item.name, v, span)
})
.collect(),
// Associated types can only be object safe if they have `Self: Sized` bounds.
ty::AssocKind::Type => {
if !tcx.features().generic_associated_types_extended
&& !tcx.generics_of(item.def_id).params.is_empty()
&& !item.is_impl_trait_in_trait()
{
Some(ObjectSafetyViolation::GAT(item.name, item.ident(tcx).span))
vec![ObjectSafetyViolation::GAT(item.name, item.ident(tcx).span)]
} else {
// We will permit associated types if they are explicitly mentioned in the trait object.
// We can't check this here, as here we only check if it is guaranteed to not be possible.
None
Vec::new()
}
}
}
Expand All @@ -417,7 +419,7 @@ fn virtual_call_violation_for_method<'tcx>(
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
method: ty::AssocItem,
) -> Option<MethodViolationCode> {
) -> Vec<MethodViolationCode> {
let sig = tcx.fn_sig(method.def_id).instantiate_identity();

// The method's first parameter must be named `self`
Expand All @@ -442,9 +444,14 @@ fn virtual_call_violation_for_method<'tcx>(
} else {
None
};
return Some(MethodViolationCode::StaticMethod(sugg));

// Not having `self` parameter messes up the later checks,
// so we need to return instead of pushing
return vec![MethodViolationCode::StaticMethod(sugg)];
}

let mut errors = Vec::new();

for (i, &input_ty) in sig.skip_binder().inputs().iter().enumerate().skip(1) {
if contains_illegal_self_type_reference(tcx, trait_def_id, sig.rebind(input_ty)) {
let span = if let Some(hir::Node::TraitItem(hir::TraitItem {
Expand All @@ -456,20 +463,20 @@ fn virtual_call_violation_for_method<'tcx>(
} else {
None
};
return Some(MethodViolationCode::ReferencesSelfInput(span));
errors.push(MethodViolationCode::ReferencesSelfInput(span));
}
}
if contains_illegal_self_type_reference(tcx, trait_def_id, sig.output()) {
return Some(MethodViolationCode::ReferencesSelfOutput);
errors.push(MethodViolationCode::ReferencesSelfOutput);
}
if let Some(code) = contains_illegal_impl_trait_in_trait(tcx, method.def_id, sig.output()) {
return Some(code);
errors.push(code);
}

// We can't monomorphize things like `fn foo<A>(...)`.
let own_counts = tcx.generics_of(method.def_id).own_counts();
if own_counts.types > 0 || own_counts.consts > 0 {
return Some(MethodViolationCode::Generic);
errors.push(MethodViolationCode::Generic);
}

let receiver_ty = tcx.liberate_late_bound_regions(method.def_id, sig.input(0));
Expand All @@ -489,7 +496,7 @@ fn virtual_call_violation_for_method<'tcx>(
} else {
None
};
return Some(MethodViolationCode::UndispatchableReceiver(span));
errors.push(MethodViolationCode::UndispatchableReceiver(span));
} else {
// Do sanity check to make sure the receiver actually has the layout of a pointer.

Expand Down Expand Up @@ -601,10 +608,10 @@ fn virtual_call_violation_for_method<'tcx>(

contains_illegal_self_type_reference(tcx, trait_def_id, pred)
}) {
return Some(MethodViolationCode::WhereClauseReferencesSelf);
errors.push(MethodViolationCode::WhereClauseReferencesSelf);
}

None
errors
}

/// Performs a type substitution to produce the version of `receiver_ty` when `Self = self_ty`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ LL | fn use_dyn(v: &dyn Foo) {
| ^^^^^^^ `Foo` cannot be made into an object
|
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/object-safety-err-ret.rs:8:23
--> $DIR/object-safety-err-ret.rs:8:8
|
LL | trait Foo {
| --- this trait cannot be made into an object...
LL | fn test(&self) -> [u8; bar::<Self>()];
| ^^^^^^^^^^^^^^^^^^^ ...because method `test` references the `Self` type in its return type
| ^^^^ ^^^^^^^^^^^^^^^^^^^ ...because method `test` references the `Self` type in its return type
| |
| ...because method `test` references the `Self` type in its `where` clause
= help: consider moving `test` to another trait
= help: consider moving `test` to another trait
Comment on lines +16 to 17
Copy link
Copy Markdown
Member

@Noratrieb Noratrieb Jul 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think users need that much help when getting the error

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You see, this is tricky, because those lines are triggered by different errors:

ObjectSafetyViolation::AssocConst(name, _)
| ObjectSafetyViolation::GAT(name, _)
| ObjectSafetyViolation::Method(name, ..) => {
err.help(format!("consider moving `{name}` to another trait"));

I couldn't find an easy way to fix this...


error: aborting due to previous error
Expand Down