bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #111881 - lcnr:leak-check, r=nikomatsakis,jackh726

Browse source 
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`
This commit is contained in:
bors 2023-05-30 18:48:12 +00:00
commit f0411ffceb
138 changed files with 320 additions and 244 deletions
Download patch file Download diff file Expand all files Collapse all files

7
compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs
View file

@ -137,6 +137,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
#[instrument(level = "debug", skip(self), ret)]
fn compute_external_query_constraints(&self) -> Result<ExternalConstraints<'tcx>, NoSolution> {
// We only check for leaks from universes which were entered inside
// of the query.
self.infcx.leak_check(self.max_input_universe, None).map_err(|e| {
debug!(?e, "failed the leak check");
NoSolution
})?;
// Cannot use `take_registered_region_obligations` as we may compute the response
// inside of a `probe` whenever we have multiple choices inside of the solver.
let region_obligations = self.infcx.inner.borrow().region_obligations().to_owned();

111
compiler/rustc_trait_selection/src/traits/coherence.rs
View file

@ -5,7 +5,7 @@
//! [trait-specialization]: https://rustc-dev-guide.rust-lang.org/traits/specialization.html
use crate::infer::outlives::env::OutlivesEnvironment;
use crate::infer::{CombinedSnapshot, InferOk};
use crate::infer::InferOk;
use crate::traits::outlives_bounds::InferCtxtExt as _;
use crate::traits::select::IntercrateAmbiguityCause;
use crate::traits::util::impl_subject_and_oblig;
@ -62,6 +62,21 @@ pub fn add_placeholder_note(err: &mut Diagnostic) {
);
}
#[derive(Debug, Clone, Copy)]
enum TrackAmbiguityCauses {
Yes,
No,
}
impl TrackAmbiguityCauses {
fn is_yes(self) -> bool {
match self {
TrackAmbiguityCauses::Yes => true,
TrackAmbiguityCauses::No => false,
}
}
}
/// If there are types that satisfy both impls, returns `Some`
/// with a suitably-freshened `ImplHeader` with those types
/// substituted. Otherwise, returns `None`.
@ -97,29 +112,28 @@ pub fn overlapping_impls(
return None;
}
let infcx = tcx
.infer_ctxt()
.with_opaque_type_inference(DefiningAnchor::Bubble)
.intercrate(true)
.build();
let selcx = &mut SelectionContext::new(&infcx);
let overlaps =
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).is_some();
if !overlaps {
return None;
}
let _overlap_with_bad_diagnostics = overlap(
tcx,
TrackAmbiguityCauses::No,
skip_leak_check,
impl1_def_id,
impl2_def_id,
overlap_mode,
)?;
// In the case where we detect an error, run the check again, but
// this time tracking intercrate ambiguity causes for better
// diagnostics. (These take time and can lead to false errors.)
let infcx = tcx
.infer_ctxt()
.with_opaque_type_inference(DefiningAnchor::Bubble)
.intercrate(true)
.build();
let selcx = &mut SelectionContext::new(&infcx);
selcx.enable_tracking_intercrate_ambiguity_causes();
Some(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap())
let overlap = overlap(
tcx,
TrackAmbiguityCauses::Yes,
skip_leak_check,
impl1_def_id,
impl2_def_id,
overlap_mode,
)
.unwrap();
Some(overlap)
}
fn with_fresh_ty_vars<'cx, 'tcx>(
@ -146,40 +160,34 @@ fn with_fresh_ty_vars<'cx, 'tcx>(
/// Can both impl `a` and impl `b` be satisfied by a common type (including
/// where-clauses)? If so, returns an `ImplHeader` that unifies the two impls.
fn overlap<'cx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'tcx>,
#[instrument(level = "debug", skip(tcx))]
fn overlap<'tcx>(
tcx: TyCtxt<'tcx>,
track_ambiguity_causes: TrackAmbiguityCauses,
skip_leak_check: SkipLeakCheck,
impl1_def_id: DefId,
impl2_def_id: DefId,
overlap_mode: OverlapMode,
) -> Option<OverlapResult<'tcx>> {
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(selcx, impl1_def_id, impl2_def_id, overlap_mode, snapshot)
})
}
fn overlap_within_probe<'cx, 'tcx>(
selcx: &mut SelectionContext<'cx, 'tcx>,
impl1_def_id: DefId,
impl2_def_id: DefId,
overlap_mode: OverlapMode,
snapshot: &CombinedSnapshot<'tcx>,
) -> Option<OverlapResult<'tcx>> {
let infcx = selcx.infcx;
if overlap_mode.use_negative_impl() {
if negative_impl(infcx.tcx, impl1_def_id, impl2_def_id)
|| negative_impl(infcx.tcx, impl2_def_id, impl1_def_id)
if negative_impl(tcx, impl1_def_id, impl2_def_id)
|| negative_impl(tcx, impl2_def_id, impl1_def_id)
{
return None;
}
}
let infcx = tcx
.infer_ctxt()
.with_opaque_type_inference(DefiningAnchor::Bubble)
.skip_leak_check(skip_leak_check.is_yes())
.intercrate(true)
.build();
let selcx = &mut SelectionContext::new(&infcx);
if track_ambiguity_causes.is_yes() {
selcx.enable_tracking_intercrate_ambiguity_causes();
}
// For the purposes of this check, we don't bring any placeholder
// types into scope; instead, we replace the generic types with
// fresh type variables, and hence we do our evaluations in an
@ -198,18 +206,23 @@ fn overlap_within_probe<'cx, 'tcx>(
}
}
// We disable the leak when creating the `snapshot` by using
// `infcx.probe_maybe_disable_leak_check`.
if infcx.leak_check(true, snapshot).is_err() {
// We toggle the `leak_check` by using `skip_leak_check` when constructing the
// inference context, so this may be a noop.
if infcx.leak_check(ty::UniverseIndex::ROOT, None).is_err() {
debug!("overlap: leak check failed");
return None;
}
let intercrate_ambiguity_causes = selcx.take_intercrate_ambiguity_causes();
debug!("overlap: intercrate_ambiguity_causes={:#?}", intercrate_ambiguity_causes);
let involves_placeholder =
matches!(selcx.infcx.region_constraints_added_in_snapshot(snapshot), Some(true));
let involves_placeholder = infcx
.inner
.borrow_mut()
.unwrap_region_constraints()
.data()
.constraints
.iter()
.any(|c| c.0.involves_placeholders());
let impl_header = selcx.infcx.resolve_vars_if_possible(impl1_header);
Some(OverlapResult { impl_header, intercrate_ambiguity_causes, involves_placeholder })

2
compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs
View file

@ -90,7 +90,7 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
Ok(EvaluationResult::EvaluatedToAmbig)
} else if self.opaque_types_added_in_snapshot(snapshot) {
Ok(EvaluationResult::EvaluatedToOkModuloOpaqueTypes)
} else if self.region_constraints_added_in_snapshot(snapshot).is_some() {
} else if self.region_constraints_added_in_snapshot(snapshot) {
Ok(EvaluationResult::EvaluatedToOkModuloRegions)
} else {
Ok(EvaluationResult::EvaluatedToOk)

10
compiler/rustc_trait_selection/src/traits/select/mod.rs
View file

@ -561,9 +561,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
op: impl FnOnce(&mut Self) -> Result<EvaluationResult, OverflowError>,
) -> Result<EvaluationResult, OverflowError> {
self.infcx.probe(|snapshot| -> Result<EvaluationResult, OverflowError> {
let outer_universe = self.infcx.universe();
let result = op(self)?;
match self.infcx.leak_check(true, snapshot) {
match self.infcx.leak_check(outer_universe, Some(snapshot)) {
Ok(()) => {}
Err(_) => return Ok(EvaluatedToErr),
}
@ -572,9 +573,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
return Ok(result.max(EvaluatedToOkModuloOpaqueTypes));
}
match self.infcx.region_constraints_added_in_snapshot(snapshot) {
None => Ok(result),
Some(_) => Ok(result.max(EvaluatedToOkModuloRegions)),
if self.infcx.region_constraints_added_in_snapshot(snapshot) {
Ok(result.max(EvaluatedToOkModuloRegions))
} else {
Ok(result)
}
})
}