From 228441dbd6e9fc823da545ed29499f6f8e5d7101 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 16 Feb 2024 17:56:15 +0000 Subject: [PATCH] Use fulfillment in next trait solver coherence --- .../src/solve/inspect/analyse.rs | 8 ++- .../src/traits/coherence.rs | 70 ++++++++++--------- tests/ui/coherence/coherent-due-to-fulfill.rs | 20 ++++++ .../incoherent-even-though-we-fulfill.rs | 22 ++++++ .../incoherent-even-though-we-fulfill.stderr | 14 ++++ 5 files changed, 97 insertions(+), 37 deletions(-) create mode 100644 tests/ui/coherence/coherent-due-to-fulfill.rs create mode 100644 tests/ui/coherence/incoherent-even-though-we-fulfill.rs create mode 100644 tests/ui/coherence/incoherent-even-though-we-fulfill.stderr diff --git a/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs b/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs index f33d0f397ce9e..9020c11b2559e 100644 --- a/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs +++ b/compiler/rustc_trait_selection/src/solve/inspect/analyse.rs @@ -230,8 +230,10 @@ impl<'tcx> ProofTreeInferCtxtExt<'tcx> for InferCtxt<'tcx> { goal: Goal<'tcx, ty::Predicate<'tcx>>, visitor: &mut V, ) -> ControlFlow { - let (_, proof_tree) = self.evaluate_root_goal(goal, GenerateProofTree::Yes); - let proof_tree = proof_tree.unwrap(); - visitor.visit_goal(&InspectGoal::new(self, 0, &proof_tree)) + self.probe(|_| { + let (_, proof_tree) = self.evaluate_root_goal(goal, GenerateProofTree::Yes); + let proof_tree = proof_tree.unwrap(); + visitor.visit_goal(&InspectGoal::new(self, 0, &proof_tree)) + }) } } diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index e48bd437f59c5..f663f02f87289 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -8,23 +8,21 @@ use crate::infer::outlives::env::OutlivesEnvironment; use crate::infer::InferOk; use crate::regions::InferCtxtRegionExt; use crate::solve::inspect::{InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor}; -use crate::solve::{deeply_normalize_for_diagnostics, inspect}; -use crate::traits::engine::TraitEngineExt; -use crate::traits::query::evaluate_obligation::InferCtxtExt; +use crate::solve::{deeply_normalize_for_diagnostics, inspect, FulfillmentCtxt}; +use crate::traits::engine::TraitEngineExt as _; use crate::traits::select::IntercrateAmbiguityCause; use crate::traits::structural_normalize::StructurallyNormalizeExt; use crate::traits::NormalizeExt; use crate::traits::SkipLeakCheck; use crate::traits::{ - Obligation, ObligationCause, ObligationCtxt, PredicateObligation, PredicateObligations, - SelectionContext, + Obligation, ObligationCause, PredicateObligation, PredicateObligations, SelectionContext, }; use rustc_data_structures::fx::FxIndexSet; use rustc_errors::Diagnostic; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_infer::infer::{DefineOpaqueTypes, InferCtxt, TyCtxtInferExt}; -use rustc_infer::traits::{util, TraitEngine}; +use rustc_infer::traits::{util, TraitEngine, TraitEngineExt}; use rustc_middle::traits::query::NoSolution; use rustc_middle::traits::solve::{CandidateSource, Certainty, Goal}; use rustc_middle::traits::specialization_graph::OverlapMode; @@ -310,29 +308,35 @@ fn equate_impl_headers<'tcx>( fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>( selcx: &mut SelectionContext<'cx, 'tcx>, obligations: &'a [PredicateObligation<'tcx>], -) -> Option<&'a PredicateObligation<'tcx>> { +) -> Option> { let infcx = selcx.infcx; - obligations.iter().find(|obligation| { - let evaluation_result = if infcx.next_trait_solver() { - infcx.evaluate_obligation(obligation) - } else { + if infcx.next_trait_solver() { + let mut fulfill_cx = FulfillmentCtxt::new(infcx); + fulfill_cx.register_predicate_obligations(infcx, obligations.iter().cloned()); + + // We only care about the obligations that are *definitely* true errors. + // Ambiguities do not prove the disjointness of two impls. + let mut errors = fulfill_cx.select_where_possible(infcx); + errors.pop().map(|err| err.obligation) + } else { + obligations.iter().cloned().find(|obligation| { // We use `evaluate_root_obligation` to correctly track intercrate // ambiguity clauses. We cannot use this in the new solver. - selcx.evaluate_root_obligation(obligation) - }; - - match evaluation_result { - Ok(result) => !result.may_apply(), - // If overflow occurs, we need to conservatively treat the goal as possibly holding, - // since there can be instantiations of this goal that don't overflow and result in - // success. This isn't much of a problem in the old solver, since we treat overflow - // fatally (this still can be encountered: ), - // but in the new solver, this is very important for correctness, since overflow - // *must* be treated as ambiguity for completeness. - Err(_overflow) => false, - } - }) + let evaluation_result = selcx.evaluate_root_obligation(obligation); + + match evaluation_result { + Ok(result) => !result.may_apply(), + // If overflow occurs, we need to conservatively treat the goal as possibly holding, + // since there can be instantiations of this goal that don't overflow and result in + // success. This isn't much of a problem in the old solver, since we treat overflow + // fatally (this still can be encountered: ), + // but in the new solver, this is very important for correctness, since overflow + // *must* be treated as ambiguity for completeness. + Err(_overflow) => false, + } + }) + } } /// Check if both impls can be satisfied by a common type by considering whether @@ -522,15 +526,13 @@ fn try_prove_negated_where_clause<'tcx>( // Without this, we over-eagerly register coherence ambiguity candidates when // impl candidates do exist. let ref infcx = root_infcx.fork_with_intercrate(false); - let ocx = ObligationCtxt::new(infcx); - - ocx.register_obligation(Obligation::new( - infcx.tcx, - ObligationCause::dummy(), - param_env, - negative_predicate, - )); - if !ocx.select_all_or_error().is_empty() { + let mut fulfill_cx = FulfillmentCtxt::new(infcx); + + fulfill_cx.register_predicate_obligation( + infcx, + Obligation::new(infcx.tcx, ObligationCause::dummy(), param_env, negative_predicate), + ); + if !fulfill_cx.select_all_or_error(infcx).is_empty() { return false; } diff --git a/tests/ui/coherence/coherent-due-to-fulfill.rs b/tests/ui/coherence/coherent-due-to-fulfill.rs new file mode 100644 index 0000000000000..084f9be0a8c3f --- /dev/null +++ b/tests/ui/coherence/coherent-due-to-fulfill.rs @@ -0,0 +1,20 @@ +//@ compile-flags: -Znext-solver=coherence +//@ check-pass + +trait Mirror { + type Assoc; +} +impl Mirror for T { + type Assoc = T; +} + +trait Foo {} +trait Bar {} + +// self type starts out as `?0` but is constrained to `()` +// due to the where clause below. Because `(): Bar` does not +// hold in intercrate mode, we can prove the impls disjoint. +impl Foo for T where (): Mirror {} +impl Foo for T where T: Bar {} + +fn main() {} diff --git a/tests/ui/coherence/incoherent-even-though-we-fulfill.rs b/tests/ui/coherence/incoherent-even-though-we-fulfill.rs new file mode 100644 index 0000000000000..b3c9cf328c21c --- /dev/null +++ b/tests/ui/coherence/incoherent-even-though-we-fulfill.rs @@ -0,0 +1,22 @@ +//@ compile-flags: -Znext-solver=coherence + +trait Mirror { + type Assoc; +} +impl Mirror for T { + type Assoc = T; +} + +trait Foo {} + +// Even though using fulfillment in coherence allows us to figure out that +// `?T = ()`, we still treat it as incoherent because `(): Iterator` may be +// added upstream. +impl Foo for T where (): Mirror {} +//~^ NOTE first implementation here +impl Foo for T where T: Iterator {} +//~^ ERROR conflicting implementations of trait `Foo` for type `()` +//~| NOTE conflicting implementation for `()` +//~| NOTE upstream crates may add a new impl of trait `std::iter::Iterator` for type `()` in future versions + +fn main() {} diff --git a/tests/ui/coherence/incoherent-even-though-we-fulfill.stderr b/tests/ui/coherence/incoherent-even-though-we-fulfill.stderr new file mode 100644 index 0000000000000..b16465d201140 --- /dev/null +++ b/tests/ui/coherence/incoherent-even-though-we-fulfill.stderr @@ -0,0 +1,14 @@ +error[E0119]: conflicting implementations of trait `Foo` for type `()` + --> $DIR/incoherent-even-though-we-fulfill.rs:17:1 + | +LL | impl Foo for T where (): Mirror {} + | --------------------------------------------- first implementation here +LL | +LL | impl Foo for T where T: Iterator {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `()` + | + = note: upstream crates may add a new impl of trait `std::iter::Iterator` for type `()` in future versions + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0119`.