clean up fulfillment error reporting

Author: Lukas Markeffsky

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -589,11 +589,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         mutate_fulfillment_errors: impl Fn(&mut Vec<traits::FulfillmentError<'tcx>>),
     ) {
-        let mut result = self.fulfillment_cx.borrow_mut().select_where_possible(self);
-        if !result.is_empty() {
-            mutate_fulfillment_errors(&mut result);
-            self.adjust_fulfillment_errors_for_expr_obligation(&mut result);
-            self.err_ctxt().report_fulfillment_errors(result);
+        let mut errors = self.fulfillment_cx.borrow_mut().select_where_possible(self);
+        if !errors.is_empty() {
+            mutate_fulfillment_errors(&mut errors);
+            if errors.is_empty() {
+                // We sometimes skip reporting fulfillment errors while constructing
+                // a different error.
+                self.dcx().span_delayed_bug(
+                    self.tcx.def_span(self.body_id),
+                    "skipped reporting fulfillment errors",
+                );
+                return;
+            }
+            self.adjust_fulfillment_errors_for_expr_obligation(&mut errors);
+            self.err_ctxt().report_fulfillment_errors(errors);
         }
     }
 

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

@@ -80,14 +80,28 @@ pub fn suggest_new_overflow_limit<'tcx, G: EmissionGuarantee>(
 
 #[extension(pub trait TypeErrCtxtExt<'tcx>)]
 impl<'tcx> TypeErrCtxt<'_, 'tcx> {
+    #[instrument(skip(self), level = "debug")]
     fn report_fulfillment_errors(
         &self,
         mut errors: Vec<FulfillmentError<'tcx>>,
     ) -> ErrorGuaranteed {
+        if errors.is_empty() {
+            bug!("attempted to report fulfillment errors, but there we no errors");
+        }
+
         self.sub_relations
             .borrow_mut()
             .add_constraints(self, errors.iter().map(|e| e.obligation.predicate));
 
+        let mut reported = None;
+
+        // We want to ignore desugarings when filtering errors: spans are equivalent even
+        // if one is the result of a desugaring and the other is not.
+        let strip_desugaring = |span: Span| {
+            let expn_data = span.ctxt().outer_expn_data();
+            if let ExpnKind::Desugaring(_) = expn_data.kind { expn_data.call_site } else { span }
+        };
+
         #[derive(Debug)]
         struct ErrorDescriptor<'tcx> {
             predicate: ty::Predicate<'tcx>,
@@ -98,40 +112,32 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             .reported_trait_errors
             .borrow()
             .iter()
-            .map(|(&span, predicates)| {
-                (
-                    span,
-                    predicates
-                        .0
-                        .iter()
-                        .map(|&predicate| ErrorDescriptor { predicate, index: None })
-                        .collect(),
-                )
+            .map(|(&span, &(ref predicates, guar))| {
+                reported = Some(guar);
+                let span = strip_desugaring(span);
+                let reported_errors = predicates
+                    .iter()
+                    .map(|&predicate| ErrorDescriptor { predicate, index: None })
+                    .collect();
+                (span, reported_errors)
             })
             .collect();
 
         // Ensure `T: Sized` and `T: WF` obligations come last. This lets us display diagnostics
-        // with more relevant type information and hide redundant E0282 errors.
+        // with more relevant type information and hide redundant E0282 ("type annotations needed") errors.
         errors.sort_by_key(|e| match e.obligation.predicate.kind().skip_binder() {
             ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred))
                 if Some(pred.def_id()) == self.tcx.lang_items().sized_trait() =>
             {
                 1
             }
-            ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(_)) => 3,
             ty::PredicateKind::Coerce(_) => 2,
+            ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(_)) => 3,
             _ => 0,
         });
 
         for (index, error) in errors.iter().enumerate() {
-            // We want to ignore desugarings here: spans are equivalent even
-            // if one is the result of a desugaring and the other is not.
-            let mut span = error.obligation.cause.span;
-            let expn_data = span.ctxt().outer_expn_data();
-            if let ExpnKind::Desugaring(_) = expn_data.kind {
-                span = expn_data.call_site;
-            }
-
+            let span = strip_desugaring(error.obligation.cause.span);
             error_map.entry(span).or_default().push(ErrorDescriptor {
                 predicate: error.obligation.predicate,
                 index: Some(index),
@@ -144,59 +150,48 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         for (_, error_set) in error_map.iter() {
             // We want to suppress "duplicate" errors with the same span.
             for error in error_set {
-                if let Some(index) = error.index {
-                    // Suppress errors that are either:
-                    // 1) strictly implied by another error.
-                    // 2) implied by an error with a smaller index.
-                    for error2 in error_set {
-                        if error2.index.is_some_and(|index2| is_suppressed[index2]) {
-                            // Avoid errors being suppressed by already-suppressed
-                            // errors, to prevent all errors from being suppressed
-                            // at once.
-                            continue;
-                        }
+                let Some(index) = error.index else {
+                    continue;
+                };
 
-                        if self.error_implies(error2.predicate, error.predicate)
-                            && !(error2.index >= error.index
-                                && self.error_implies(error.predicate, error2.predicate))
-                        {
-                            info!("skipping {:?} (implied by {:?})", error, error2);
-                            is_suppressed[index] = true;
-                            break;
-                        }
+                // Suppress errors that are either:
+                // 1) strictly implied by another error.
+                // 2) implied by an error with a smaller index.
+                for error2 in error_set {
+                    if self.error_implies(error2.predicate, error.predicate)
+                        && (!error2.index.is_some_and(|index2| index2 >= index)
+                            || !self.error_implies(error.predicate, error2.predicate))
+                    {
+                        info!("skipping `{}` (implied by `{}`)", error.predicate, error2.predicate);
+                        is_suppressed[index] = true;
+                        break;
                     }
                 }
             }
         }
 
-        let mut reported = None;
-
         for from_expansion in [false, true] {
-            for (error, suppressed) in iter::zip(&errors, &is_suppressed) {
-                if !suppressed && error.obligation.cause.span.from_expansion() == from_expansion {
-                    let guar = self.report_fulfillment_error(error);
-                    reported = Some(guar);
-                    // We want to ignore desugarings here: spans are equivalent even
-                    // if one is the result of a desugaring and the other is not.
-                    let mut span = error.obligation.cause.span;
-                    let expn_data = span.ctxt().outer_expn_data();
-                    if let ExpnKind::Desugaring(_) = expn_data.kind {
-                        span = expn_data.call_site;
-                    }
-                    self.reported_trait_errors
-                        .borrow_mut()
-                        .entry(span)
-                        .or_insert_with(|| (vec![], guar))
-                        .0
-                        .push(error.obligation.predicate);
+            for (error, &suppressed) in iter::zip(&errors, &is_suppressed) {
+                let span = error.obligation.cause.span;
+                if suppressed || span.from_expansion() != from_expansion {
+                    continue;
                 }
+
+                let guar = self.report_fulfillment_error(error);
+                reported = Some(guar);
+
+                self.reported_trait_errors
+                    .borrow_mut()
+                    .entry(span)
+                    .or_insert_with(|| (vec![], guar))
+                    .0
+                    .push(error.obligation.predicate);
             }
         }
 
-        // It could be that we don't report an error because we have seen an `ErrorReported` from
-        // another source. We should probably be able to fix most of these, but some are delayed
-        // bugs that get a proper error after this function.
-        reported.unwrap_or_else(|| self.dcx().delayed_bug("failed to report fulfillment errors"))
+        // If all errors are suppressed, then we must have reported at least one error
+        // from a previous call to this function.
+        reported.unwrap_or_else(|| bug!("failed to report fulfillment errors"))
     }
 
     /// Reports that an overflow has occurred and halts compilation. We