Auto merge of #90700 - fee1-dead:select-returns-vec, r=davidtwco

Make `select_*` methods return `Vec` for `TraitEngine`

This reduces some complexity as an empty vec means no errors and non-empty vec means errors occurred.

Author: bors

compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs

@@ -339,7 +339,7 @@ fn try_extract_error_from_fulfill_cx<'tcx>(
     // We generally shouldn't have errors here because the query was
     // already run, but there's no point using `delay_span_bug`
     // when we're going to emit an error here anyway.
-    let _errors = fulfill_cx.select_all_or_error(infcx).err().unwrap_or_else(Vec::new);
+    let _errors = fulfill_cx.select_all_or_error(infcx);
 
     let (sub_region, cause) = infcx.with_region_constraints(|region_constraints| {
         debug!("{:#?}", region_constraints);

compiler/rustc_const_eval/src/transform/check_consts/check.rs

@@ -1054,8 +1054,9 @@ fn check_return_ty_is_sync(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, hir_id: HirId)
         let mut fulfillment_cx = traits::FulfillmentContext::new();
         let sync_def_id = tcx.require_lang_item(LangItem::Sync, Some(body.span));
         fulfillment_cx.register_bound(&infcx, ty::ParamEnv::empty(), ty, sync_def_id, cause);
-        if let Err(err) = fulfillment_cx.select_all_or_error(&infcx) {
-            infcx.report_fulfillment_errors(&err, None, false);
+        let errors = fulfillment_cx.select_all_or_error(&infcx);
+        if !errors.is_empty() {
+            infcx.report_fulfillment_errors(&errors, None, false);
         }
     });
 }

compiler/rustc_infer/src/infer/canonical/query_response.rs

@@ -108,7 +108,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
         let tcx = self.tcx;
 
         // Select everything, returning errors.
-        let true_errors = fulfill_cx.select_where_possible(self).err().unwrap_or_else(Vec::new);
+        let true_errors = fulfill_cx.select_where_possible(self);
         debug!("true_errors = {:#?}", true_errors);
 
         if !true_errors.is_empty() {
@@ -118,7 +118,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
         }
 
         // Anything left unselected *now* must be an ambiguity.
-        let ambig_errors = fulfill_cx.select_all_or_error(self).err().unwrap_or_else(Vec::new);
+        let ambig_errors = fulfill_cx.select_all_or_error(self);
         debug!("ambig_errors = {:#?}", ambig_errors);
 
         let region_obligations = self.take_registered_region_obligations();

compiler/rustc_infer/src/traits/engine.rs

@@ -46,30 +46,25 @@ pub trait TraitEngine<'tcx>: 'tcx {
         obligation: PredicateObligation<'tcx>,
     );
 
-    fn select_all_or_error(
-        &mut self,
-        infcx: &InferCtxt<'_, 'tcx>,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>>;
+    fn select_all_or_error(&mut self, infcx: &InferCtxt<'_, 'tcx>) -> Vec<FulfillmentError<'tcx>>;
 
     fn select_all_with_constness_or_error(
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
         _constness: hir::Constness,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
+    ) -> Vec<FulfillmentError<'tcx>> {
         self.select_all_or_error(infcx)
     }
 
-    fn select_where_possible(
-        &mut self,
-        infcx: &InferCtxt<'_, 'tcx>,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>>;
+    fn select_where_possible(&mut self, infcx: &InferCtxt<'_, 'tcx>)
+    -> Vec<FulfillmentError<'tcx>>;
 
     // FIXME(fee1-dead) this should not provide a default body for chalk as chalk should be updated
     fn select_with_constness_where_possible(
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
         _constness: hir::Constness,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
+    ) -> Vec<FulfillmentError<'tcx>> {
         self.select_where_possible(infcx)
     }
 

compiler/rustc_trait_selection/src/autoderef.rs

@@ -152,11 +152,12 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
             },
             cause,
         );
