Skip to content

Commit

Permalink
Return multiple object-safety violation errors
Browse files Browse the repository at this point in the history
  • Loading branch information
WaffleLapkin committed Oct 25, 2023
1 parent 8958235 commit ecdbefa
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 37 deletions.
77 changes: 42 additions & 35 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_violations_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_violations_for_assoc_item(tcx, trait_def_id, item))
.collect();

// Check the trait itself.
Expand Down Expand Up @@ -361,49 +360,52 @@ fn generics_require_sized_self(tcx: TyCtxt<'_>, def_id: DefId) -> bool {

/// Returns `Some(_)` if this item makes the containing trait not object safe.
#[instrument(level = "debug", skip(tcx), ret)]
fn object_safety_violation_for_assoc_item(
fn object_safety_violations_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_violations_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 @@ -413,11 +415,11 @@ fn object_safety_violation_for_assoc_item(
/// object; this does not necessarily imply that the enclosing trait
/// is not object safe, because the method might have a where clause
/// `Self:Sized`.
fn virtual_call_violation_for_method<'tcx>(
fn virtual_call_violations_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 @@ -598,10 +605,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

error: aborting due to previous error
Expand Down

0 comments on commit ecdbefa

Please sign in to comment.