Rollup merge of #64110 - estebank:receiver-type, r=Centril

Refer to "`self` type" instead of "receiver type"

Fix #42603.

Author: Centril

src/librustc/error_codes.rs

@@ -39,7 +39,7 @@ Generally, `Self: Sized` is used to indicate that the trait should not be used
 as a trait object. If the trait comes from your own crate, consider removing
 this restriction.
 
-### Method references the `Self` type in its arguments or return type
+### Method references the `Self` type in its parameters or return type
 
 This happens when a trait has a method like the following:
 

src/librustc/infer/error_reporting/mod.rs

@@ -1627,7 +1627,7 @@ impl<'tcx> ObligationCause<'tcx> {
             MainFunctionType => Error0580("main function has wrong type"),
             StartFunctionType => Error0308("start function has wrong type"),
             IntrinsicType => Error0308("intrinsic has wrong type"),
-            MethodReceiver => Error0308("mismatched method receiver"),
+            MethodReceiver => Error0308("mismatched `self` parameter type"),
 
             // In the case where we have no more specific thing to
             // say, also take a look at the error code, maybe we can

src/librustc/traits/error_reporting.rs

@@ -1384,7 +1384,10 @@ impl<'tcx> TyCtxt<'tcx> {
         let mut reported_violations = FxHashSet::default();
         for violation in violations {
             if reported_violations.insert(violation.clone()) {
-                err.note(&violation.error_msg());
+                match violation.span() {
+                    Some(span) => err.span_label(span, violation.error_msg()),
+                    None => err.note(&violation.error_msg()),
+                };
             }
         }
         Some(err)

src/librustc/traits/object_safety.rs

@@ -20,7 +20,7 @@ use std::borrow::Cow;
 use std::iter::{self};
 use syntax::ast::{self};
 use syntax::symbol::InternedString;
-use syntax_pos::Span;
+use syntax_pos::{Span, DUMMY_SP};
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
 pub enum ObjectSafetyViolation {
@@ -32,10 +32,10 @@ pub enum ObjectSafetyViolation {
     SupertraitSelf,
 
     /// Method has something illegal.
-    Method(ast::Name, MethodViolationCode),
+    Method(ast::Name, MethodViolationCode, Span),
 
     /// Associated const.
-    AssocConst(ast::Name),
+    AssocConst(ast::Name, Span),
 }
 
 impl ObjectSafetyViolation {
@@ -46,22 +46,35 @@ impl ObjectSafetyViolation {
             ObjectSafetyViolation::SupertraitSelf =>
                 "the trait cannot use `Self` as a type parameter \
                  in the supertraits or where-clauses".into(),
-            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod) =>
-                format!("method `{}` has no receiver", name).into(),
-            ObjectSafetyViolation::Method(name, MethodViolationCode::ReferencesSelf) =>
-                format!("method `{}` references the `Self` type \
-                         in its arguments or return type", name).into(),
-            ObjectSafetyViolation::Method(name,
-                                            MethodViolationCode::WhereClauseReferencesSelf(_)) =>
-                format!("method `{}` references the `Self` type in where clauses", name).into(),
-            ObjectSafetyViolation::Method(name, MethodViolationCode::Generic) =>
+            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod, _) =>
+                format!("associated function `{}` has no `self` parameter", name).into(),
+            ObjectSafetyViolation::Method(name, MethodViolationCode::ReferencesSelf, _) => format!(
+                "method `{}` references the `Self` type in its parameters or return type",
+                name,
+            ).into(),
+            ObjectSafetyViolation::Method(
+                name,
+                MethodViolationCode::WhereClauseReferencesSelf,
+                _,
+            ) => format!("method `{}` references the `Self` type in where clauses", name).into(),
+            ObjectSafetyViolation::Method(name, MethodViolationCode::Generic, _) =>
                 format!("method `{}` has generic type parameters", name).into(),
-            ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver) =>
-                format!("method `{}`'s receiver cannot be dispatched on", name).into(),
-            ObjectSafetyViolation::AssocConst(name) =>
+            ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver, _) =>
+                format!("method `{}`'s `self` parameter cannot be dispatched on", name).into(),
+            ObjectSafetyViolation::AssocConst(name, _) =>
                 format!("the trait cannot contain associated consts like `{}`", name).into(),
         }
     }