-        if let Err(e) = fulfillcx.select_where_possible(&self.infcx) {
+        let errors = fulfillcx.select_where_possible(&self.infcx);
+        if !errors.is_empty() {
             // This shouldn't happen, except for evaluate/fulfill mismatches,
             // but that's not a reason for an ICE (`predicate_may_hold` is conservative
             // by design).
-            debug!("overloaded_deref_ty: encountered errors {:?} while fulfilling", e);
+            debug!("overloaded_deref_ty: encountered errors {:?} while fulfilling", errors);
             return None;
         }
         let obligations = fulfillcx.pending_obligations();

compiler/rustc_trait_selection/src/traits/auto_trait.rs

@@ -187,9 +187,11 @@ impl<'tcx> AutoTraitFinder<'tcx> {
             // an additional sanity check.
             let mut fulfill = FulfillmentContext::new();
             fulfill.register_bound(&infcx, full_env, ty, trait_did, ObligationCause::dummy());
-            fulfill.select_all_or_error(&infcx).unwrap_or_else(|e| {
-                panic!("Unable to fulfill trait {:?} for '{:?}': {:?}", trait_did, ty, e)
-            });
+            let errors = fulfill.select_all_or_error(&infcx);
+
+            if !errors.is_empty() {
+                panic!("Unable to fulfill trait {:?} for '{:?}': {:?}", trait_did, ty, errors);
+            }
 
             let body_id_map: FxHashMap<_, _> = infcx
                 .inner

compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs

@@ -49,34 +49,32 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
         self.obligations.insert(obligation);
     }
 
-    fn select_all_or_error(
-        &mut self,
-        infcx: &InferCtxt<'_, 'tcx>,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
-        self.select_where_possible(infcx)?;
-
-        if self.obligations.is_empty() {
-            Ok(())
-        } else {
-            let errors = self
-                .obligations
-                .iter()
-                .map(|obligation| FulfillmentError {
-                    obligation: obligation.clone(),
-                    code: FulfillmentErrorCode::CodeAmbiguity,
-                    // FIXME - does Chalk have a notation of 'root obligation'?
-                    // This is just for diagnostics, so it's okay if this is wrong
-                    root_obligation: obligation.clone(),
-                })
-                .collect();
-            Err(errors)
+    fn select_all_or_error(&mut self, infcx: &InferCtxt<'_, 'tcx>) -> Vec<FulfillmentError<'tcx>> {
+        {
+            let errors = self.select_where_possible(infcx);
+
+            if !errors.is_empty() {
+                return errors;
+            }
         }
+
+        // any remaining obligations are errors
+        self.obligations
+            .iter()
+            .map(|obligation| FulfillmentError {
+                obligation: obligation.clone(),
+                code: FulfillmentErrorCode::CodeAmbiguity,
+                // FIXME - does Chalk have a notation of 'root obligation'?
+                // This is just for diagnostics, so it's okay if this is wrong
+                root_obligation: obligation.clone(),
+            })
+            .collect()
     }
 
     fn select_where_possible(
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
+    ) -> Vec<FulfillmentError<'tcx>> {
         assert!(!infcx.is_in_snapshot());
 
         let mut errors = Vec::new();
@@ -147,7 +145,7 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
             }
         }
 
-        if errors.is_empty() { Ok(()) } else { Err(errors) }
+        errors
     }
 
     fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>> {

compiler/rustc_trait_selection/src/traits/codegen.rs

@@ -120,7 +120,8 @@ where
     // In principle, we only need to do this so long as `result`
     // contains unbound type parameters. It could be a slight
     // optimization to stop iterating early.
-    if let Err(errors) = fulfill_cx.select_all_or_error(infcx) {
+    let errors = fulfill_cx.select_all_or_error(infcx);
+    if !errors.is_empty() {
         infcx.tcx.sess.delay_span_bug(
             rustc_span::DUMMY_SP,
             &format!("Encountered errors `{:?}` resolving bounds after type-checking", errors),

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -126,10 +126,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
     }
 
     /// Attempts to select obligations using `selcx`.
-    fn select(
-        &mut self,
-        selcx: &mut SelectionContext<'a, 'tcx>,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
+    fn select(&mut self, selcx: &mut SelectionContext<'a, 'tcx>) -> Vec<FulfillmentError<'tcx>> {
         let span = debug_span!("select", obligation_forest_size = ?self.predicates.len());
         let _enter = span.enter();
 
@@ -163,7 +160,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
             errors.len()
         );
 
-        if errors.is_empty() { Ok(()) } else { Err(errors) }
+        errors
     }
 }
 
@@ -223,41 +220,36 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
             .register_obligation(PendingPredicateObligation { obligation, stalled_on: vec![] });
     }
 
