Auto merge of #110100 - compiler-errors:no-infer-pred-must-hold, r=jackh726

bors · bors · commit 19ca5692f69d · 2023-05-19T03:36:37.000Z
do not allow inference in `predicate_must_hold` (alternative approach) See the FCP description for more info, but tl;dr is that we should not return `EvaluatedToOkModuloRegions` if an obligation may hold only with some choice of inference vars being constrained. Attempts to solve this in the approach laid out by lcnr here: #109558 (comment), rather than by eagerly replacing infer vars with placeholders which is a bit too restrictive. r? `@ghost`
diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs
@@ -143,35 +143,36 @@ pub fn type_known_to_meet_bound_modulo_regions<'tcx>(
 fn pred_known_to_hold_modulo_regions<'tcx>(
     infcx: &InferCtxt<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
-    pred: impl ToPredicate<'tcx> + TypeVisitable<TyCtxt<'tcx>>,
+    pred: impl ToPredicate<'tcx>,
 ) -> bool {
-    let has_non_region_infer = pred.has_non_region_infer();
     let obligation = Obligation::new(infcx.tcx, ObligationCause::dummy(), param_env, pred);
 
     let result = infcx.evaluate_obligation_no_overflow(&obligation);
     debug!(?result);
 
-    if result.must_apply_modulo_regions() && !has_non_region_infer {
+    if result.must_apply_modulo_regions() {
         true
     } else if result.may_apply() {
-        // Because of inference "guessing", selection can sometimes claim
-        // to succeed while the success requires a guess. To ensure
-        // this function's result remains infallible, we must confirm
-        // that guess. While imperfect, I believe this is sound.
-
-        // The handling of regions in this area of the code is terrible,
-        // see issue #29149. We should be able to improve on this with
-        // NLL.
-        let ocx = ObligationCtxt::new(infcx);
-        ocx.register_obligation(obligation);
-        let errors = ocx.select_all_or_error();
-        match errors.as_slice() {
-            [] => true,
-            errors => {
-                debug!(?errors);
-                false
+        // Sometimes obligations are ambiguous because the recursive evaluator
+        // is not smart enough, so we fall back to fulfillment when we're not certain
+        // that an obligation holds or not. Even still, we must make sure that
+        // the we do no inference in the process of checking this obligation.
+        let goal = infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env));
+        infcx.probe(|_| {
+            let ocx = ObligationCtxt::new_in_snapshot(infcx);
+            ocx.register_obligation(obligation);
+
+            let errors = ocx.select_all_or_error();
+            match errors.as_slice() {
+                // Only known to hold if we did no inference.
+                [] => infcx.shallow_resolve(goal) == goal,
+
+                errors => {
+                    debug!(?errors);
+                    false
+                }
             }
-        }
+        })
     } else {
         false
     }
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -537,14 +537,22 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
     ) -> Result<EvaluationResult, OverflowError> {
         self.evaluation_probe(|this| {
-            if this.tcx().trait_solver_next() {
-                this.evaluate_predicates_recursively_in_new_solver([obligation.clone()])
+            let goal =
+                this.infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env));
+            let mut result = if this.tcx().trait_solver_next() {
+                this.evaluate_predicates_recursively_in_new_solver([obligation.clone()])?
             } else {
                 this.evaluate_predicate_recursively(
                     TraitObligationStackList::empty(&ProvisionalEvaluationCache::default()),
                     obligation.clone(),
-                )
+                )?
+            };
+            // If the predicate has done any inference, then downgrade the
+            // result to ambiguous.
+            if this.infcx.shallow_resolve(goal) != goal {
+                result = result.max(EvaluatedToAmbig);
             }
+            Ok(result)
         })
     }
 
diff --git a/tests/ui/traits/copy-guessing.rs b/tests/ui/traits/copy-guessing.rs
@@ -1,5 +1,3 @@
-// run-pass
-
 #![allow(dead_code)]
 #![allow(drop_copy)]
 
@@ -20,6 +18,7 @@ fn assert_impls_fn<R,T: Fn()->R>(_: &T){}
 
 fn main() {
     let n = None;
+    //~^ ERROR type annotations needed for `Option<T>`
     let e = S(&n);
     let f = || {
         // S being copy is critical for this to work
diff --git a/tests/ui/traits/copy-guessing.stderr b/tests/ui/traits/copy-guessing.stderr
@@ -0,0 +1,14 @@
+error[E0282]: type annotations needed for `Option<T>`
+  --> $DIR/copy-guessing.rs:20:9
+   |
+LL |     let n = None;
+   |         ^
+   |
+help: consider giving `n` an explicit type, where the type for type parameter `T` is specified
+   |
+LL |     let n: Option<T> = None;
+   |          +++++++++++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0282`.
