Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix negative overlap check regions #93652

Merged
merged 9 commits into from
Feb 14, 2022
2 changes: 2 additions & 0 deletions compiler/rustc_data_structures/src/snapshot_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -30,6 +31,7 @@ where
}
}

#[derive(Clone)]
pub enum UndoLog<K, V> {
Inserted(K),
Overwrite(K, V),
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_infer/src/infer/type_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>),
Expand Down Expand Up @@ -58,6 +59,7 @@ impl<'tcx> Rollback<UndoLog<'tcx>> for TypeVariableStorage<'tcx> {
}
}

#[derive(Clone)]
pub struct TypeVariableStorage<'tcx> {
values: sv::SnapshotVecStorage<Delegate>,

Expand Down Expand Up @@ -137,6 +139,7 @@ pub enum TypeVariableOriginKind {
LatticeVariable,
}

#[derive(Clone)]
pub(crate) struct TypeVariableData {
origin: TypeVariableOrigin,
}
Expand Down Expand Up @@ -165,6 +168,7 @@ impl<'tcx> TypeVariableValue<'tcx> {
}
}

#[derive(Clone)]
pub(crate) struct Instantiate;

pub(crate) struct Delegate;
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_infer/src/infer/undo_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>>),
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
}
Expand Down
77 changes: 52 additions & 25 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@
//! [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::traits::query::evaluate_obligation::InferCtxtExt;
use crate::infer::outlives::env::OutlivesEnvironment;
use crate::infer::{CombinedSnapshot, InferOk, RegionckMode};
use crate::traits::select::IntercrateAmbiguityCause;
use crate::traits::util::impl_trait_ref_and_oblig;
use crate::traits::SkipLeakCheck;
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, StripReferences};
use rustc_middle::ty::fold::TypeFoldable;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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()
Expand All @@ -279,12 +287,7 @@ fn implicit_negative<'cx, 'tcx>(
predicate: p,
})
.chain(obligations)
.find(|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).
.find(|o| !selcx.predicate_may_hold_fatal(o));

if let Some(failing_obligation) = opt_failing_obligation {
debug!("overlap: obligation unsatisfiable {:?}", failing_obligation);
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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
selcx.infcx().predicate_must_hold_modulo_regions(o)
let mut fulfillment_cx = FulfillmentContext::new();
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)
}
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/coherence/auxiliary/error_lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![crate_type = "lib"]
#![feature(negative_impls)]
#![feature(with_negative_coherence)]

pub trait Error {}
impl !Error for &str {}
12 changes: 12 additions & 0 deletions src/test/ui/coherence/coherence-negative-outlives-lifetimes.rs
Original file line number Diff line number Diff line change
@@ -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() {}
11 changes: 11 additions & 0 deletions src/test/ui/coherence/coherence-negative-outlives-lifetimes.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// check-pass

#![feature(negative_impls)]
#![feature(with_negative_coherence)]

use std::ops::DerefMut;

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/coherence/coherence-overlap-negative-trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/coherence/coherence-overlap-with-regions.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![feature(negative_impls)]
#![feature(with_negative_coherence)]

pub trait ForeignTrait {}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// check-pass

#![feature(negative_impls)]
#![feature(with_negative_coherence)]

// aux-build: foreign_trait.rs

Expand All @@ -16,8 +17,8 @@
extern crate foreign_trait;
use foreign_trait::ForeignTrait;

trait LocalTrait { }
impl<T: ForeignTrait> LocalTrait for T { }
impl LocalTrait for String { }
trait LocalTrait {}
impl<T: ForeignTrait> LocalTrait for T {}
impl LocalTrait for String {}

fn main() { }
fn main() {}