Auto merge of #93652 - spastorino:fix-negative-overlap-check-regions, r=nikomatsakis

Fix negative overlap check regions

r? `@nikomatsakis`

compiler/rustc_data_structures/src/snapshot_map/mod.rs

@@ -13,6 +13,7 @@ mod tests;
 pub type SnapshotMapStorage<K, V> = SnapshotMap<K, V, FxHashMap<K, V>, ()>;
 pub type SnapshotMapRef<'a, K, V, L> = SnapshotMap<K, V, &'a mut FxHashMap<K, V>, &'a mut L>;
 
+#[derive(Clone)]
 pub struct SnapshotMap<K, V, M = FxHashMap<K, V>, L = VecLog<UndoLog<K, V>>> {
     map: M,
     undo_log: L,
@@ -30,6 +31,7 @@ where
     }
 }
 
+#[derive(Clone)]
 pub enum UndoLog<K, V> {
     Inserted(K),
     Overwrite(K, V),

compiler/rustc_infer/src/infer/at.rs

@@ -51,6 +51,28 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     ) -> At<'a, 'tcx> {
         At { infcx: self, cause, param_env }
     }
+
+    /// Forks the inference context, creating a new inference context with the same inference
+    /// variables in the same state. This can be used to "branch off" many tests from the same
+    /// common state. Used in coherence.
+    pub fn fork(&self) -> Self {
+        Self {
+            tcx: self.tcx.clone(),
+            defining_use_anchor: self.defining_use_anchor.clone(),
+            in_progress_typeck_results: self.in_progress_typeck_results.clone(),
+            inner: self.inner.clone(),
+            skip_leak_check: self.skip_leak_check.clone(),
+            lexical_region_resolutions: self.lexical_region_resolutions.clone(),
+            selection_cache: self.selection_cache.clone(),
+            evaluation_cache: self.evaluation_cache.clone(),
+            reported_trait_errors: self.reported_trait_errors.clone(),
+            reported_closure_mismatch: self.reported_closure_mismatch.clone(),
+            tainted_by_errors_flag: self.tainted_by_errors_flag.clone(),
+            err_count_on_creation: self.err_count_on_creation,
+            in_snapshot: self.in_snapshot.clone(),
+            universe: self.universe.clone(),
+        }
+    }
 }
 
 pub trait ToTrace<'tcx>: Relate<'tcx> + Copy {

compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs

@@ -61,6 +61,7 @@ pub(crate) fn resolve<'tcx>(
 
 /// Contains the result of lexical region resolution. Offers methods
 /// to lookup up the final value of a region variable.
+#[derive(Clone)]
 pub struct LexicalRegionResolutions<'tcx> {
     values: IndexVec<RegionVid, VarValue<'tcx>>,
     error_region: ty::Region<'tcx>,

compiler/rustc_infer/src/infer/mod.rs

@@ -130,6 +130,7 @@ impl RegionckMode {
 /// `RefCell` and are involved with taking/rolling back snapshots. Snapshot
 /// operations are hot enough that we want only one call to `borrow_mut` per
 /// call to `start_snapshot` and `rollback_to`.
+#[derive(Clone)]
 pub struct InferCtxtInner<'tcx> {
     /// Cache for projections. This cache is snapshotted along with the infcx.
     ///

compiler/rustc_infer/src/infer/region_constraints/mod.rs

@@ -28,7 +28,7 @@ mod leak_check;
 
 pub use rustc_middle::infer::MemberConstraint;
 
-#[derive(Default)]
+#[derive(Clone, Default)]
 pub struct RegionConstraintStorage<'tcx> {
     /// For each `RegionVid`, the corresponding `RegionVariableOrigin`.
     var_infos: IndexVec<RegionVid, RegionVariableInfo>,

compiler/rustc_infer/src/infer/type_variable.rs

@@ -14,6 +14,7 @@ use std::ops::Range;
 use rustc_data_structures::undo_log::{Rollback, UndoLogs};
 
 /// Represents a single undo-able action that affects a type inference variable.
+#[derive(Clone)]
 pub(crate) enum UndoLog<'tcx> {
     EqRelation(sv::UndoLog<ut::Delegate<TyVidEqKey<'tcx>>>),
     SubRelation(sv::UndoLog<ut::Delegate<ty::TyVid>>),
@@ -58,6 +59,7 @@ impl<'tcx> Rollback<UndoLog<'tcx>> for TypeVariableStorage<'tcx> {
     }
 }
 
+#[derive(Clone)]
 pub struct TypeVariableStorage<'tcx> {
     values: sv::SnapshotVecStorage<Delegate>,
 
@@ -137,6 +139,7 @@ pub enum TypeVariableOriginKind {
     LatticeVariable,
 }
 
+#[derive(Clone)]
 pub(crate) struct TypeVariableData {
     origin: TypeVariableOrigin,
 }
@@ -165,6 +168,7 @@ impl<'tcx> TypeVariableValue<'tcx> {
     }
 }
 