-    fn select_all_or_error(
-        &mut self,
-        infcx: &InferCtxt<'_, 'tcx>,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
-        self.select_where_possible(infcx)?;
-
-        let errors: Vec<_> = self
-            .predicates
-            .to_errors(CodeAmbiguity)
-            .into_iter()
-            .map(to_fulfillment_error)
-            .collect();
-        if errors.is_empty() { Ok(()) } else { Err(errors) }
+    fn select_all_or_error(&mut self, infcx: &InferCtxt<'_, 'tcx>) -> Vec<FulfillmentError<'tcx>> {
+        {
+            let errors = self.select_where_possible(infcx);
+            if !errors.is_empty() {
+                return errors;
+            }
+        }
+
+        self.predicates.to_errors(CodeAmbiguity).into_iter().map(to_fulfillment_error).collect()
     }
 
     fn select_all_with_constness_or_error(
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
         constness: rustc_hir::Constness,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
-        self.select_with_constness_where_possible(infcx, constness)?;
-
-        let errors: Vec<_> = self
-            .predicates
-            .to_errors(CodeAmbiguity)
-            .into_iter()
-            .map(to_fulfillment_error)
-            .collect();
-        if errors.is_empty() { Ok(()) } else { Err(errors) }
+    ) -> Vec<FulfillmentError<'tcx>> {
+        {
+            let errors = self.select_with_constness_where_possible(infcx, constness);
+            if !errors.is_empty() {
+                return errors;
+            }
+        }
+
+        self.predicates.to_errors(CodeAmbiguity).into_iter().map(to_fulfillment_error).collect()
     }
 
     fn select_where_possible(
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
+    ) -> Vec<FulfillmentError<'tcx>> {
         let mut selcx = SelectionContext::new(infcx);
         self.select(&mut selcx)
     }
@@ -266,7 +258,7 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
         constness: hir::Constness,
-    ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
+    ) -> Vec<FulfillmentError<'tcx>> {
         let mut selcx = SelectionContext::with_constness(infcx, constness);
         self.select(&mut selcx)
     }

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -180,21 +180,21 @@ pub fn type_known_to_meet_bound_modulo_regions<'a, 'tcx>(
         // Note: we only assume something is `Copy` if we can
         // *definitively* show that it implements `Copy`. Otherwise,
         // assume it is move; linear is always ok.
-        match fulfill_cx.select_all_or_error(infcx) {
-            Ok(()) => {
+        match fulfill_cx.select_all_or_error(infcx).as_slice() {
+            [] => {
                 debug!(
                     "type_known_to_meet_bound_modulo_regions: ty={:?} bound={} success",
                     ty,
                     infcx.tcx.def_path_str(def_id)
                 );
                 true
             }
-            Err(e) => {
+            errors => {
                 debug!(
-                    "type_known_to_meet_bound_modulo_regions: ty={:?} bound={} errors={:?}",
-                    ty,
-                    infcx.tcx.def_path_str(def_id),
-                    e
+                    ?ty,
+                    bound = %infcx.tcx.def_path_str(def_id),
+                    ?errors,
+                    "type_known_to_meet_bound_modulo_regions"
                 );
                 false
             }
@@ -410,7 +410,10 @@ where
     }
 
     debug!("fully_normalize: select_all_or_error start");
-    fulfill_cx.select_all_or_error(infcx)?;
+    let errors = fulfill_cx.select_all_or_error(infcx);
+    if !errors.is_empty() {
+        return Err(errors);
+    }
     debug!("fully_normalize: select_all_or_error complete");
     let resolved_value = infcx.resolve_vars_if_possible(normalized_value);
     debug!("fully_normalize: resolved_value={:?}", resolved_value);
@@ -441,7 +444,9 @@ pub fn impossible_predicates<'tcx>(
             fulfill_cx.register_predicate_obligation(&infcx, obligation);
         }
 
-        fulfill_cx.select_all_or_error(&infcx).is_err()
+        let errors = fulfill_cx.select_all_or_error(&infcx);
+
+        !errors.is_empty()
     });
     debug!("impossible_predicates = {:?}", result);
     result