Tweak obligation error output

src/librustc/traits/error_reporting/mod.rs

@@ -1034,6 +1034,10 @@ pub fn report_object_safety_error(
     violations: Vec<ObjectSafetyViolation>,
 ) -> DiagnosticBuilder<'tcx> {
     let trait_str = tcx.def_path_str(trait_def_id);
+    let trait_span = tcx.hir().get_if_local(trait_def_id).and_then(|node| match node {
+        hir::Node::Item(item) => Some(item.ident.span),
+        _ => None,
+    });
     let span = tcx.sess.source_map().def_span(span);
     let mut err = struct_span_err!(
         tcx.sess,
@@ -1045,14 +1049,39 @@ pub fn report_object_safety_error(
     err.span_label(span, format!("the trait `{}` cannot be made into an object", trait_str));
 
     let mut reported_violations = FxHashSet::default();
+    let mut had_span_label = false;
     for violation in violations {
+        if let ObjectSafetyViolation::SizedSelf(sp) = &violation {
+            if !sp.is_empty() {
+                // Do not report `SizedSelf` without spans pointing at `SizedSelf` obligations
+                // with a `Span`.
+                reported_violations.insert(ObjectSafetyViolation::SizedSelf(vec![].into()));
+            }
+        }
         if reported_violations.insert(violation.clone()) {
-            match violation.span() {
-                Some(span) => err.span_label(span, violation.error_msg()),
-                None => err.note(&violation.error_msg()),
+            let spans = violation.spans();
+            let msg = if trait_span.is_none() || spans.is_empty() {
+                format!("the trait cannot be made into an object because {}", violation.error_msg())
+            } else {
+                had_span_label = true;
+                format!("...because {}", violation.error_msg())
             };
+            if spans.is_empty() {
+                err.note(&msg);
+            } else {
+                for span in spans {
+                    err.span_label(span, &msg);
+                }
+            }
+            if let (Some(_), Some(note)) = (trait_span, violation.solution()) {
+                // Only provide the help if its a local trait, otherwise it's not actionable.
+                err.help(&note);
+            }
         }
     }
+    if let (Some(trait_span), true) = (trait_span, had_span_label) {
+        err.span_label(trait_span, "this trait cannot be made into an object...");
+    }
 
     if tcx.sess.trait_methods_not_found.borrow().contains(&span) {
         // Avoid emitting error caused by non-existing method (#58734)

src/librustc/traits/object_safety.rs

@@ -18,15 +18,16 @@ use rustc_hir::def_id::DefId;
 use rustc_session::lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY;
 use rustc_span::symbol::Symbol;
 use rustc_span::{Span, DUMMY_SP};
+use smallvec::{smallvec, SmallVec};
 use syntax::ast;
 
 use std::borrow::Cow;
 use std::iter::{self};
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum ObjectSafetyViolation {
     /// `Self: Sized` declared on the trait.
-    SizedSelf,
+    SizedSelf(SmallVec<[Span; 1]>),
 
     /// Supertrait reference references `Self` an in illegal location
     /// (e.g., `trait Foo : Bar<Self>`).
@@ -42,12 +43,9 @@ pub enum ObjectSafetyViolation {
 impl ObjectSafetyViolation {
     pub fn error_msg(&self) -> Cow<'static, str> {
         match *self {
-            ObjectSafetyViolation::SizedSelf => {
-                "the trait cannot require that `Self : Sized`".into()
-            }
+            ObjectSafetyViolation::SizedSelf(_) => "it requires `Self: Sized`".into(),
             ObjectSafetyViolation::SupertraitSelf => {
-                "the trait cannot use `Self` as a type parameter \
-                 in the supertraits or where-clauses"
+                "it cannot use `Self` as a type parameter in the supertraits or `where`-clauses"
                     .into()
             }
             ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod, _) => {
@@ -62,30 +60,57 @@ impl ObjectSafetyViolation {
                 name,
                 MethodViolationCode::WhereClauseReferencesSelf,
                 _,
-            ) => format!("method `{}` references the `Self` type in where clauses", name).into(),
+            ) => {
+                format!("method `{}` references the `Self` type in its `where` clause", name).into()
+            }
             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::AssocConst(_, DUMMY_SP) => {
+                "it cannot contain associated consts".into()
+            }
             ObjectSafetyViolation::AssocConst(name, _) => {
-                format!("the trait cannot contain associated consts like `{}`", name).into()
+                format!("it cannot contain associated consts like `{}`", name).into()
             }
         }
     }
 
-    pub fn span(&self) -> Option<Span> {
+    pub fn solution(&self) -> Option<String> {
+        Some(match *self {
+            ObjectSafetyViolation::SizedSelf(_) | ObjectSafetyViolation::SupertraitSelf => {
+                return None;
+            }
+            ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod, _) => format!(
+                "consider turning `{}` into a method by giving it a `&self` argument or \
+                 constraining it with `where Self: Sized`",
+                name
+            ),
+            ObjectSafetyViolation::Method(name, MethodViolationCode::UndispatchableReceiver, _) => {
+                format!("consider changing method `{}`'s `self` parameter to be `&self`", name)
+                    .into()
+            }
+            ObjectSafetyViolation::AssocConst(name, _)
+            | ObjectSafetyViolation::Method(name, ..) => {
+                format!("consider moving `{}` to another trait", name)
+            }
+        })
+    }
+
+    pub fn spans(&self) -> SmallVec<[Span; 1]> {
         // 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 {
+        match self {
+            ObjectSafetyViolation::SizedSelf(spans) => spans.clone(),
             ObjectSafetyViolation::AssocConst(_, span)
             | ObjectSafetyViolation::Method(_, _, span)
-                if span != DUMMY_SP =>
+                if *span != DUMMY_SP =>
             {
-                Some(span)
+                smallvec![*span]
             }
-            _ => None,
+            _ => smallvec![],
         }
     }
 }
@@ -179,17 +204,31 @@ fn object_safety_violations_for_trait(
             {
                 // 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.
-                tcx.struct_span_lint_hir(
+                let mut err = tcx.struct_span_lint_hir(
                     WHERE_CLAUSES_OBJECT_SAFETY,
                     hir::CRATE_HIR_ID,
                     *span,
                     &format!(
                         "the trait `{}` cannot be made into an object",
                         tcx.def_path_str(trait_def_id)
                     ),
-                )
-                .note(&violation.error_msg())
-                .emit();
+                );
+                let node = tcx.hir().get_if_local(trait_def_id);
+                let msg = if let Some(hir::Node::Item(item)) = node {
+                    err.span_label(item.ident.span, "this trait cannot be made into an object...");
+                    format!("...because {}", violation.error_msg())
+                } else {
+                    format!(
+                        "the trait cannot be made into an object because {}",
+                        violation.error_msg()
+                    )
+                };
+                err.span_label(*span, &msg);
+                if let (Some(_), Some(note)) = (node, violation.solution()) {
+                    // Only provide the help if its a local trait, otherwise it's not actionable.
+                    err.help(&note);
+                }
+                err.emit();
                 false
             } else {
                 true
@@ -199,7 +238,9 @@ fn object_safety_violations_for_trait(
 
     // Check the trait itself.
     if trait_has_sized_self(tcx, trait_def_id) {
-        violations.push(ObjectSafetyViolation::SizedSelf);
+        // We don't want to include the requirement from `Sized` itself to be `Sized` in the list.
+        let spans = get_sized_bounds(tcx, trait_def_id);
+        violations.push(ObjectSafetyViolation::SizedSelf(spans));
     }
     if predicates_reference_self(tcx, trait_def_id, false) {
         violations.push(ObjectSafetyViolation::SupertraitSelf);
@@ -219,6 +260,59 @@ fn object_safety_violations_for_trait(
     violations
 }
 
+fn get_sized_bounds(tcx: TyCtxt<'_>, trait_def_id: DefId) -> SmallVec<[Span; 1]> {
+    tcx.hir()
+        .get_if_local(trait_def_id)
+        .and_then(|node| match node {
+            hir::Node::Item(hir::Item {
+                kind: hir::ItemKind::Trait(.., generics, bounds, _),
+                ..
+            }) => Some(
+                generics
+                    .where_clause
+                    .predicates
+                    .iter()
+                    .filter_map(|pred| {
+                        match pred {
+                            hir::WherePredicate::BoundPredicate(pred)
+                                if pred.bounded_ty.hir_id.owner_def_id() == trait_def_id =>
+                            {
+                                // Fetch spans for trait bounds that are Sized:
+                                // `trait T where Self: Pred`
+                                Some(pred.bounds.iter().filter_map(|b| match b {
+                                    hir::GenericBound::Trait(
+                                        trait_ref,
+                                        hir::TraitBoundModifier::None,
+                                    ) if trait_has_sized_self(
+                                        tcx,
+                                        trait_ref.trait_ref.trait_def_id(),
+                                    ) =>
+                                    {
+                                        Some(trait_ref.span)
+                                    }
+                                    _ => None,
+                                }))
+                            }
+                            _ => None,
+                        }
+                    })
+                    .flatten()
+                    .chain(bounds.iter().filter_map(|b| match b {
+                        hir::GenericBound::Trait(trait_ref, hir::TraitBoundModifier::None)
+                            if trait_has_sized_self(tcx, trait_ref.trait_ref.trait_def_id()) =>
+                        {
+                            // Fetch spans for supertraits that are `Sized`: `trait T: Super`
+                            Some(trait_ref.span)
+                        }
+                        _ => None,
+                    }))
+                    .collect::<SmallVec<[Span; 1]>>(),
+            ),
+            _ => None,
+        })
+        .unwrap_or_else(SmallVec::new)
+}
+
 fn predicates_reference_self(tcx: TyCtxt<'_>, trait_def_id: DefId, supertraits_only: bool) -> bool {
     let trait_ref = ty::Binder::dummy(ty::TraitRef::identity(tcx, trait_def_id));
     let predicates = if supertraits_only {

src/librustc/ty/mod.rs

@@ -1015,6 +1015,7 @@ impl<'tcx> GenericPredicates<'tcx> {
     ) -> InstantiatedPredicates<'tcx> {
         InstantiatedPredicates {
             predicates: self.predicates.iter().map(|(p, _)| p.subst(tcx, substs)).collect(),
+            spans: self.predicates.iter().map(|(_, sp)| *sp).collect(),
         }
     }
 
@@ -1028,6 +1029,7 @@ impl<'tcx> GenericPredicates<'tcx> {
             tcx.predicates_of(def_id).instantiate_into(tcx, instantiated, substs);
         }
         instantiated.predicates.extend(self.predicates.iter().map(|(p, _)| p.subst(tcx, substs)));
+        instantiated.spans.extend(self.predicates.iter().map(|(_, sp)| *sp));
     }
 
     pub fn instantiate_identity(&self, tcx: TyCtxt<'tcx>) -> InstantiatedPredicates<'tcx> {
@@ -1044,7 +1046,8 @@ impl<'tcx> GenericPredicates<'tcx> {
         if let Some(def_id) = self.parent {
             tcx.predicates_of(def_id).instantiate_identity_into(tcx, instantiated);
         }
-        instantiated.predicates.extend(self.predicates.iter().map(|&(p, _)| p))
+        instantiated.predicates.extend(self.predicates.iter().map(|(p, _)| p));
+        instantiated.spans.extend(self.predicates.iter().map(|(_, s)| s));
     }
 
     pub fn instantiate_supertrait(
@@ -1059,6 +1062,7 @@ impl<'tcx> GenericPredicates<'tcx> {
                 .iter()
                 .map(|(pred, _)| pred.subst_supertrait(tcx, poly_trait_ref))
                 .collect(),
+            spans: self.predicates.iter().map(|(_, sp)| *sp).collect(),
         }
     }
 }
@@ -1511,11 +1515,12 @@ impl<'tcx> Predicate<'tcx> {
 #[derive(Clone, Debug, TypeFoldable)]
 pub struct InstantiatedPredicates<'tcx> {
     pub predicates: Vec<Predicate<'tcx>>,
+    pub spans: Vec<Span>,
 }
 
 impl<'tcx> InstantiatedPredicates<'tcx> {
     pub fn empty() -> InstantiatedPredicates<'tcx> {
-        InstantiatedPredicates { predicates: vec![] }
+        InstantiatedPredicates { predicates: vec![], spans: vec![] }
     }
 
     pub fn is_empty(&self) -> bool {

src/librustc_traits/lowering/environment.rs

@@ -161,7 +161,7 @@ crate fn environment(tcx: TyCtxt<'_>, def_id: DefId) -> Environment<'_> {
     }
 
     // Compute the bounds on `Self` and the type parameters.
-    let ty::InstantiatedPredicates { predicates } =
+    let ty::InstantiatedPredicates { predicates, .. } =
         tcx.predicates_of(def_id).instantiate_identity(tcx);
 
     let clauses = predicates

src/librustc_ty/ty.rs

@@ -228,7 +228,7 @@ fn param_env(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ParamEnv<'_> {
     }
     // Compute the bounds on Self and the type parameters.
 
-    let ty::InstantiatedPredicates { predicates } =
+    let ty::InstantiatedPredicates { predicates, .. } =
         tcx.predicates_of(def_id).instantiate_identity(tcx);
 
     // Finally, we have to normalize the bounds in the environment, in

src/librustc_typeck/check/method/confirm.rs

@@ -32,7 +32,7 @@ impl<'a, 'tcx> Deref for ConfirmContext<'a, 'tcx> {
 
 pub struct ConfirmResult<'tcx> {
     pub callee: MethodCallee<'tcx>,
-    pub illegal_sized_bound: bool,
+    pub illegal_sized_bound: Option<Span>,
 }
 
 impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
@@ -112,7 +112,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
         // Add any trait/regions obligations specified on the method's type parameters.
         // We won't add these if we encountered an illegal sized bound, so that we can use
         // a custom error in that case.
-        if !illegal_sized_bound {
+        if illegal_sized_bound.is_none() {
             let method_ty = self.tcx.mk_fn_ptr(ty::Binder::bind(method_sig));
             self.add_obligations(method_ty, all_substs, &method_predicates);
         }
@@ -561,23 +561,31 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
     fn predicates_require_illegal_sized_bound(
         &self,
         predicates: &ty::InstantiatedPredicates<'tcx>,
-    ) -> bool {
+    ) -> Option<Span> {
         let sized_def_id = match self.tcx.lang_items().sized_trait() {
             Some(def_id) => def_id,
-            None => return false,
+            None => return None,
         };
 
         traits::elaborate_predicates(self.tcx, predicates.predicates.clone())
             .filter_map(|predicate| match predicate {
                 ty::Predicate::Trait(trait_pred, _) if trait_pred.def_id() == sized_def_id => {
-                    Some(trait_pred)
+                    let span = predicates
+                        .predicates
+                        .iter()
+                        .zip(predicates.spans.iter())
+                        .filter_map(|(p, span)| if *p == predicate { Some(*span) } else { None })
+                        .next()
+                        .unwrap_or(rustc_span::DUMMY_SP);
+                    Some((trait_pred, span))
                 }
                 _ => None,
             })
-            .any(|trait_pred| match trait_pred.skip_binder().self_ty().kind {
-                ty::Dynamic(..) => true,
-                _ => false,
+            .filter_map(|(trait_pred, span)| match trait_pred.skip_binder().self_ty().kind {
+                ty::Dynamic(..) => Some(span),
+                _ => None,
             })
+            .next()
     }
 
     fn enforce_illegal_method_limitations(&self, pick: &probe::Pick<'_>) {