+
+    pub fn span(&self) -> Option<Span> {
+        // When `span` comes from a separate crate, it'll be `DUMMY_SP`. Treat it as `None` so
+        // diagnostics use a `note` instead of a `span_label`.
+        match *self {
+            ObjectSafetyViolation::AssocConst(_, span) |
+            ObjectSafetyViolation::Method(_, _, span) if span != DUMMY_SP => Some(span),
+            _ => None,
+        }
+    }
 }
 
 /// Reasons a method might not be object-safe.
@@ -74,7 +87,7 @@ pub enum MethodViolationCode {
     ReferencesSelf,
 
     /// e.g., `fn foo(&self) where Self: Clone`
-    WhereClauseReferencesSelf(Span),
+    WhereClauseReferencesSelf,
 
     /// e.g., `fn foo<A>()`
     Generic,
@@ -88,9 +101,10 @@ impl<'tcx> TyCtxt<'tcx> {
     /// astconv -- currently, `Self` in supertraits. This is needed
     /// because `object_safety_violations` can't be used during
     /// type collection.
-    pub fn astconv_object_safety_violations(self, trait_def_id: DefId)
-                                            -> Vec<ObjectSafetyViolation>
-    {
+    pub fn astconv_object_safety_violations(
+        self,
+        trait_def_id: DefId,
+    ) -> Vec<ObjectSafetyViolation> {
         debug_assert!(self.generics_of(trait_def_id).has_self);
         let violations = traits::supertrait_def_ids(self, trait_def_id)
             .filter(|&def_id| self.predicates_reference_self(def_id, true))
@@ -128,7 +142,7 @@ impl<'tcx> TyCtxt<'tcx> {
         }
 
         match self.virtual_call_violation_for_method(trait_def_id, method) {
-            None | Some(MethodViolationCode::WhereClauseReferencesSelf(_)) => true,
+            None | Some(MethodViolationCode::WhereClauseReferencesSelf) => true,
             Some(_) => false,
         }
     }
@@ -138,12 +152,15 @@ impl<'tcx> TyCtxt<'tcx> {
         let mut violations: Vec<_> = self.associated_items(trait_def_id)
             .filter(|item| item.kind == ty::AssocKind::Method)
             .filter_map(|item|
-                self.object_safety_violation_for_method(trait_def_id, &item)
-                    .map(|code| ObjectSafetyViolation::Method(item.ident.name, code))
+                self.object_safety_violation_for_method(trait_def_id, &item).map(|code| {
+                    ObjectSafetyViolation::Method(item.ident.name, code, item.ident.span)
+                })
             ).filter(|violation| {
-                if let ObjectSafetyViolation::Method(_,
-                    MethodViolationCode::WhereClauseReferencesSelf(span)) = violation
-                {
+                if let ObjectSafetyViolation::Method(
+                    _,
+                    MethodViolationCode::WhereClauseReferencesSelf,
+                    span,
+                ) = violation {
                     // Using `CRATE_NODE_ID` is wrong, but it's hard to get a more precise id.
                     // It's also hard to get a use site span, so we use the method definition span.
                     self.lint_node_note(
@@ -169,7 +186,7 @@ impl<'tcx> TyCtxt<'tcx> {
 
         violations.extend(self.associated_items(trait_def_id)
             .filter(|item| item.kind == ty::AssocKind::Const)
-            .map(|item| ObjectSafetyViolation::AssocConst(item.ident.name)));
+            .map(|item| ObjectSafetyViolation::AssocConst(item.ident.name, item.ident.span)));
 
         debug!("object_safety_violations_for_trait(trait_def_id={:?}) = {:?}",
                trait_def_id,
@@ -325,8 +342,7 @@ impl<'tcx> TyCtxt<'tcx> {
                 .visit_tys_shallow(|t| {
                     self.contains_illegal_self_type_reference(trait_def_id, t)
                 }) {
-            let span = self.def_span(method.def_id);
-            return Some(MethodViolationCode::WhereClauseReferencesSelf(span));
+            return Some(MethodViolationCode::WhereClauseReferencesSelf);
         }
 
         let receiver_ty = self.liberate_late_bound_regions(

src/librustc_typeck/check/wfcheck.rs

@@ -762,19 +762,19 @@ fn check_opaque_types<'fcx, 'tcx>(
     substituted_predicates
 }
 
+const HELP_FOR_SELF_TYPE: &str =
+    "consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, \
+     `self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one \
+     of the previous types except `Self`)";
+
 fn check_method_receiver<'fcx, 'tcx>(
     fcx: &FnCtxt<'fcx, 'tcx>,
     method_sig: &hir::MethodSig,
     method: &ty::AssocItem,
     self_ty: Ty<'tcx>,
 ) {
-    const HELP_FOR_SELF_TYPE: &str =
-        "consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, \
-         `self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one \
-         of the previous types except `Self`)";
     // Check that the method has a valid receiver type, given the type `Self`.
-    debug!("check_method_receiver({:?}, self_ty={:?})",
-           method, self_ty);
+    debug!("check_method_receiver({:?}, self_ty={:?})", method, self_ty);
 
     if !method.method_has_self_argument {
         return;
@@ -805,12 +805,7 @@ fn check_method_receiver<'fcx, 'tcx>(
     if fcx.tcx.features().arbitrary_self_types {
         if !receiver_is_valid(fcx, span, receiver_ty, self_ty, true) {
             // Report error; `arbitrary_self_types` was enabled.
-            fcx.tcx.sess.diagnostic().mut_span_err(
-                span, &format!("invalid method receiver type: {:?}", receiver_ty)
-            ).note("type of `self` must be `Self` or a type that dereferences to it")
-            .help(HELP_FOR_SELF_TYPE)
-            .code(DiagnosticId::Error("E0307".into()))
-            .emit();
+            e0307(fcx, span, receiver_ty);
         }
     } else {
         if !receiver_is_valid(fcx, span, receiver_ty, self_ty, false) {
@@ -830,17 +825,22 @@ fn check_method_receiver<'fcx, 'tcx>(
                 .emit();
             } else {
                 // Report error; would not have worked with `arbitrary_self_types`.
-                fcx.tcx.sess.diagnostic().mut_span_err(
-                    span, &format!("invalid method receiver type: {:?}", receiver_ty)
-                ).note("type must be `Self` or a type that dereferences to it")
-                .help(HELP_FOR_SELF_TYPE)
-                .code(DiagnosticId::Error("E0307".into()))
-                .emit();
+                e0307(fcx, span, receiver_ty);
             }
         }
     }
 }
 
+fn e0307(fcx: &FnCtxt<'fcx, 'tcx>, span: Span, receiver_ty: Ty<'_>) {
+    fcx.tcx.sess.diagnostic().mut_span_err(
+        span,
+        &format!("invalid `self` parameter type: {:?}", receiver_ty)
+    ).note("type of `self` must be `Self` or a type that dereferences to it")
+    .help(HELP_FOR_SELF_TYPE)
+    .code(DiagnosticId::Error("E0307".into()))
+    .emit();
+}
+
 /// Returns whether `receiver_ty` would be considered a valid receiver type for `self_ty`. If
 /// `arbitrary_self_types` is enabled, `receiver_ty` must transitively deref to `self_ty`, possibly
 /// through a `*const/mut T` raw pointer. If the feature is not enabled, the requirements are more

src/librustc_typeck/error_codes.rs

@@ -212,7 +212,7 @@ match string {
 E0033: r##"
 This error indicates that a pointer to a trait type cannot be implicitly
 dereferenced by a pattern. Every trait defines a type, but because the
-size of trait implementors isn't fixed, this type has no compile-time size.
+size of trait implementers isn't fixed, this type has no compile-time size.
 Therefore, all accesses to trait types must be through pointers. If you
 encounter this error you should try to avoid dereferencing the pointer.
 
@@ -2425,6 +2425,87 @@ struct Bar<S, T> { x: Foo<S, T> }
 ```
 "##,
 
+E0307: r##"
+This error indicates that the `self` parameter in a method has an invalid
+"reciever type".
+
+Methods take a special first parameter, of which there are three variants:
+`self`, `&self`, and `&mut self`. These are syntactic sugar for
+`self: Self`, `self: &Self`, and `self: &mut Self` respectively.
+
+```
+# struct Foo;
+trait Trait {
+    fn foo(&self);
+//         ^^^^^ `self` here is a reference to the receiver object
+}
+
+impl Trait for Foo {
+    fn foo(&self) {}
+//         ^^^^^ the receiver type is `&Foo`
+}
+```
+
+The type `Self` acts as an alias to the type of the current trait
+implementer, or "receiver type". Besides the already mentioned `Self`,
+`&Self` and `&mut Self` valid receiver types, the following are also valid:
+`self: Box<Self>`, `self: Rc<Self>`, `self: Arc<Self>`, and `self: Pin<P>`
+(where P is one of the previous types except `Self`). Note that `Self` can
+also be the underlying implementing type, like `Foo` in the following
+example:
+
+```
+# struct Foo;
+# trait Trait {
+#     fn foo(&self);
+# }
+impl Trait for Foo {
+    fn foo(self: &Foo) {}
+}
+```
+
+E0307 will be emitted by the compiler when using an invalid reciver type,
+like in the following example:
+
+```compile_fail,E0307
+# struct Foo;
+# struct Bar;
+# trait Trait {
+#     fn foo(&self);
+# }
+impl Trait for Foo {
+    fn foo(self: &Bar) {}
+}
+```
+
+The nightly feature [Arbintrary self types][AST] extends the accepted
+set of receiver types to also include any type that can dereference to
+`Self`:
+
+```
+#![feature(arbitrary_self_types)]
+
+struct Foo;
+struct Bar;
+
+// Because you can dereference `Bar` into `Foo`...
+impl std::ops::Deref for Bar {
+    type Target = Foo;
+
+    fn deref(&self) -> &Foo {
+        &Foo
+    }
+}
+
+impl Foo {
+    fn foo(self: Bar) {}
+//         ^^^^^^^^^ ...it can be used as the receiver type
+}
+```
+
+[AST]: https://doc.rust-lang.org/unstable-book/language-features/arbitrary-self-types.html
+"##,
+
 E0321: r##"
 A cross-crate opt-out trait was implemented on something which wasn't a struct
 or enum type. Erroneous code example:
@@ -4851,7 +4932,6 @@ register_diagnostics! {
 //  E0247,
 //  E0248, // value used as a type, now reported earlier during resolution as E0412
 //  E0249,
-    E0307, // invalid method `self` type
 //  E0319, // trait impls for defaulted traits allowed just for structs/enums
 //  E0372, // coherence not object safe
     E0377, // the trait `CoerceUnsized` may only be implemented for a coercion

src/test/ui/associated-const/associated-const-in-trait.stderr

@@ -1,10 +1,11 @@
 error[E0038]: the trait `Trait` cannot be made into an object
   --> $DIR/associated-const-in-trait.rs:9:6
    |
+LL |     const N: usize;
+   |           - the trait cannot contain associated consts like `N`
+...
 LL | impl dyn Trait {
    |      ^^^^^^^^^ the trait `Trait` cannot be made into an object
-   |
-   = note: the trait cannot contain associated consts like `N`
 
 error: aborting due to previous error
 

src/test/ui/coherence/coherence-impl-trait-for-trait-object-safe.old.stderr

@@ -1,10 +1,10 @@
 error[E0038]: the trait `NotObjectSafe` cannot be made into an object
   --> $DIR/coherence-impl-trait-for-trait-object-safe.rs:11:6
    |
+LL | trait NotObjectSafe { fn eq(&self, other: Self); }
+   |                          -- method `eq` references the `Self` type in its parameters or return type
 LL | impl NotObjectSafe for dyn NotObjectSafe { }
    |      ^^^^^^^^^^^^^ the trait `NotObjectSafe` cannot be made into an object
-   |
-   = note: method `eq` references the `Self` type in its arguments or return type
 
 error: aborting due to previous error
 

src/test/ui/coherence/coherence-impl-trait-for-trait-object-safe.re.stderr

@@ -1,10 +1,10 @@
 error[E0038]: the trait `NotObjectSafe` cannot be made into an object
   --> $DIR/coherence-impl-trait-for-trait-object-safe.rs:11:6
    |
+LL | trait NotObjectSafe { fn eq(&self, other: Self); }
+   |                          -- method `eq` references the `Self` type in its parameters or return type
 LL | impl NotObjectSafe for dyn NotObjectSafe { }
    |      ^^^^^^^^^^^^^ the trait `NotObjectSafe` cannot be made into an object
-   |
-   = note: method `eq` references the `Self` type in its arguments or return type
 
 error: aborting due to previous error
 

src/test/ui/did_you_mean/issue-40006.stderr

@@ -61,8 +61,9 @@ error[E0038]: the trait `X` cannot be made into an object
    |
 LL | impl dyn X {
    |      ^^^^^ the trait `X` cannot be made into an object
-   |
-   = note: method `xxx` has no receiver
+...
+LL |     fn xxx() { ### }
+   |        --- associated function `xxx` has no `self` parameter
 
 error: aborting due to 9 previous errors