do not allow inference in predicate_must_hold

compiler/rustc_infer/src/infer/mod.rs

@@ -1603,6 +1603,73 @@ impl<'tcx> InferCtxt<'tcx> {
         }
     }
 
+    /// Replaces substs that reference param or infer variables with suitable
+    /// placeholders. This function is meant to remove these param and infer
+    /// substs when they're not actually needed to evaluate a constant.
+    ///
+    /// This loses information about inference variables so it should only be used
+    /// if incorrectly failing to prove or equate something is acceptable.
+    pub fn replace_infer_vars_with_placeholders<T: TypeFoldable<TyCtxt<'tcx>>>(
+        &self,
+        value: T,
+    ) -> T {
+        struct ReplaceInferVarsWithPlaceholders<'a, 'tcx> {
+            infcx: &'a InferCtxt<'tcx>,
+            map: FxHashMap<ty::GenericArg<'tcx>, u32>,
+        }
+
+        impl<'a, 'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceInferVarsWithPlaceholders<'a, 'tcx> {
+            fn interner(&self) -> TyCtxt<'tcx> {
+                self.infcx.tcx
+            }
+
+            fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
+                // This replaces integer and float inference vars with placeholders, losing
+                // information.
+                if let ty::Infer(_) = t.kind() {
+                    let new_idx = self.map.len() as u32;
+                    let idx = *self.map.entry(t.into()).or_insert(new_idx);
+                    self.interner().mk_placeholder(ty::PlaceholderType {
+                        universe: ty::UniverseIndex::ROOT,
+                        name: ty::BoundTyKind::Anon(idx),
+                    })
+                } else {
+                    t.super_fold_with(self)
+                }
+            }
+
+            fn fold_const(&mut self, c: ty::Const<'tcx>) -> ty::Const<'tcx> {
+                if let ty::ConstKind::Infer(_) = c.kind() {
+                    let ty = c.ty();
+                    if !ty.is_global() {
+                        bug!("const `{c}`'s type should not reference params or types");
+                    }
+                    let new_idx = self.map.len() as u32;
+                    let idx = *self.map.entry(c.into()).or_insert(new_idx);
+                    self.interner().mk_const(
+                        ty::PlaceholderConst {
+                            universe: ty::UniverseIndex::ROOT,
+                            name: ty::BoundVar::from_u32(idx),
+                        },
+                        ty,
+                    )
+                } else {
+                    c.super_fold_with(self)
+                }
+            }
+        }
+
+        let value = self.resolve_vars_if_possible(value);
+        if value.has_non_region_infer() {
+            value.fold_with(&mut ReplaceInferVarsWithPlaceholders {
+                infcx: self,
+                map: Default::default(),
+            })
+        } else {
+            value
+        }
+    }
+
     /// Resolves and evaluates a constant.
     ///
     /// The constant can be located on a trait like `<A as B>::C`, in which case the given
@@ -1636,7 +1703,7 @@ impl<'tcx> InferCtxt<'tcx> {
                 } else if ct.has_non_region_infer() || ct.has_non_region_param() {
                     return Err(ErrorHandled::TooGeneric);
                 } else {
-                    substs = replace_param_and_infer_substs_with_placeholder(tcx, substs);
+                    substs = self.replace_infer_vars_with_placeholders(substs);
                 }
             } else {
                 substs = InternalSubsts::identity_for_item(tcx, unevaluated.def.did);
@@ -2087,62 +2154,3 @@ impl RegionVariableOrigin {
         }
     }
 }
-
-/// Replaces substs that reference param or infer variables with suitable
-/// placeholders. This function is meant to remove these param and infer
-/// substs when they're not actually needed to evaluate a constant.
-fn replace_param_and_infer_substs_with_placeholder<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    substs: SubstsRef<'tcx>,
-) -> SubstsRef<'tcx> {
-    struct ReplaceParamAndInferWithPlaceholder<'tcx> {
-        tcx: TyCtxt<'tcx>,
-        idx: u32,
-    }
-
-    impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceParamAndInferWithPlaceholder<'tcx> {
-        fn interner(&self) -> TyCtxt<'tcx> {
-            self.tcx
-        }
-
-        fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
-            if let ty::Infer(_) = t.kind() {
-                self.tcx.mk_placeholder(ty::PlaceholderType {
-                    universe: ty::UniverseIndex::ROOT,
-                    name: ty::BoundTyKind::Anon({
-                        let idx = self.idx;
-                        self.idx += 1;
-                        idx
-                    }),
-                })
-            } else {
-                t.super_fold_with(self)
-            }
-        }
-
-        fn fold_const(&mut self, c: ty::Const<'tcx>) -> ty::Const<'tcx> {
-            if let ty::ConstKind::Infer(_) = c.kind() {
-                let ty = c.ty();
-                // If the type references param or infer then ICE ICE ICE
-                if ty.has_non_region_param() || ty.has_non_region_infer() {
-                    bug!("const `{c}`'s type should not reference params or types");
-                }
-                self.tcx.mk_const(
-                    ty::PlaceholderConst {
-                        universe: ty::UniverseIndex::ROOT,
-                        name: ty::BoundVar::from_u32({
-                            let idx = self.idx;
-                            self.idx += 1;
-                            idx
-                        }),
-                    },
-                    ty,
-                )
-            } else {
-                c.super_fold_with(self)
-            }
-        }
-    }
-
-    substs.fold_with(&mut ReplaceParamAndInferWithPlaceholder { tcx, idx: 0 })
-}

compiler/rustc_middle/src/ty/sty.rs

@@ -2339,9 +2339,7 @@ impl<'tcx> Ty<'tcx> {
                 _ => bug!("cannot convert type `{:?}` to a closure kind", self),
             },
 
-            // "Bound" types appear in canonical queries when the
-            // closure type is not yet known
-            Bound(..) | Infer(_) => None,
+            Infer(_) | Placeholder(_) => None,
 
             Error(_) => Some(ty::ClosureKind::Fn),
 

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -119,61 +119,19 @@ pub fn predicates_for_generics<'tcx>(
     })
 }
 
-/// Determines whether the type `ty` is known to meet `bound` and
-/// returns true if so. Returns false if `ty` either does not meet
-/// `bound` or is not known to meet bound (note that this is
-/// conservative towards *no impl*, which is the opposite of the
-/// `evaluate` methods).
+/// Returns whether the type `ty` is known to meet `bound`.
 pub fn type_known_to_meet_bound_modulo_regions<'tcx>(
     infcx: &InferCtxt<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     ty: Ty<'tcx>,
     def_id: DefId,
 ) -> bool {
-    let trait_ref = ty::Binder::dummy(infcx.tcx.mk_trait_ref(def_id, [ty]));
-    pred_known_to_hold_modulo_regions(infcx, param_env, trait_ref.without_const())
-}
-
-/// FIXME(@lcnr): this function doesn't seem right and shouldn't exist?
-///
-/// Ping me on zulip if you want to use this method and need help with finding
-/// an appropriate replacement.
-#[instrument(level = "debug", skip(infcx, param_env, pred), ret)]
-fn pred_known_to_hold_modulo_regions<'tcx>(
-    infcx: &InferCtxt<'tcx>,
-    param_env: ty::ParamEnv<'tcx>,
-    pred: impl ToPredicate<'tcx> + TypeVisitable<TyCtxt<'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 {
-        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
-            }
-        }
-    } else {
-        false
-    }
+    infcx.predicate_must_hold_modulo_regions(&Obligation::new(
+        infcx.tcx,
+        ObligationCause::dummy(),
+        param_env,
+        infcx.tcx.mk_trait_ref(def_id, [ty]),
+    ))
 }
 
 #[instrument(level = "debug", skip(tcx, elaborated_env))]

compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs

@@ -39,25 +39,27 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
     }
 
     /// Evaluates whether the predicate can be satisfied in the given
-    /// `ParamEnv`, and returns `false` if not certain. However, this is
-    /// not entirely accurate if inference variables are involved.
+    /// `ParamEnv`, and returns `false` if not certain. If this returns
+    /// true then the given `obligation` is guaranteed to hold.
     ///
     /// This version may conservatively fail when outlives obligations
     /// are required.
     fn predicate_must_hold_considering_regions(
         &self,
         obligation: &PredicateObligation<'tcx>,
     ) -> bool {
-        self.evaluate_obligation_no_overflow(obligation).must_apply_considering_regions()
+        let obligation = obligation
+            .with(self.tcx, self.replace_infer_vars_with_placeholders(obligation.predicate));
+        self.evaluate_obligation_no_overflow(&obligation).must_apply_considering_regions()
     }
 
     /// Evaluates whether the predicate can be satisfied in the given