+#[derive(Clone)]
 pub(crate) struct Instantiate;
 
 pub(crate) struct Delegate;

compiler/rustc_infer/src/infer/undo_log.rs

@@ -17,6 +17,7 @@ pub struct Snapshot<'tcx> {
 }
 
 /// Records the "undo" data for a single operation that affects some form of inference variable.
+#[derive(Clone)]
 pub(crate) enum UndoLog<'tcx> {
     TypeVariables(type_variable::UndoLog<'tcx>),
     ConstUnificationTable(sv::UndoLog<ut::Delegate<ty::ConstVid<'tcx>>>),
@@ -84,6 +85,7 @@ impl<'tcx> Rollback<UndoLog<'tcx>> for InferCtxtInner<'tcx> {
 
 /// The combined undo log for all the various unification tables. For each change to the storage
 /// for any kind of inference variable, we record an UndoLog entry in the vector here.
+#[derive(Clone)]
 pub(crate) struct InferCtxtUndoLogs<'tcx> {
     logs: Vec<UndoLog<'tcx>>,
     num_open_snapshots: usize,

compiler/rustc_infer/src/traits/project.rs

@@ -70,7 +70,7 @@ pub struct ProjectionCache<'a, 'tcx> {
     undo_log: &'a mut InferCtxtUndoLogs<'tcx>,
 }
 
-#[derive(Default)]
+#[derive(Clone, Default)]
 pub struct ProjectionCacheStorage<'tcx> {
     map: SnapshotMapStorage<ProjectionCacheKey<'tcx>, ProjectionCacheEntry<'tcx>>,
 }

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -4,8 +4,8 @@
 //! [trait-resolution]: https://rustc-dev-guide.rust-lang.org/traits/resolution.html
 //! [trait-specialization]: https://rustc-dev-guide.rust-lang.org/traits/specialization.html
 
-use crate::infer::{CombinedSnapshot, InferOk, TyCtxtInferExt};
+use crate::infer::outlives::env::OutlivesEnvironment;
-use crate::traits::query::evaluate_obligation::InferCtxtExt;
+use crate::infer::{CombinedSnapshot, InferOk, RegionckMode};
 use crate::traits::select::IntercrateAmbiguityCause;
 use crate::traits::util::impl_trait_ref_and_oblig;
 use crate::traits::SkipLeakCheck;
@@ -13,7 +13,11 @@ use crate::traits::{
     self, FulfillmentContext, Normalized, Obligation, ObligationCause, PredicateObligation,
     PredicateObligations, SelectionContext,
 };
+//use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_hir::CRATE_HIR_ID;
+use rustc_infer::infer::TyCtxtInferExt;
+use rustc_infer::traits::TraitEngine;
 use rustc_middle::traits::specialization_graph::OverlapMode;
 use rustc_middle::ty::fast_reject::{self, SimplifyParams};
 use rustc_middle::ty::fold::TypeFoldable;
@@ -150,7 +154,10 @@ fn overlap<'cx, 'tcx>(
     impl2_def_id: DefId,
     overlap_mode: OverlapMode,
 ) -> Option<OverlapResult<'tcx>> {
-    debug!("overlap(impl1_def_id={:?}, impl2_def_id={:?})", impl1_def_id, impl2_def_id);
+    debug!(
+        "overlap(impl1_def_id={:?}, impl2_def_id={:?}, overlap_mode={:?})",
+        impl1_def_id, impl2_def_id, overlap_mode
+    );
 
     selcx.infcx().probe_maybe_skip_leak_check(skip_leak_check.is_yes(), |snapshot| {
         overlap_within_probe(
@@ -191,9 +198,6 @@ fn overlap_within_probe<'cx, 'tcx>(
     let impl1_header = with_fresh_ty_vars(selcx, param_env, impl1_def_id);
     let impl2_header = with_fresh_ty_vars(selcx, param_env, impl2_def_id);
 
-    debug!("overlap: impl1_header={:?}", impl1_header);
-    debug!("overlap: impl2_header={:?}", impl2_header);
-
     let obligations = equate_impl_headers(selcx, &impl1_header, &impl2_header)?;
     debug!("overlap: unification check succeeded");
 
@@ -226,6 +230,7 @@ fn equate_impl_headers<'cx, 'tcx>(
     impl2_header: &ty::ImplHeader<'tcx>,
 ) -> Option<PredicateObligations<'tcx>> {
     // Do `a` and `b` unify? If not, no overlap.
+    debug!("equate_impl_headers(impl1_header={:?}, impl2_header={:?}", impl1_header, impl2_header);
     selcx
         .infcx()
         .at(&ObligationCause::dummy(), ty::ParamEnv::empty())
@@ -264,8 +269,11 @@ fn implicit_negative<'cx, 'tcx>(
     // If the obligation `&'?a str: Error` holds, it means that there's overlap. If that doesn't
     // hold we need to check if `&'?a str: !Error` holds, if doesn't hold there's overlap because
     // at some point an impl for `&'?a str: Error` could be added.
+    debug!(
+        "implicit_negative(impl1_header={:?}, impl2_header={:?}, obligations={:?})",
+        impl1_header, impl2_header, obligations
+    );
     let infcx = selcx.infcx();
-    let tcx = infcx.tcx;
     let opt_failing_obligation = impl1_header
         .predicates
         .iter()
@@ -279,12 +287,7 @@ fn implicit_negative<'cx, 'tcx>(
             predicate: p,
         })
         .chain(obligations)
-        .find(|o| {
+        .find(|o| !selcx.predicate_may_hold_fatal(o));
-            loose_check(selcx, o) || tcx.features().negative_impls && negative_impl_exists(selcx, o)
-        });
-    // FIXME: the call to `selcx.predicate_may_hold_fatal` above should be ported
-    // to the canonical trait query form, `infcx.predicate_may_hold`, once
-    // the new system supports intercrate mode (which coherence needs).
 
     if let Some(failing_obligation) = opt_failing_obligation {
         debug!("overlap: obligation unsatisfiable {:?}", failing_obligation);
@@ -301,6 +304,7 @@ fn negative_impl<'cx, 'tcx>(
     impl1_def_id: DefId,
     impl2_def_id: DefId,
 ) -> bool {
+    debug!("negative_impl(impl1_def_id={:?}, impl2_def_id={:?})", impl1_def_id, impl2_def_id);
     let tcx = selcx.infcx().tcx;
 
     // create a parameter environment corresponding to a (placeholder) instantiation of impl1
@@ -348,7 +352,7 @@ fn negative_impl<'cx, 'tcx>(
         let opt_failing_obligation = obligations
             .into_iter()
             .chain(more_obligations)
-            .find(|o| negative_impl_exists(selcx, o));
+            .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o));
 
         if let Some(failing_obligation) = opt_failing_obligation {
             debug!("overlap: obligation unsatisfiable {:?}", failing_obligation);
@@ -359,24 +363,47 @@ fn negative_impl<'cx, 'tcx>(
     })
 }
 
-fn loose_check<'cx, 'tcx>(
-    selcx: &mut SelectionContext<'cx, 'tcx>,
-    o: &PredicateObligation<'tcx>,
-) -> bool {
-    !selcx.predicate_may_hold_fatal(o)
-}
-
 fn negative_impl_exists<'cx, 'tcx>(
     selcx: &SelectionContext<'cx, 'tcx>,
+    param_env: ty::ParamEnv<'tcx>,
+    region_context: DefId,
     o: &PredicateObligation<'tcx>,
 ) -> bool {
-    let infcx = selcx.infcx();
+    let infcx = &selcx.infcx().fork();
     let tcx = infcx.tcx;
     o.flip_polarity(tcx)
-        .as_ref()
         .map(|o| {
-            // FIXME This isn't quite correct, regions should be included
+            let mut fulfillment_cx = FulfillmentContext::new();
-            selcx.infcx().predicate_must_hold_modulo_regions(o)
+            fulfillment_cx.register_predicate_obligation(infcx, o);
+
+            let errors = fulfillment_cx.select_all_or_error(infcx);
+            if !errors.is_empty() {
+                return false;
+            }
+
+            let mut outlives_env = OutlivesEnvironment::new(param_env);
+            // FIXME -- add "assumed to be well formed" types into the `outlives_env`
+
+            // "Save" the accumulated implied bounds into the outlives environment
+            // (due to the FIXME above, there aren't any, but this step is still needed).
+            // The "body id" is given as `CRATE_HIR_ID`, which is the same body-id used
+            // by the "dummy" causes elsewhere (body-id is only relevant when checking
+            // function bodies with closures).
+            outlives_env.save_implied_bounds(CRATE_HIR_ID);
+
+            infcx.process_registered_region_obligations(
+                outlives_env.region_bound_pairs_map(),
+                Some(tcx.lifetimes.re_root_empty),
+                param_env,
+            );
+
+            let errors =
+                infcx.resolve_regions(region_context, &outlives_env, RegionckMode::default());
+            if !errors.is_empty() {
+                return false;
+            }
+
+            true
         })
         .unwrap_or(false)
 }

src/test/ui/coherence/auxiliary/error_lib.rs

@@ -1,5 +1,6 @@
 #![crate_type = "lib"]
 #![feature(negative_impls)]
+#![feature(with_negative_coherence)]
 
 pub trait Error {}
 impl !Error for &str {}

src/test/ui/coherence/coherence-negative-outlives-lifetimes.rs

@@ -0,0 +1,12 @@
+#![feature(negative_impls)]
+
+// FIXME: this should compile
+
+trait MyPredicate<'a> {}
+impl<'a, T> !MyPredicate<'a> for &T where T: 'a {}
+trait MyTrait<'a> {}
+impl<'a, T: MyPredicate<'a>> MyTrait<'a> for T {}
+impl<'a, T> MyTrait<'a> for &'a T {}
+//~^ ERROR: conflicting implementations of trait `MyTrait<'_>` for type `&_`
+
+fn main() {}

src/test/ui/coherence/coherence-negative-outlives-lifetimes.stderr

@@ -0,0 +1,11 @@
+error[E0119]: conflicting implementations of trait `MyTrait<'_>` for type `&_`
+  --> $DIR/coherence-negative-outlives-lifetimes.rs:9:1
+   |
+LL | impl<'a, T: MyPredicate<'a>> MyTrait<'a> for T {}
+   | ---------------------------------------------- first implementation here
+LL | impl<'a, T> MyTrait<'a> for &'a T {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `&_`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0119`.

src/test/ui/coherence/coherence-overlap-negate-use-feature-gate.rs

@@ -1,6 +1,6 @@
 // check-pass
 
-#![feature(negative_impls)]
+#![feature(with_negative_coherence)]
 
 use std::ops::DerefMut;
 

src/test/ui/coherence/coherence-overlap-negative-trait.rs

@@ -3,7 +3,7 @@
 //
 // Check that if we promise to not impl what would overlap it doesn't actually overlap
 
-#![feature(negative_impls)]
+#![feature(with_negative_coherence)]
 
 extern crate error_lib as lib;
 use lib::Error;

src/test/ui/coherence/coherence-overlap-with-regions.rs

@@ -0,0 +1,16 @@
+// check-pass
+
+#![feature(negative_impls)]
+#![feature(rustc_attrs)]
+#![feature(with_negative_coherence)]
+
+#[rustc_strict_coherence]
+trait Foo {}
+impl<T> !Foo for &T where T: 'static {}
+
+#[rustc_strict_coherence]
+trait Bar {}
+impl<T: Foo> Bar for T {}
+impl<T> Bar for &T where T: 'static {}
+
+fn main() {}

src/test/ui/traits/negative-impls/auxiliary/foreign_trait.rs

@@ -1,4 +1,5 @@
 #![feature(negative_impls)]
+#![feature(with_negative_coherence)]
 
 pub trait ForeignTrait {}
 

src/test/ui/traits/negative-impls/rely-on-negative-impl-in-coherence.rs

@@ -1,6 +1,7 @@
 // check-pass
 
 #![feature(negative_impls)]
+#![feature(with_negative_coherence)]
 
 // aux-build: foreign_trait.rs
 
@@ -16,8 +17,8 @@
 extern crate foreign_trait;
 use foreign_trait::ForeignTrait;
 
-trait LocalTrait { }
+trait LocalTrait {}
-impl<T: ForeignTrait> LocalTrait for T { }
+impl<T: ForeignTrait> LocalTrait for T {}
-impl LocalTrait for String { }
+impl LocalTrait for String {}
 
-fn main() { }
+fn main() {}