diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index a2b33bb2737ab..0112f63dc4168 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -811,7 +811,7 @@ impl ObjectSafetyViolation { } ObjectSafetyViolation::Method( name, - MethodViolationCode::UndispatchableReceiver(_), + MethodViolationCode::UndispatchableReceiver { .. }, _, ) => format!("method `{}`'s `self` parameter cannot be dispatched on", name).into(), ObjectSafetyViolation::AssocConst(name, DUMMY_SP) => { @@ -856,7 +856,7 @@ impl ObjectSafetyViolation { } ObjectSafetyViolation::Method( name, - MethodViolationCode::UndispatchableReceiver(Some(span)), + MethodViolationCode::UndispatchableReceiver { self_span: Some(span), .. }, _, ) => { err.span_suggestion( @@ -918,7 +918,13 @@ pub enum MethodViolationCode { Generic, /// the method's receiver (`self` argument) can't be dispatched on - UndispatchableReceiver(Option), + UndispatchableReceiver { + /// Span of the `self` argument, including the type. + self_span: Option, + + /// Does the `self` parameter have type `Self`? + plain_self: bool, + }, } /// These are the error cases for `codegen_select_candidate`. diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index abb05be80e92c..5c85bedcbdc8a 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -97,6 +97,10 @@ fn check_is_object_safe(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool { /// object. Note that object-safe traits can have some /// non-vtable-safe methods, so long as they require `Self: Sized` or /// otherwise ensure that they cannot be used when `Self = Trait`. +/// +/// [`MethodViolationCode::WhereClauseReferencesSelf`] is considered object safe due to backwards +/// compatibility, see and +/// [`WHERE_CLAUSES_OBJECT_SAFETY`](rustc_session::lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY). pub fn is_vtable_safe_method(tcx: TyCtxt<'_>, trait_def_id: DefId, method: ty::AssocItem) -> bool { debug_assert!(tcx.generics_of(trait_def_id).has_self); debug!("is_vtable_safe_method({:?}, {:?})", trait_def_id, method); @@ -105,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( @@ -119,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. @@ -361,45 +364,63 @@ fn object_safety_violation_for_assoc_item( tcx: TyCtxt<'_>, trait_def_id: DefId, item: ty::AssocItem, -) -> Option { +) -> Vec { // 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() + // Until `unsized_locals` is fully implemented, `self: Self` can't be dispatched on. + // However, this is already considered object-safe. We allow it as a special case here. + // FIXME(mikeyhew) get rid of this `filter` once `receiver_is_dispatchable` allows + // `Receiver: Unsize dyn Trait]>`. + .filter(|violation| { + !matches!( + violation, + MethodViolationCode::UndispatchableReceiver { plain_self: true, .. } + ) + }) + .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 { + self_span: 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() } } } @@ -413,7 +434,7 @@ fn virtual_call_violation_for_method<'tcx>( tcx: TyCtxt<'tcx>, trait_def_id: DefId, method: ty::AssocItem, -) -> Option { +) -> Vec { let sig = tcx.fn_sig(method.def_id).instantiate_identity(); // The method's first parameter must be named `self` @@ -438,9 +459,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 { @@ -452,31 +478,31 @@ 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(...)`. let own_counts = tcx.generics_of(method.def_id).own_counts(); - if own_counts.types + own_counts.consts != 0 { - return Some(MethodViolationCode::Generic); + if own_counts.types > 0 || own_counts.consts > 0 { + errors.push(MethodViolationCode::Generic); } let receiver_ty = tcx.liberate_late_bound_regions(method.def_id, sig.input(0)); - // Until `unsized_locals` is fully implemented, `self: Self` can't be dispatched on. - // However, this is already considered object-safe. We allow it as a special case here. - // FIXME(mikeyhew) get rid of this `if` statement once `receiver_is_dispatchable` allows - // `Receiver: Unsize dyn Trait]>`. - if receiver_ty != tcx.types.self_param { + // Don't error on `self` methods if the trait is `FnOnce` or unsized locals are on + if receiver_ty != tcx.types.self_param + || (Some(trait_def_id) != tcx.lang_items().fn_once_trait() + && !tcx.features().unsized_locals) + { if !receiver_is_dispatchable(tcx, method, receiver_ty) { - let span = if let Some(hir::Node::TraitItem(hir::TraitItem { + let self_span = if let Some(hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(sig, _), .. })) = tcx.hir().get_if_local(method.def_id).as_ref() @@ -485,7 +511,10 @@ fn virtual_call_violation_for_method<'tcx>( } else { None }; - return Some(MethodViolationCode::UndispatchableReceiver(span)); + errors.push(MethodViolationCode::UndispatchableReceiver { + self_span, + plain_self: receiver_ty == tcx.types.self_param, + }); } else { // Do sanity check to make sure the receiver actually has the layout of a pointer. @@ -597,10 +626,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`. @@ -713,7 +742,6 @@ fn object_ty_for_trait<'tcx>( // FIXME(mikeyhew) when unsized receivers are implemented as part of unsized rvalues, add this // fallback query: `Receiver: Unsize U]>` to support receivers like // `self: Wrapper`. -#[allow(dead_code)] fn receiver_is_dispatchable<'tcx>( tcx: TyCtxt<'tcx>, method: ty::AssocItem, diff --git a/tests/ui/const-generics/generic_const_exprs/object-safety-err-ret.stderr b/tests/ui/const-generics/generic_const_exprs/object-safety-err-ret.stderr index 4e1d71f154558..7ce2b9ac95a59 100644 --- a/tests/ui/const-generics/generic_const_exprs/object-safety-err-ret.stderr +++ b/tests/ui/const-generics/generic_const_exprs/object-safety-err-ret.stderr @@ -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 - --> $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::()]; - | ^^^^^^^^^^^^^^^^^^^ ...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 diff --git a/tests/ui/traits/vtable/plain-self.rs b/tests/ui/traits/vtable/plain-self.rs new file mode 100644 index 0000000000000..3cfffeaea4da8 --- /dev/null +++ b/tests/ui/traits/vtable/plain-self.rs @@ -0,0 +1,50 @@ +// Make sure we don't include vtable entries for methods that take self by-value, +// see . +// +// build-fail +#![crate_type = "lib"] +#![feature(rustc_attrs)] + +use std::ops::*; + +#[rustc_dump_vtable] +pub trait Simple { + //~^ error: vtable entries for `<() as Simple>`: [ + fn f(self); + fn g(self: Self); +} + +impl Simple for () { + fn f(self) {} + fn g(self: Self) {} +} + +#[rustc_dump_vtable] +pub trait RefNum: NumOps + for<'r> NumOps<&'r Base, Base> {} +//~^ error: vtable entries for `>`: [ + +impl RefNum for T where T: NumOps + for<'r> NumOps<&'r Base, Base> {} + +pub trait NumOps: + Add + + Sub + + Mul + + Div + + Rem +{ +} + +impl NumOps for T where + T: Add + + Sub + + Mul + + Div + + Rem +{ +} + +pub fn require_vtables() { + fn require_vtables(_: &dyn RefNum, _: &dyn Simple) {} + + require_vtables(&1u32, &()) +} diff --git a/tests/ui/traits/vtable/plain-self.stderr b/tests/ui/traits/vtable/plain-self.stderr new file mode 100644 index 0000000000000..c3ce1752e78a8 --- /dev/null +++ b/tests/ui/traits/vtable/plain-self.stderr @@ -0,0 +1,22 @@ +error: vtable entries for `>`: [ + MetadataDropInPlace, + MetadataSize, + MetadataAlign, + ] + --> $DIR/plain-self.rs:23:1 + | +LL | pub trait RefNum: NumOps + for<'r> NumOps<&'r Base, Base> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: vtable entries for `<() as Simple>`: [ + MetadataDropInPlace, + MetadataSize, + MetadataAlign, + ] + --> $DIR/plain-self.rs:11:1 + | +LL | pub trait Simple { + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors +