From dbbb42442cdb8fdfa610a03355f8614bd1d08ffb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 9 Apr 2023 01:10:47 +0000 Subject: [PATCH 1/2] EvaluateToAmbig if evaluate_root_obligation does inference --- .../rustc_trait_selection/src/traits/select/mod.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index e4f5a84f4244e..065e7b1aee4b3 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -537,14 +537,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { obligation: &PredicateObligation<'tcx>, ) -> Result { self.evaluation_probe(|this| { - if this.tcx().trait_solver_next() { - this.evaluate_predicates_recursively_in_new_solver([obligation.clone()]) + let goal = 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) }) } From c06e61151cbcb89c6dc3df4fa2f0f9eaf914452d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 9 Apr 2023 00:49:50 +0000 Subject: [PATCH 2/2] do not allow inference in `pred_known_to_hold_modulo_regions` --- .../rustc_trait_selection/src/traits/mod.rs | 41 ++++++++++--------- .../src/traits/select/mod.rs | 3 +- tests/ui/traits/copy-guessing.rs | 3 +- tests/ui/traits/copy-guessing.stderr | 14 +++++++ 4 files changed, 38 insertions(+), 23 deletions(-) create mode 100644 tests/ui/traits/copy-guessing.stderr diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index 38daca5377a8b..53398a5971253 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -142,35 +142,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>, + 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 index 065e7b1aee4b3..ed0410178af51 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -537,7 +537,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { obligation: &PredicateObligation<'tcx>, ) -> Result { self.evaluation_probe(|this| { - let goal = infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env)); + 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 { diff --git a/tests/ui/traits/copy-guessing.rs b/tests/ui/traits/copy-guessing.rs index 558303c2e40bc..c1ed4c28a0300 100644 --- 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_fnR>(_: &T){} fn main() { let n = None; + //~^ ERROR type annotations needed for `Option` 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 new file mode 100644 index 0000000000000..568b7e5a64af9 --- /dev/null +++ b/tests/ui/traits/copy-guessing.stderr @@ -0,0 +1,14 @@ +error[E0282]: type annotations needed for `Option` + --> $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 = None; + | +++++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`.