Skip to content

Commit

Permalink
Auto merge of rust-lang#111881 - lcnr:leak-check, r=nikomatsakis,jack…
Browse files Browse the repository at this point in the history
…h726

refactor and cleanup the leak check, add it to new solver

ended up being a bit more involved than I wanted but is hopefully still easy enough to review as a single PR, can split it into separate ones otherwise.

this can be reviewed commit by commit:
a473d55cdb9284aa2b01282d1b529a2a4d26547b 31a686646534ca006d906ec757ece4e771d6f973 949039c107852a5e36361c08b62821a0613656f5 242917bf5170d9a723c6c8e23e9d9d0c2fa8dc9d ed2b25a7aa28be3184be9e3022c2796a30eaad87 are all pretty straightforward.

03dd83b4c3f4ff27558f5c8ab859bd9f83db1d04 makes it easier to refactor coherence in a later commit, see the commit description, cc `@oli-obk`

4fe311d807a77b6270f384e41689bf5d58f46aec I don't quite remember what we wanted to test here, this definitely doesn't test that the occurs check doesn't cause incorrect errors in coherence, also cc `@oli-obk` here. I may end up writing a new test for this myself later.

5c200d88a91b75bd0875b973150655bd581ef97a is the main refactor of the leak check, changing it to take the `outer_universe` instead of getting it from a snapshot. Using a snapshot requires us to be in a probe which we aren't in the new solver, it also just feels dirty as snapshots don't really have anything to do with universes.

with all of this cfc230d54188d9c7ed867a9a0d1f51be77b485f9 is now kind of trivial.

r? `@nikomatsakis`
  • Loading branch information
bors committed May 30, 2023
2 parents a9251b6 + 0b81f99 commit f0411ff
Show file tree
Hide file tree
Showing 138 changed files with 320 additions and 244 deletions.
4 changes: 3 additions & 1 deletion compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,8 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
G: FnOnce(Ty<'tcx>) -> Vec<Adjustment<'tcx>>,
{
self.commit_if_ok(|snapshot| {
let outer_universe = self.infcx.universe();

let result = if let ty::FnPtr(fn_ty_b) = b.kind()
&& let (hir::Unsafety::Normal, hir::Unsafety::Unsafe) =
(fn_ty_a.unsafety(), fn_ty_b.unsafety())
Expand All @@ -824,7 +826,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
// want the coerced type to be the actual supertype of these two,
// but for now, we want to just error to ensure we don't lock
// ourselves into a specific behavior with NLL.
self.leak_check(false, snapshot)?;
self.leak_check(outer_universe, Some(snapshot))?;

result
})
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ impl<'tcx> InferCtxt<'tcx> {
tcx: self.tcx,
defining_use_anchor: self.defining_use_anchor,
considering_regions: self.considering_regions,
skip_leak_check: self.skip_leak_check,
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(),
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_infer/src/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,29 +105,31 @@ impl<'tcx> InferCtxt<'tcx> {
self.tcx.replace_bound_vars_uncached(binder, delegate)
}

/// See [RegionConstraintCollector::leak_check][1].
/// See [RegionConstraintCollector::leak_check][1]. We only check placeholder
/// leaking into `outer_universe`, i.e. placeholders which cannot be named by that
/// universe.
///
/// [1]: crate::infer::region_constraints::RegionConstraintCollector::leak_check
pub fn leak_check(
&self,
overly_polymorphic: bool,
snapshot: &CombinedSnapshot<'tcx>,
outer_universe: ty::UniverseIndex,
only_consider_snapshot: Option<&CombinedSnapshot<'tcx>>,
) -> RelateResult<'tcx, ()> {
// If the user gave `-Zno-leak-check`, or we have been
// configured to skip the leak check, then skip the leak check
// completely. The leak check is deprecated. Any legitimate
// subtyping errors that it would have caught will now be
// caught later on, during region checking. However, we
// continue to use it for a transition period.
if self.tcx.sess.opts.unstable_opts.no_leak_check || self.skip_leak_check.get() {
if self.tcx.sess.opts.unstable_opts.no_leak_check || self.skip_leak_check {
return Ok(());
}

self.inner.borrow_mut().unwrap_region_constraints().leak_check(
self.tcx,
overly_polymorphic,
outer_universe,
self.universe(),
snapshot,
only_consider_snapshot,
)
}
}
55 changes: 22 additions & 33 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,13 @@ pub struct InferCtxt<'tcx> {
/// solving is left to borrowck instead.
pub considering_regions: bool,

pub inner: RefCell<InferCtxtInner<'tcx>>,

/// If set, this flag causes us to skip the 'leak check' during
/// higher-ranked subtyping operations. This flag is a temporary one used
/// to manage the removal of the leak-check: for the time being, we still run the
/// leak-check, but we issue warnings. This flag can only be set to true
/// when entering a snapshot.
skip_leak_check: Cell<bool>,
/// leak-check, but we issue warnings.
skip_leak_check: bool,

pub inner: RefCell<InferCtxtInner<'tcx>>,

/// Once region inference is done, the values for each variable.
lexical_region_resolutions: RefCell<Option<LexicalRegionResolutions<'tcx>>>,
Expand Down Expand Up @@ -543,6 +542,7 @@ pub struct InferCtxtBuilder<'tcx> {
tcx: TyCtxt<'tcx>,
defining_use_anchor: DefiningAnchor,
considering_regions: bool,
skip_leak_check: bool,
/// Whether we are in coherence mode.
intercrate: bool,
}
Expand All @@ -557,6 +557,7 @@ impl<'tcx> TyCtxtInferExt<'tcx> for TyCtxt<'tcx> {
tcx: self,
defining_use_anchor: DefiningAnchor::Error,
considering_regions: true,
skip_leak_check: false,
intercrate: false,
}
}
Expand Down Expand Up @@ -584,6 +585,11 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
self
}

