Remove self: Self methods from vtables

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<Span>),
+    UndispatchableReceiver {
+        /// Span of the `self` argument, including the type.
+        self_span: Option<Span>,
+
+        /// Does the `self` parameter have type `Self`?
+        plain_self: bool,
+    },
 }
 
 /// These are the error cases for `codegen_select_candidate`.

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 <https://github.com/rust-lang/rust/issues/51443> 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<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()
+            // 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<Receiver[Self => 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<MethodViolationCode> {
+) -> Vec<MethodViolationCode> {
     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<A>(...)`.
     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<Receiver[Self => 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<Receiver[Self => U]>` to support receivers like
 // `self: Wrapper<Self>`.
-#[allow(dead_code)]
 fn receiver_is_dispatchable<'tcx>(
     tcx: TyCtxt<'tcx>,
     method: ty::AssocItem,

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 <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

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 <https://github.com/rust-lang/rust/issues/114007>.
+//
+// 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<Base>: NumOps<Base, Base> + for<'r> NumOps<&'r Base, Base> {}
+//~^ error: vtable entries for `<u32 as RefNum<u32>>`: [
+
+impl<T, Base> RefNum<Base> for T where T: NumOps<Base, Base> + for<'r> NumOps<&'r Base, Base> {}
+
+pub trait NumOps<Rhs = Self, Output = Self>:
+    Add<Rhs, Output = Output>
+    + Sub<Rhs, Output = Output>
+    + Mul<Rhs, Output = Output>
+    + Div<Rhs, Output = Output>
+    + Rem<Rhs, Output = Output>
+{
+}
+
+impl<T, Rhs, Output> NumOps<Rhs, Output> for T where
+    T: Add<Rhs, Output = Output>
+        + Sub<Rhs, Output = Output>
+        + Mul<Rhs, Output = Output>
+        + Div<Rhs, Output = Output>
+        + Rem<Rhs, Output = Output>
+{
+}
+
+pub fn require_vtables() {
+    fn require_vtables(_: &dyn RefNum<u32>, _: &dyn Simple) {}
+
+    require_vtables(&1u32, &())
+}

tests/ui/traits/vtable/plain-self.stderr

@@ -0,0 +1,22 @@
+error: vtable entries for `<u32 as RefNum<u32>>`: [
+           MetadataDropInPlace,
+           MetadataSize,
+           MetadataAlign,
+       ]
+  --> $DIR/plain-self.rs:23:1
+   |
+LL | pub trait RefNum<Base>: NumOps<Base, Base> + 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
+