-    /// `ParamEnv`, and returns `false` if not certain. However, this is
-    /// not entirely accurate if inference variables are involved.
-    ///
-    /// This version ignores all outlives constraints.
+    /// `ParamEnv`. If this returns true then the given `obligation` is
+    /// guaranteed to hold except for region constraints.
     fn predicate_must_hold_modulo_regions(&self, obligation: &PredicateObligation<'tcx>) -> bool {
-        self.evaluate_obligation_no_overflow(obligation).must_apply_modulo_regions()
+        let obligation = obligation
+            .with(self.tcx, self.replace_infer_vars_with_placeholders(obligation.predicate));
+        self.evaluate_obligation_no_overflow(&obligation).must_apply_modulo_regions()
     }
 
     /// Evaluate a given predicate, capturing overflow and propagating it back.

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

@@ -2250,11 +2250,12 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             ty::Closure(_, substs) => {
                 // (*) binder moved here
                 let ty = self.infcx.shallow_resolve(substs.as_closure().tupled_upvars_ty());
-                if let ty::Infer(ty::TyVar(_)) = ty.kind() {
-                    // Not yet resolved.
-                    Ambiguous
-                } else {
-                    Where(obligation.predicate.rebind(substs.as_closure().upvar_tys().collect()))
+                match ty.kind() {
+                    ty::Infer(ty::TyVar(_)) => Ambiguous,
+                    ty::Placeholder(_) => None,
+                    _ => Where(
+                        obligation.predicate.rebind(substs.as_closure().upvar_tys().collect()),
+                    ),
                 }
             }
 

src/tools/clippy/tests/ui/crashes/ice-6250.stderr

@@ -12,11 +12,6 @@ LL |     for reference in vec![1, 2, 3] {
 ...
 LL |         Some(reference) = cache.data.get(key) {
    |              ^^^^^^^^^ expected integer, found `&i32`
-   |
-help: consider dereferencing the borrow
-   |
-LL |         Some(*reference) = cache.data.get(key) {
-   |              +
 
 error[E0308]: mismatched types
   --> $DIR/ice-6250.rs:12:9

tests/ui/suggestions/into-convert.rs

@@ -13,7 +13,6 @@ fn main() {
 
     let z: AtomicU32 = 1;
     //~^ ERROR mismatched types
-    //~| HELP call `Into::into` on this expression to convert `{integer}` into `AtomicU32`
 }
 
 struct A;

tests/ui/suggestions/into-convert.stderr

@@ -33,11 +33,6 @@ LL |     let z: AtomicU32 = 1;
    |            ---------   ^ expected `AtomicU32`, found integer
    |            |
    |            expected due to this
-   |
-help: call `Into::into` on this expression to convert `{integer}` into `AtomicU32`
-   |
-LL |     let z: AtomicU32 = 1.into();
-   |                         +++++++
 
 error: aborting due to 3 previous errors
 

tests/ui/traits/copy-guessing.rs

@@ -1,4 +1,3 @@
-// run-pass
 #![allow(dead_code)]
 // "guessing" in trait selection can affect `copy_or_move`. Check that this
 // is correctly handled. I am not sure what is the "correct" behaviour,
@@ -10,15 +9,18 @@ struct U([u8; 1337]);
 
 struct S<'a,T:'a>(&'a T);
 impl<'a, T> Clone for S<'a, T> { fn clone(&self) -> Self { S(self.0) } }
-/// This impl triggers inference "guessing" - S<_>: Copy => _ = U
+/// This impl could trigger inference "guessing" - S<_>: Copy => _ = U
+///
+/// The current behavior is that due to inference fallback we end up with
+/// `S<Option<()>>` instead.
 impl<'a> Copy for S<'a, Option<U>> {}
 
 fn assert_impls_fn<R,T: Fn()->R>(_: &T){}
 
 fn main() {
     let n = None;
     let e = S(&n);
-    let f = || {
+    let f = || { //~ ERROR expected a closure that implements the `Fn` trait
         // S being copy is critical for this to work
         drop(e);
         mem::size_of_val(e.0)

tests/ui/traits/copy-guessing.stderr

@@ -0,0 +1,23 @@
+error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
+  --> $DIR/copy-guessing.rs:23:13
+   |
+LL |     let f = || {
+   |             ^^ this closure implements `FnOnce`, not `Fn`
+LL |         // S being copy is critical for this to work
+LL |         drop(e);
+   |              - closure is `FnOnce` because it moves the variable `e` out of its environment
+...
+LL |     assert_impls_fn(&f);
+   |     --------------- -- the requirement to implement `Fn` derives from here
+   |     |
+   |     required by a bound introduced by this call
+   |
+note: required by a bound in `assert_impls_fn`
+  --> $DIR/copy-guessing.rs:18:25
+   |
+LL | fn assert_impls_fn<R,T: Fn()->R>(_: &T){}
+   |                         ^^^^^^^ required by this bound in `assert_impls_fn`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0525`.