Auto merge of #89427 - estebank:collect-overlapping-impls, r=jackh726

Point at overlapping impls when type annotations are needed

Address #89254.

Author: bors

compiler/rustc_errors/src/diagnostic.rs

@@ -465,10 +465,14 @@ impl Diagnostic {
         suggestions: impl Iterator<Item = String>,
         applicability: Applicability,
     ) -> &mut Self {
+        let mut suggestions: Vec<_> = suggestions.collect();
+        suggestions.sort();
+        let substitutions = suggestions
+            .into_iter()
+            .map(|snippet| Substitution { parts: vec![SubstitutionPart { snippet, span: sp }] })
+            .collect();
         self.suggestions.push(CodeSuggestion {
-            substitutions: suggestions
-                .map(|snippet| Substitution { parts: vec![SubstitutionPart { snippet, span: sp }] })
-                .collect(),
+            substitutions,
             msg: msg.to_owned(),
             style: SuggestionStyle::ShowCode,
             applicability,

compiler/rustc_middle/src/traits/mod.rs

@@ -440,16 +440,28 @@ pub struct DerivedObligationCause<'tcx> {
 
 #[derive(Clone, Debug, TypeFoldable, Lift)]
 pub enum SelectionError<'tcx> {
+    /// The trait is not implemented.
     Unimplemented,
+    /// After a closure impl has selected, its "outputs" were evaluated
+    /// (which for closures includes the "input" type params) and they
+    /// didn't resolve. See `confirm_poly_trait_refs` for more.
     OutputTypeParameterMismatch(
         ty::PolyTraitRef<'tcx>,
         ty::PolyTraitRef<'tcx>,
         ty::error::TypeError<'tcx>,
     ),
+    /// The trait pointed by `DefId` is not object safe.
     TraitNotObjectSafe(DefId),
+    /// A given constant couldn't be evaluated.
     NotConstEvaluatable(NotConstEvaluatable),
+    /// Exceeded the recursion depth during type projection.
     Overflow,
+    /// Signaling that an error has already been emitted, to avoid
+    /// multiple errors being shown.
     ErrorReporting,
+    /// Multiple applicable `impl`s where found. The `DefId`s correspond to
+    /// all the `impl`s' Items.
+    Ambiguous(Vec<DefId>),
 }
 
 /// When performing resolution, it is typically the case that there

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -34,6 +34,7 @@ use std::iter;
 
 use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
 use crate::traits::query::normalize::AtExt as _;
+use crate::traits::specialize::to_pretty_impl_header;
 use on_unimplemented::InferCtxtExt as _;
 use suggestions::InferCtxtExt as _;
 
@@ -241,6 +242,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         let mut span = obligation.cause.span;
 
         let mut err = match *error {
+            SelectionError::Ambiguous(ref impls) => {
+                let mut err = self.tcx.sess.struct_span_err(
+                    obligation.cause.span,
+                    &format!("multiple applicable `impl`s for `{}`", obligation.predicate),
+                );
+                self.annotate_source_of_ambiguity(&mut err, impls, obligation.predicate);
+                err.emit();
+                return;
+            }
             SelectionError::Unimplemented => {
                 // If this obligation was generated as a result of well-formedness checking, see if we
                 // can get a better error message by performing HIR-based well-formedness checking.
@@ -1138,6 +1148,13 @@ trait InferCtxtPrivExt<'tcx> {
         obligation: &PredicateObligation<'tcx>,
     );
 
+    fn annotate_source_of_ambiguity(
+        &self,
+        err: &mut DiagnosticBuilder<'tcx>,
+        impls: &[DefId],
+        predicate: ty::Predicate<'tcx>,
+    );
+
     fn maybe_suggest_unsized_generics(
         &self,
         err: &mut DiagnosticBuilder<'tcx>,
@@ -1549,11 +1566,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
             ?predicate, ?obligation.cause.code,
         );
 
-        // Ambiguity errors are often caused as fallout from earlier
-        // errors. So just ignore them if this infcx is tainted.
-        if self.is_tainted_by_errors() {
-            return;
-        }
+        // Ambiguity errors are often caused as fallout from earlier errors.
+        // We ignore them if this `infcx` is tainted in some cases below.
 
         let bound_predicate = predicate.kind();
         let mut err = match bound_predicate.skip_binder() {
@@ -1601,10 +1615,19 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
                 // check upstream for type errors and don't add the obligations to
                 // begin with in those cases.
                 if self.tcx.lang_items().sized_trait() == Some(trait_ref.def_id()) {
-                    self.emit_inference_failure_err(body_id, span, subst, vec![], ErrorCode::E0282)
+                    if !self.is_tainted_by_errors() {
+                        self.emit_inference_failure_err(
+                            body_id,
+                            span,
+                            subst,
+                            vec![],
+                            ErrorCode::E0282,
+                        )
                         .emit();
+                    }
                     return;
                 }
+
                 let impl_candidates = self.find_similar_impl_candidates(trait_ref);
                 let mut err = self.emit_inference_failure_err(
                     body_id,
@@ -1613,7 +1636,29 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
                     impl_candidates,
                     ErrorCode::E0283,
                 );
-                err.note(&format!("cannot satisfy `{}`", predicate));
+
+                let obligation = Obligation::new(
+                    obligation.cause.clone(),
+                    obligation.param_env,
+                    trait_ref.to_poly_trait_predicate(),
+                );
+                let mut selcx = SelectionContext::with_query_mode(
+                    &self,
+                    crate::traits::TraitQueryMode::Standard,
+                );
+                match selcx.select_from_obligation(&obligation) {
+                    Err(SelectionError::Ambiguous(impls)) if impls.len() > 1 => {
+                        self.annotate_source_of_ambiguity(&mut err, &impls, predicate);
+                    }
+                    _ => {
+                        if self.is_tainted_by_errors() {
+                            err.cancel();
+                            return;
+                        }
+                        err.note(&format!("cannot satisfy `{}`", predicate));
+                    }
+                }
+
                 if let ObligationCauseCode::ItemObligation(def_id) = obligation.cause.code {
                     self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
                 } else if let (
@@ -1674,15 +1719,21 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
             ty::PredicateKind::WellFormed(arg) => {
                 // Same hacky approach as above to avoid deluging user
                 // with error messages.
-                if arg.references_error() || self.tcx.sess.has_errors() {
+                if arg.references_error()
+                    || self.tcx.sess.has_errors()
+                    || self.is_tainted_by_errors()
+                {
                     return;
                 }
 
                 self.emit_inference_failure_err(body_id, span, arg, vec![], ErrorCode::E0282)
             }
 
             ty::PredicateKind::Subtype(data) => {
-                if data.references_error() || self.tcx.sess.has_errors() {
+                if data.references_error()
+                    || self.tcx.sess.has_errors()
+                    || self.is_tainted_by_errors()
+                {
                     // no need to overload user in such cases
                     return;
                 }
@@ -1694,7 +1745,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
             ty::PredicateKind::Projection(data) => {
                 let self_ty = data.projection_ty.self_ty();
                 let ty = data.ty;
-                if predicate.references_error() {
+                if predicate.references_error() || self.is_tainted_by_errors() {
                     return;
                 }
                 if self_ty.needs_infer() && ty.needs_infer() {
@@ -1722,7 +1773,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
             }
 
             _ => {
-                if self.tcx.sess.has_errors() {
+                if self.tcx.sess.has_errors() || self.is_tainted_by_errors() {
                     return;
                 }
                 let mut err = struct_span_err!(
@@ -1740,6 +1791,96 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
         err.emit();
     }
 
+    fn annotate_source_of_ambiguity(
+        &self,
+        err: &mut DiagnosticBuilder<'tcx>,
+        impls: &[DefId],
+        predicate: ty::Predicate<'tcx>,
+    ) {
+        let mut spans = vec![];
+        let mut crates = vec![];
+        let mut post = vec![];
+        for def_id in impls {
+            match self.tcx.span_of_impl(*def_id) {
+                Ok(span) => spans.push(self.tcx.sess.source_map().guess_head_span(span)),
+                Err(name) => {
+                    crates.push(name);
+                    if let Some(header) = to_pretty_impl_header(self.tcx, *def_id) {
+                        post.push(header);
+                    }
+                }
+            }
+        }
+        let msg = format!("multiple `impl`s satisfying `{}` found", predicate);
+        let mut crate_names: Vec<_> = crates.iter().map(|n| format!("`{}`", n)).collect();
+        crate_names.sort();
+        crate_names.dedup();
+        post.sort();
+        post.dedup();
+
+        if self.is_tainted_by_errors()
+            && crate_names.len() == 1
+            && crate_names[0] == "`core`"
+            && spans.len() == 0
+        {
+            // Avoid complaining about other inference issues for expressions like
+            // `42 >> 1`, where the types are still `{integer}`, but we want to
+            // Do we need `trait_ref.skip_binder().self_ty().is_numeric() &&` too?
+            err.cancel();
+            return;
+        }
+        let post = if post.len() > 4 {
+            format!(
+                ":\n{}\nand {} more",
+                post.iter().map(|p| format!("- {}", p)).take(4).collect::<Vec<_>>().join("\n"),
+                post.len() - 4,
+            )
+        } else if post.len() > 1 || (post.len() == 1 && post[0].contains("\n")) {
+            format!(":\n{}", post.iter().map(|p| format!("- {}", p)).collect::<Vec<_>>().join("\n"),)
+        } else if post.len() == 1 {
+            format!(": `{}`", post[0])
+        } else {
+            String::new()
+        };
+
+        match (spans.len(), crates.len(), crate_names.len()) {
+            (0, 0, 0) => {
+                err.note(&format!("cannot satisfy `{}`", predicate));
+            }
+            (0, _, 1) => {
+                err.note(&format!("{} in the `{}` crate{}", msg, crates[0], post,));
+            }
+            (0, _, _) => {
+                err.note(&format!(
+                    "{} in the following crates: {}{}",
+                    msg,
+                    crate_names.join(", "),
+                    post,
+                ));
+            }
+            (_, 0, 0) => {
+                let span: MultiSpan = spans.into();
+                err.span_note(span, &msg);
+            }
+            (_, 1, 1) => {
+                let span: MultiSpan = spans.into();
+                err.span_note(span, &msg);
+                err.note(
+                    &format!("and another `impl` found in the `{}` crate{}", crates[0], post,),
+                );
+            }
+            _ => {
+                let span: MultiSpan = spans.into();
+                err.span_note(span, &msg);
+                err.note(&format!(
+                    "and more `impl`s found in the following crates: {}{}",
+                    crate_names.join(", "),
+                    post,
+                ));
+            }
+        }
+    }
+
     /// Returns `true` if the trait predicate may apply for *some* assignment
     /// to the type parameters.
     fn predicate_can_apply(

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -299,18 +299,15 @@ fn suggest_restriction(
                 generics,
                 trait_ref.without_const().to_predicate(tcx).to_string(),
             ),
-            (None, Some((ident, []))) => (
-                ident.span.shrink_to_hi(),
-                format!(": {}", trait_ref.print_only_trait_path().to_string()),
-            ),
-            (_, Some((_, [.., bounds]))) => (
-                bounds.span().shrink_to_hi(),
-                format!(" + {}", trait_ref.print_only_trait_path().to_string()),
-            ),
-            (Some(_), Some((_, []))) => (
-                generics.span.shrink_to_hi(),
-                format!(": {}", trait_ref.print_only_trait_path().to_string()),
-            ),
+            (None, Some((ident, []))) => {
+                (ident.span.shrink_to_hi(), format!(": {}", trait_ref.print_only_trait_path()))
+            }
+            (_, Some((_, [.., bounds]))) => {
+                (bounds.span().shrink_to_hi(), format!(" + {}", trait_ref.print_only_trait_path()))
+            }
+            (Some(_), Some((_, []))) => {
+                (generics.span.shrink_to_hi(), format!(": {}", trait_ref.print_only_trait_path()))
+            }
         };
 
         err.span_suggestion_verbose(

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

@@ -18,7 +18,7 @@ use crate::traits;
 use crate::traits::coherence::Conflict;
 use crate::traits::query::evaluate_obligation::InferCtxtExt;
 use crate::traits::{util, SelectionResult};
-use crate::traits::{ErrorReporting, Overflow, Unimplemented};
+use crate::traits::{Ambiguous, ErrorReporting, Overflow, Unimplemented};
 
 use super::BuiltinImplConditions;
 use super::IntercrateAmbiguityCause;
@@ -197,7 +197,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     // and report ambiguity.
                     if i > 1 {
                         debug!("multiple matches, ambig");
-                        return Ok(None);
+                        return Err(Ambiguous(
+                            candidates
+                                .into_iter()
+                                .filter_map(|c| match c.candidate {
+                                    SelectionCandidate::ImplCandidate(def_id) => Some(def_id),
+                                    _ => None,
+                                })
+                                .collect(),
+                        ));
                     }
                 }
             }

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -357,18 +357,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         &mut self,
         obligation: &TraitObligation<'tcx>,
     ) -> SelectionResult<'tcx, Selection<'tcx>> {
-        debug_assert!(!obligation.predicate.has_escaping_bound_vars());
-
-        let pec = &ProvisionalEvaluationCache::default();
-        let stack = self.push_stack(TraitObligationStackList::empty(pec), obligation);
-
-        let candidate = match self.candidate_from_obligation(&stack) {
+        let candidate = match self.select_from_obligation(obligation) {
             Err(SelectionError::Overflow) => {
                 // In standard mode, overflow must have been caught and reported
                 // earlier.
                 assert!(self.query_mode == TraitQueryMode::Canonical);
                 return Err(SelectionError::Overflow);
             }
+            Err(SelectionError::Ambiguous(_)) => {
+                return Ok(None);
+            }
             Err(e) => {
                 return Err(e);
             }
@@ -391,6 +389,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         }
     }
 
+    crate fn select_from_obligation(
+        &mut self,
+        obligation: &TraitObligation<'tcx>,
+    ) -> SelectionResult<'tcx, SelectionCandidate<'tcx>> {
+        debug_assert!(!obligation.predicate.has_escaping_bound_vars());
+
+        let pec = &ProvisionalEvaluationCache::default();
+        let stack = self.push_stack(TraitObligationStackList::empty(pec), obligation);
+
+        self.candidate_from_obligation(&stack)
+    }
+
     ///////////////////////////////////////////////////////////////////////////
     // EVALUATION
     //
@@ -915,6 +925,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         match self.candidate_from_obligation(stack) {
             Ok(Some(c)) => self.evaluate_candidate(stack, &c),
+            Err(SelectionError::Ambiguous(_)) => Ok(EvaluatedToAmbig),
             Ok(None) => Ok(EvaluatedToAmbig),
             Err(Overflow) => Err(OverflowError::Canonical),
             Err(ErrorReporting) => Err(OverflowError::ErrorReporting),