Rollup merge of #98506 - compiler-errors:object-safety-suggestions, r=oli-obk

Fix span issues in object safety suggestions

Fixes #98500

Author: matthiaskrgr

compiler/rustc_middle/src/traits/mod.rs

@@ -863,7 +863,7 @@ pub enum ObjectSafetyViolation {
 
 impl ObjectSafetyViolation {
     pub fn error_msg(&self) -> Cow<'static, str> {
-        match *self {
+        match self {
             ObjectSafetyViolation::SizedSelf(_) => "it requires `Self: Sized`".into(),
             ObjectSafetyViolation::SupertraitSelf(ref spans) => {
                 if spans.iter().any(|sp| *sp != DUMMY_SP) {
@@ -873,7 +873,7 @@ impl ObjectSafetyViolation {
                         .into()
                 }
             }
-            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod(_, _, _), _) => {
+            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod(_), _) => {
                 format!("associated function `{}` has no `self` parameter", name).into()
             }
             ObjectSafetyViolation::Method(
@@ -897,9 +897,11 @@ impl ObjectSafetyViolation {
             ObjectSafetyViolation::Method(name, MethodViolationCode::Generic, _) => {
                 format!("method `{}` has generic type parameters", name).into()
             }
-            ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver, _) => {
-                format!("method `{}`'s `self` parameter cannot be dispatched on", name).into()
-            }
+            ObjectSafetyViolation::Method(
+                name,
+                MethodViolationCode::UndispatchableReceiver(_),
+                _,
+            ) => format!("method `{}`'s `self` parameter cannot be dispatched on", name).into(),
             ObjectSafetyViolation::AssocConst(name, DUMMY_SP) => {
                 format!("it contains associated `const` `{}`", name).into()
             }
@@ -911,51 +913,40 @@ impl ObjectSafetyViolation {
     }
 
     pub fn solution(&self, err: &mut Diagnostic) {
-        match *self {
+        match self {
             ObjectSafetyViolation::SizedSelf(_) | ObjectSafetyViolation::SupertraitSelf(_) => {}
             ObjectSafetyViolation::Method(
                 name,
-                MethodViolationCode::StaticMethod(sugg, self_span, has_args),
+                MethodViolationCode::StaticMethod(Some((add_self_sugg, make_sized_sugg))),
                 _,
             ) => {
                 err.span_suggestion(
-                    self_span,
-                    &format!(
+                    add_self_sugg.1,
+                    format!(
                         "consider turning `{}` into a method by giving it a `&self` argument",
                         name
                     ),
-                    format!("&self{}", if has_args { ", " } else { "" }),
+                    add_self_sugg.0.to_string(),
+                    Applicability::MaybeIncorrect,
+                );
+                err.span_suggestion(
+                    make_sized_sugg.1,
+                    format!(
+                        "alternatively, consider constraining `{}` so it does not apply to \
+                             trait objects",
+                        name
+                    ),
+                    make_sized_sugg.0.to_string(),
                     Applicability::MaybeIncorrect,
                 );
-                match sugg {
-                    Some((sugg, span)) => {
-                        err.span_suggestion(
-                            span,
-                            &format!(
-                                "alternatively, consider constraining `{}` so it does not apply to \
-                                 trait objects",
-                                name
-                            ),
-                            sugg,
-                            Applicability::MaybeIncorrect,
-                        );
-                    }
-                    None => {
-                        err.help(&format!(
-                            "consider turning `{}` into a method by giving it a `&self` \
-                             argument or constraining it so it does not apply to trait objects",
-                            name
-                        ));
-                    }
-                }
             }
             ObjectSafetyViolation::Method(
                 name,
-                MethodViolationCode::UndispatchableReceiver,
-                span,
+                MethodViolationCode::UndispatchableReceiver(Some(span)),
+                _,
             ) => {
                 err.span_suggestion(
-                    span,
+                    *span,
                     &format!(
                         "consider changing method `{}`'s `self` parameter to be `&self`",
                         name
@@ -991,13 +982,13 @@ impl ObjectSafetyViolation {
 }
 
 /// Reasons a method might not be object-safe.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)]
 pub enum MethodViolationCode {
     /// e.g., `fn foo()`
-    StaticMethod(Option<(&'static str, Span)>, Span, bool /* has args */),
+    StaticMethod(Option<(/* add &self */ (String, Span), /* add Self: Sized */ (String, Span))>),
 
     /// e.g., `fn foo(&self, x: Self)`
-    ReferencesSelfInput(usize),
+    ReferencesSelfInput(Option<Span>),
 
     /// e.g., `fn foo(&self) -> Self`
     ReferencesSelfOutput,
@@ -1009,7 +1000,7 @@ pub enum MethodViolationCode {
     Generic,
 
     /// the method's receiver (`self` argument) can't be dispatched on
-    UndispatchableReceiver,
+    UndispatchableReceiver(Option<Span>),
 }
 
 /// These are the error cases for `codegen_fulfill_obligation`.

compiler/rustc_trait_selection/src/traits/object_safety.rs

@@ -366,15 +366,9 @@ fn object_safety_violation_for_method(
     // Get an accurate span depending on the violation.
     violation.map(|v| {
         let node = tcx.hir().get_if_local(method.def_id);
-        let span = match (v, node) {
-            (MethodViolationCode::ReferencesSelfInput(arg), Some(node)) => node
-                .fn_decl()
-                .and_then(|decl| decl.inputs.get(arg + 1))
-                .map_or(method.ident(tcx).span, |arg| arg.span),
-            (MethodViolationCode::UndispatchableReceiver, Some(node)) => node
-                .fn_decl()
-                .and_then(|decl| decl.inputs.get(0))
-                .map_or(method.ident(tcx).span, |arg| arg.span),
+        let span = match (&v, node) {
+            (MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
+            (MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
             (MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
                 node.fn_decl().map_or(method.ident(tcx).span, |decl| decl.output.span())
             }
@@ -397,32 +391,41 @@ fn virtual_call_violation_for_method<'tcx>(
 
     // The method's first parameter must be named `self`
     if !method.fn_has_self_parameter {
-        // We'll attempt to provide a structured suggestion for `Self: Sized`.
-        let sugg =
-            tcx.hir().get_if_local(method.def_id).as_ref().and_then(|node| node.generics()).map(
-                |generics| match generics.predicates {
-                    [] => (" where Self: Sized", generics.where_clause_span),
-                    [.., pred] => (", Self: Sized", pred.span().shrink_to_hi()),
-                },
-            );
-        // Get the span pointing at where the `self` receiver should be.
-        let sm = tcx.sess.source_map();
-        let self_span = method.ident(tcx).span.to(tcx
-            .hir()
-            .span_if_local(method.def_id)
-            .unwrap_or_else(|| sm.next_point(method.ident(tcx).span))
-            .shrink_to_hi());
-        let self_span = sm.span_through_char(self_span, '(').shrink_to_hi();
-        return Some(MethodViolationCode::StaticMethod(
-            sugg,
-            self_span,
-            !sig.inputs().skip_binder().is_empty(),
-        ));
+        let sugg = if let Some(hir::Node::TraitItem(hir::TraitItem {
+            generics,
+            kind: hir::TraitItemKind::Fn(sig, _),
+            ..
+        })) = tcx.hir().get_if_local(method.def_id).as_ref()
+        {
+            let sm = tcx.sess.source_map();
+            Some((
+                (
+                    format!("&self{}", if sig.decl.inputs.is_empty() { "" } else { ", " }),
+                    sm.span_through_char(sig.span, '(').shrink_to_hi(),
+                ),
+                (
+                    format!("{} Self: Sized", generics.add_where_or_trailing_comma()),
+                    generics.tail_span_for_predicate_suggestion(),
+                ),
+            ))
+        } else {
+            None
+        };
+        return Some(MethodViolationCode::StaticMethod(sugg));
     }
 
-    for (i, &input_ty) in sig.skip_binder().inputs()[1..].iter().enumerate() {
+    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)) {
-            return Some(MethodViolationCode::ReferencesSelfInput(i));
+            let span = if let Some(hir::Node::TraitItem(hir::TraitItem {
+                kind: hir::TraitItemKind::Fn(sig, _),
+                ..
+            })) = tcx.hir().get_if_local(method.def_id).as_ref()
+            {
+                Some(sig.decl.inputs[i].span)
+            } else {
+                None
+            };
+            return Some(MethodViolationCode::ReferencesSelfInput(span));
         }
     }
     if contains_illegal_self_type_reference(tcx, trait_def_id, sig.output()) {
@@ -456,7 +459,16 @@ fn virtual_call_violation_for_method<'tcx>(
     // `Receiver: Unsize<Receiver[Self => dyn Trait]>`.
     if receiver_ty != tcx.types.self_param {
         if !receiver_is_dispatchable(tcx, method, receiver_ty) {
-            return Some(MethodViolationCode::UndispatchableReceiver);
+            let span = if let Some(hir::Node::TraitItem(hir::TraitItem {
+                kind: hir::TraitItemKind::Fn(sig, _),
+                ..
+            })) = tcx.hir().get_if_local(method.def_id).as_ref()
+            {
+                Some(sig.decl.inputs[0].span)
+            } else {
+                None
+            };
+            return Some(MethodViolationCode::UndispatchableReceiver(span));
         } else {
             // Do sanity check to make sure the receiver actually has the layout of a pointer.
 

src/test/ui/suggestions/auxiliary/not-object-safe.rs

@@ -0,0 +1,6 @@
+use std::sync::Arc;
+
+pub trait A {
+    fn f();
+    fn f2(self: &Arc<Self>);
+}

src/test/ui/suggestions/issue-98500.rs

@@ -0,0 +1,14 @@
+// aux-build:not-object-safe.rs
+
+extern crate not_object_safe;
+
+pub trait B where
+    Self: not_object_safe::A,
+{
+    fn f2(&self);
+}
+
+struct S(Box<dyn B>);
+//~^ ERROR the trait `B` cannot be made into an object
+
+fn main() {}

src/test/ui/suggestions/issue-98500.stderr

@@ -0,0 +1,24 @@
+error[E0038]: the trait `B` cannot be made into an object
+  --> $DIR/issue-98500.rs:11:14
+   |
+LL | struct S(Box<dyn B>);
+   |              ^^^^^ `B` 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/auxiliary/not-object-safe.rs:4:8
+   |
+LL |     fn f();
+   |        ^ ...because associated function `f` has no `self` parameter
+LL |     fn f2(self: &Arc<Self>);
+   |        ^^ ...because method `f2`'s `self` parameter cannot be dispatched on
+   |
+  ::: $DIR/issue-98500.rs:5:11
+   |
+LL | pub trait B where
+   |           - this trait cannot be made into an object...
+   = help: consider moving `f` to another trait
+   = help: consider moving `f2` to another trait
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0038`.

src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.fixed

@@ -2,7 +2,7 @@
 #![allow(unused_variables, dead_code)]
 
 trait Trait {
-    fn foo(&self) where Self: Other, Self: Sized, { }
+    fn foo(&self) where Self: Other, Self: Sized { }
     fn bar(self: &Self) {} //~ ERROR invalid `self` parameter type
 }
 

src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.stderr

@@ -28,8 +28,8 @@ LL |     fn foo(&self) where Self: Other, { }
    |            +++++
 help: alternatively, consider constraining `foo` so it does not apply to trait objects
    |
-LL |     fn foo() where Self: Other, Self: Sized, { }
-   |                               +++++++++++++
+LL |     fn foo() where Self: Other, Self: Sized { }
+   |                               ~~~~~~~~~~~~~
 help: consider changing method `bar`'s `self` parameter to be `&self`
    |
 LL |     fn bar(self: &Self) {}