From eff865ca7605c0c1297aea51b377b53dbda48b3f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 25 Jun 2022 14:59:45 -0700 Subject: [PATCH] Fix span issues in object safety suggestions --- compiler/rustc_middle/src/traits/mod.rs | 67 +++++++--------- .../src/traits/object_safety.rs | 78 +++++++++++-------- .../suggestions/auxiliary/not-object-safe.rs | 6 ++ src/test/ui/suggestions/issue-98500.rs | 14 ++++ src/test/ui/suggestions/issue-98500.stderr | 24 ++++++ ...-unsafe-trait-should-use-where-sized.fixed | 2 +- ...unsafe-trait-should-use-where-sized.stderr | 4 +- 7 files changed, 121 insertions(+), 74 deletions(-) create mode 100644 src/test/ui/suggestions/auxiliary/not-object-safe.rs create mode 100644 src/test/ui/suggestions/issue-98500.rs create mode 100644 src/test/ui/suggestions/issue-98500.stderr diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 5258d37a14c91..6ec950b05e7a0 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/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), /// 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), } /// These are the error cases for `codegen_fulfill_obligation`. diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 132c335a7e65d..8d3445919156f 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/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 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. diff --git a/src/test/ui/suggestions/auxiliary/not-object-safe.rs b/src/test/ui/suggestions/auxiliary/not-object-safe.rs new file mode 100644 index 0000000000000..7c9829b823ede --- /dev/null +++ b/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); +} diff --git a/src/test/ui/suggestions/issue-98500.rs b/src/test/ui/suggestions/issue-98500.rs new file mode 100644 index 0000000000000..a2717fd9206d1 --- /dev/null +++ b/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); +//~^ ERROR the trait `B` cannot be made into an object + +fn main() {} diff --git a/src/test/ui/suggestions/issue-98500.stderr b/src/test/ui/suggestions/issue-98500.stderr new file mode 100644 index 0000000000000..e7251d735e38e --- /dev/null +++ b/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); + | ^^^^^ `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 + --> $DIR/auxiliary/not-object-safe.rs:4:8 + | +LL | fn f(); + | ^ ...because associated function `f` has no `self` parameter +LL | fn f2(self: &Arc); + | ^^ ...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`. diff --git a/src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.fixed b/src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.fixed index 73bb6725f5a72..69487c565c933 100644 --- a/src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.fixed +++ b/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 } diff --git a/src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.stderr b/src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.stderr index 74237e6e6c638..66969c1706654 100644 --- a/src/test/ui/suggestions/object-unsafe-trait-should-use-where-sized.stderr +++ b/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) {}