pub fn skip_leak_check(mut self, skip_leak_check: bool) -> Self {
self.skip_leak_check = skip_leak_check;
self
}

/// Given a canonical value `C` as a starting point, create an
/// inference context that contains each of the bound values
/// within instantiated as a fresh variable. The `f` closure is
Expand All @@ -605,11 +611,18 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
}

pub fn build(&mut self) -> InferCtxt<'tcx> {
let InferCtxtBuilder { tcx, defining_use_anchor, considering_regions, intercrate } = *self;
let InferCtxtBuilder {
tcx,
defining_use_anchor,
considering_regions,
skip_leak_check,
intercrate,
} = *self;
InferCtxt {
tcx,
defining_use_anchor,
considering_regions,
skip_leak_check,
inner: RefCell::new(InferCtxtInner::new()),
lexical_region_resolutions: RefCell::new(None),
selection_cache: Default::default(),
Expand All @@ -619,7 +632,6 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
tainted_by_errors: Cell::new(None),
err_count_on_creation: tcx.sess.err_count(),
in_snapshot: Cell::new(false),
skip_leak_check: Cell::new(false),
universe: Cell::new(ty::UniverseIndex::ROOT),
intercrate,
}
Expand Down Expand Up @@ -815,32 +827,9 @@ impl<'tcx> InferCtxt<'tcx> {
r
}

/// If `should_skip` is true, then execute `f` then unroll any bindings it creates.
#[instrument(skip(self, f), level = "debug")]
pub fn probe_maybe_skip_leak_check<R, F>(&self, should_skip: bool, f: F) -> R
where
F: FnOnce(&CombinedSnapshot<'tcx>) -> R,
{
let snapshot = self.start_snapshot();
let was_skip_leak_check = self.skip_leak_check.get();
if should_skip {
self.skip_leak_check.set(true);
}
let r = f(&snapshot);
self.rollback_to("probe", snapshot);
self.skip_leak_check.set(was_skip_leak_check);
r
}

/// Scan the constraints produced since `snapshot` began and returns:
///
/// - `None` -- if none of them involves "region outlives" constraints.
/// - `Some(true)` -- if there are `'a: 'b` constraints where `'a` or `'b` is a placeholder.
/// - `Some(false)` -- if there are `'a: 'b` constraints but none involve placeholders.
pub fn region_constraints_added_in_snapshot(
&self,
snapshot: &CombinedSnapshot<'tcx>,
) -> Option<bool> {
/// Scan the constraints produced since `snapshot` and check whether
/// we added any region constraints.
pub fn region_constraints_added_in_snapshot(&self, snapshot: &CombinedSnapshot<'tcx>) -> bool {
self.inner
.borrow_mut()
.unwrap_region_constraints()
Expand Down
32 changes: 22 additions & 10 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,17 +533,29 @@ impl<'tcx> InferCtxt<'tcx> {
// these are the same span, but not in cases like `-> (impl
// Foo, impl Bar)`.
let span = cause.span;
let prev = self.inner.borrow_mut().opaque_types().register(
opaque_type_key,
OpaqueHiddenType { ty: hidden_ty, span },
origin,
);
let mut obligations = if let Some(prev) = prev {
self.at(&cause, param_env)
.eq_exp(DefineOpaqueTypes::Yes, a_is_expected, prev, hidden_ty)?
.obligations
let mut obligations = if self.intercrate {
// During intercrate we do not define opaque types but instead always
// force ambiguity unless the hidden type is known to not implement
// our trait.
vec![traits::Obligation::new(
self.tcx,
cause.clone(),
param_env,
ty::PredicateKind::Ambiguous,
)]
} else {
Vec::new()
let prev = self.inner.borrow_mut().opaque_types().register(
opaque_type_key,
OpaqueHiddenType { ty: hidden_ty, span },
origin,
);
if let Some(prev) = prev {
self.at(&cause, param_env)
.eq_exp(DefineOpaqueTypes::Yes, a_is_expected, prev, hidden_ty)?
.obligations
} else {
Vec::new()
}
};

self.add_item_bounds_for_hidden_type(
Expand Down
Loading

0 comments on commit f0411ff

Please sign in to comment.