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

remove obsolete givens from regionck #107376

Merged
merged 4 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,6 @@ fn compare_method_predicate_entailment<'tcx>(
// lifetime parameters.
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
Some(infcx),
infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys.clone()),
);
infcx.process_registered_region_obligations(
Expand Down Expand Up @@ -728,7 +727,6 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
// lifetime parameters.
let outlives_environment = OutlivesEnvironment::with_bounds(
param_env,
Some(infcx),
infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys),
);
infcx
Expand Down Expand Up @@ -2051,8 +2049,7 @@ pub(super) fn check_type_bounds<'tcx>(
// Finally, resolve all regions. This catches wily misuses of
// lifetime parameters.
let implied_bounds = infcx.implied_bounds_tys(param_env, impl_ty_def_id, assumed_wf_types);
let outlives_environment =
OutlivesEnvironment::with_bounds(param_env, Some(&infcx), implied_bounds);
let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds);

infcx.err_ctxt().check_region_obligations_and_report_errors(
impl_ty.def_id.expect_local(),
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>(
return;
}

let outlives_environment =
OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds);
let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds);

let _ = infcx
.err_ctxt()
Expand Down Expand Up @@ -676,7 +675,6 @@ fn resolve_regions_with_wf_tys<'tcx>(
let infcx = tcx.infer_ctxt().build();
let outlives_environment = OutlivesEnvironment::with_bounds(
param_env,
Some(&infcx),
infcx.implied_bounds_tys(param_env, id, wf_tys.clone()),
);
let region_bound_pairs = outlives_environment.region_bound_pairs();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ fn get_impl_substs(
}

let implied_bounds = infcx.implied_bounds_tys(param_env, impl1_def_id, assumed_wf_types);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
let _ =
infcx.err_ctxt().check_region_obligations_and_report_errors(impl1_def_id, &outlives_env);
let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else {
Expand Down
9 changes: 1 addition & 8 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Note that there are two tests to check that this remains true
// (`regions-reassign-{match,let}-bound-pointer.rs`).
//
// 2. Things go horribly wrong if we use subtype. The reason for
// THIS is a fairly subtle case involving bound regions. See the
// `givens` field in `region_constraints`, as well as the test
// 2. An outdated issue related to the old HIR borrowck. See the test
// `regions-relate-bound-regions-on-closures-to-inference-variables.rs`,
// for details. Short version is that we must sometimes detect
// relationships between specific region variables and regions
// bound in a closure signature, and that detection gets thrown
// off when we substitute fresh region variables here to enable
// subtyping.
}

/// Compute the new expected type and default binding mode from the old ones
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_infer/src/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,11 +636,9 @@ pub fn make_query_region_constraints<'tcx>(
outlives_obligations: impl Iterator<Item = (Ty<'tcx>, ty::Region<'tcx>, ConstraintCategory<'tcx>)>,
region_constraints: &RegionConstraintData<'tcx>,
) -> QueryRegionConstraints<'tcx> {
let RegionConstraintData { constraints, verifys, givens, member_constraints } =
region_constraints;
let RegionConstraintData { constraints, verifys, member_constraints } = region_constraints;

assert!(verifys.is_empty());
assert!(givens.is_empty());

debug!(?constraints);

Expand Down
47 changes: 1 addition & 46 deletions compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_data_structures::graph::implementation::{
Direction, Graph, NodeIndex, INCOMING, OUTGOING,
};
use rustc_data_structures::intern::Interned;
use rustc_index::vec::{Idx, IndexVec};
use rustc_index::vec::IndexVec;
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::PlaceholderRegion;
use rustc_middle::ty::{self, Ty, TyCtxt};
Expand Down Expand Up @@ -132,7 +132,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
}

let graph = self.construct_graph();
self.expand_givens(&graph);
self.expansion(&mut var_data);
self.collect_errors(&mut var_data, errors);
self.collect_var_errors(&var_data, &graph, errors);
Expand Down Expand Up @@ -164,38 +163,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
}
}

fn expand_givens(&mut self, graph: &RegionGraph<'_>) {
// Givens are a kind of horrible hack to account for
// constraints like 'c <= '0 that are known to hold due to
// closure signatures (see the comment above on the `givens`
// field). They should go away. But until they do, the role
// of this fn is to account for the transitive nature:
//
// Given 'c <= '0
// and '0 <= '1
// then 'c <= '1

let seeds: Vec<_> = self.data.givens.iter().cloned().collect();
for (r, vid) in seeds {
// While all things transitively reachable in the graph
// from the variable (`'0` in the example above).
let seed_index = NodeIndex(vid.index() as usize);
for succ_index in graph.depth_traverse(seed_index, OUTGOING) {
let succ_index = succ_index.0;

// The first N nodes correspond to the region
// variables. Other nodes correspond to constant
// regions.
if succ_index < self.num_vars() {
let succ_vid = RegionVid::new(succ_index);

// Add `'c <= '1`.
self.data.givens.insert((r, succ_vid));
}
}
}
}

/// Gets the LUb of a given region and the empty region
fn lub_empty(&self, a_region: Region<'tcx>) -> Result<Region<'tcx>, PlaceholderRegion> {
match *a_region {
Expand Down Expand Up @@ -362,18 +329,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
) -> bool {
debug!("expand_node({:?}, {:?} == {:?})", a_region, b_vid, b_data);

match *a_region {
// Check if this relationship is implied by a given.
ty::ReEarlyBound(_) | ty::ReFree(_) => {
if self.data.givens.contains(&(a_region, b_vid)) {
debug!("given");
return false;
}
}

_ => {}
}

match *b_data {
VarValue::Empty(empty_ui) => {
let lub = match self.lub_empty(a_region) {
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,10 +855,6 @@ impl<'tcx> InferCtxt<'tcx> {
self.inner.borrow().undo_log.opaque_types_in_snapshot(&snapshot.undo_snapshot)
}

pub fn add_given(&self, sub: ty::Region<'tcx>, sup: ty::RegionVid) {
self.inner.borrow_mut().unwrap_region_constraints().add_given(sub, sup);
}

pub fn can_sub<T>(&self, param_env: ty::ParamEnv<'tcx>, a: T, b: T) -> bool
where
T: at::ToTrace<'tcx>,
Expand Down
45 changes: 13 additions & 32 deletions compiler/rustc_infer/src/infer/outlives/env.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::infer::free_regions::FreeRegionMap;
use crate::infer::{GenericKind, InferCtxt};
use crate::infer::GenericKind;
use crate::traits::query::OutlivesBound;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::transitive_relation::TransitiveRelationBuilder;
use rustc_middle::ty::{self, ReEarlyBound, ReFree, ReVar, Region};
use rustc_middle::ty::{self, Region};

use super::explicit_outlives_bounds;

Expand Down Expand Up @@ -75,7 +75,7 @@ impl<'tcx> OutlivesEnvironment<'tcx> {
region_bound_pairs: Default::default(),
};

builder.add_outlives_bounds(None, explicit_outlives_bounds(param_env));
builder.add_outlives_bounds(explicit_outlives_bounds(param_env));

builder
}
Expand All @@ -89,11 +89,10 @@ impl<'tcx> OutlivesEnvironment<'tcx> {
/// Create a new `OutlivesEnvironment` with extra outlives bounds.
pub fn with_bounds(
param_env: ty::ParamEnv<'tcx>,
infcx: Option<&InferCtxt<'tcx>>,
extra_bounds: impl IntoIterator<Item = OutlivesBound<'tcx>>,
) -> Self {
let mut builder = Self::builder(param_env);
builder.add_outlives_bounds(infcx, extra_bounds);
builder.add_outlives_bounds(extra_bounds);
builder.build()
}

Expand All @@ -120,12 +119,7 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> {
}

/// Processes outlives bounds that are known to hold, whether from implied or other sources.
///
/// The `infcx` parameter is optional; if the implied bounds may
/// contain inference variables, it must be supplied, in which
/// case we will register "givens" on the inference context. (See
/// `RegionConstraintData`.)
fn add_outlives_bounds<I>(&mut self, infcx: Option<&InferCtxt<'tcx>>, outlives_bounds: I)
fn add_outlives_bounds<I>(&mut self, outlives_bounds: I)
where
I: IntoIterator<Item = OutlivesBound<'tcx>>,
{
Expand All @@ -142,27 +136,14 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> {
self.region_bound_pairs
.insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a));
}
OutlivesBound::RegionSubRegion(r_a, r_b) => {
if let (ReEarlyBound(_) | ReFree(_), ReVar(vid_b)) = (r_a.kind(), r_b.kind()) {
infcx
.expect("no infcx provided but region vars found")
.add_given(r_a, vid_b);
} else {
// In principle, we could record (and take
// advantage of) every relationship here, but
// we are also free not to -- it simply means
// strictly less that we can successfully type
// check. Right now we only look for things
// relationships between free regions. (It may
// also be that we should revise our inference
// system to be more general and to make use
// of *every* relationship that arises here,
// but presently we do not.)
if r_a.is_free_or_static() && r_b.is_free() {
self.region_relation.add(r_a, r_b)
}
}
}
OutlivesBound::RegionSubRegion(r_a, r_b) => match (*r_a, *r_b) {
(
ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_),
ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_),
) => self.region_relation.add(r_a, r_b),
(ty::ReError(_), _) | (_, ty::ReError(_)) => {}
_ => bug!("add_outlives_bounds: unexpected regions"),
},
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,6 @@ impl<'tcx> MiniGraph<'tcx> {
&AddConstraint(Constraint::RegSubReg(a, b)) => {
each_edge(a, b);
}
&AddGiven(a, b) => {
each_edge(a, tcx.mk_re_var(b));
}
&AddVerify(i) => span_bug!(
verifys[i].origin.span(),
"we never add verifications while doing higher-ranked things",
Expand Down
44 changes: 3 additions & 41 deletions compiler/rustc_infer/src/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use super::{
InferCtxtUndoLogs, MiscVariable, RegionVariableOrigin, Rollback, Snapshot, SubregionOrigin,
};

use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::intern::Interned;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::undo_log::UndoLogs;
Expand Down Expand Up @@ -104,26 +104,6 @@ pub struct RegionConstraintData<'tcx> {
/// An example is a `A <= B` where neither `A` nor `B` are
/// inference variables.
pub verifys: Vec<Verify<'tcx>>,

/// A "given" is a relationship that is known to hold. In
/// particular, we often know from closure fn signatures that a
/// particular free region must be a subregion of a region
/// variable:
///
/// foo.iter().filter(<'a> |x: &'a &'b T| ...)
///
/// In situations like this, `'b` is in fact a region variable
/// introduced by the call to `iter()`, and `'a` is a bound region
/// on the closure (as indicated by the `<'a>` prefix). If we are
/// naive, we wind up inferring that `'b` must be `'static`,
/// because we require that it be greater than `'a` and we do not
/// know what `'a` is precisely.
///
/// This hashmap is used to avoid that naive scenario. Basically
/// we record the fact that `'a <= 'b` is implied by the fn
/// signature, and then ignore the constraint when solving
/// equations. This is a bit of a hack but seems to work.
pub givens: FxIndexSet<(Region<'tcx>, ty::RegionVid)>,
}

/// Represents a constraint that influences the inference process.
Expand Down Expand Up @@ -297,9 +277,6 @@ pub(crate) enum UndoLog<'tcx> {
/// We added the given `verify`.
AddVerify(usize),

/// We added the given `given`.
AddGiven(Region<'tcx>, ty::RegionVid),

/// We added a GLB/LUB "combination variable".
AddCombination(CombineMapType, TwoRegions<'tcx>),
}
Expand Down Expand Up @@ -348,9 +325,6 @@ impl<'tcx> RegionConstraintStorage<'tcx> {
self.data.verifys.pop();
assert_eq!(self.data.verifys.len(), index);
}
AddGiven(sub, sup) => {
self.data.givens.remove(&(sub, sup));
}
AddCombination(Glb, ref regions) => {
self.glbs.remove(regions);
}
Expand Down Expand Up @@ -492,15 +466,6 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> {
self.undo_log.push(AddVerify(index));
}

pub(super) fn add_given(&mut self, sub: Region<'tcx>, sup: ty::RegionVid) {
// cannot add givens once regions are resolved
if self.data.givens.insert((sub, sup)) {
debug!("add_given({:?} <= {:?})", sub, sup);

self.undo_log.push(AddGiven(sub, sup));
}
}

pub(super) fn make_eqregion(
&mut self,
origin: SubregionOrigin<'tcx>,
Expand Down Expand Up @@ -804,11 +769,8 @@ impl<'tcx> RegionConstraintData<'tcx> {
/// Returns `true` if this region constraint data contains no constraints, and `false`
/// otherwise.
pub fn is_empty(&self) -> bool {
let RegionConstraintData { constraints, member_constraints, verifys, givens } = self;
constraints.is_empty()
&& member_constraints.is_empty()
&& verifys.is_empty()
&& givens.is_empty()
let RegionConstraintData { constraints, member_constraints, verifys } = self;
constraints.is_empty() && member_constraints.is_empty() && verifys.is_empty()
}
}

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ fn resolve_negative_obligation<'tcx>(
let wf_tys = ocx.assumed_wf_types(param_env, DUMMY_SP, body_def_id);
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
Some(&infcx),
infcx.implied_bounds_tys(param_env, body_def_id, wf_tys),
);

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_trait_selection/src/traits/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ pub fn type_allowed_to_implement_copy<'tcx>(
// Check regions assuming the self type of the impl is WF
let outlives_env = OutlivesEnvironment::with_bounds(
param_env,
Some(&infcx),
infcx.implied_bounds_tys(
param_env,
parent_cause.body_id,
Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_trait_selection/src/traits/outlives_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use crate::traits::query::type_op::{self, TypeOp, TypeOpOutput};
use crate::traits::query::NoSolution;
use crate::traits::ObligationCause;
use rustc_data_structures::fx::FxIndexSet;
use rustc_middle::ty::{self, ParamEnv, Ty};
use rustc_infer::infer::resolve::OpportunisticRegionResolver;
use rustc_middle::ty::{self, ParamEnv, Ty, TypeFolder, TypeVisitableExt};
use rustc_span::def_id::LocalDefId;

pub use rustc_middle::traits::query::OutlivesBound;
Expand Down Expand Up @@ -52,6 +53,10 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
body_id: LocalDefId,
ty: Ty<'tcx>,
) -> Vec<OutlivesBound<'tcx>> {
let ty = self.resolve_vars_if_possible(ty);
let ty = OpportunisticRegionResolver::new(self).fold_ty(ty);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it still needed to fold twice here?

assert!(!ty.needs_infer());

let span = self.tcx.def_span(body_id);
let result = param_env
.and(type_op::implied_outlives_bounds::ImpliedOutlivesBounds { ty })
Expand Down Expand Up @@ -105,10 +110,7 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
tys: FxIndexSet<Ty<'tcx>>,
) -> Bounds<'a, 'tcx> {
tys.into_iter()
.map(move |ty| {
let ty = self.resolve_vars_if_possible(ty);
self.implied_outlives_bounds(param_env, body_id, ty)
})
.map(move |ty| self.implied_outlives_bounds(param_env, body_id, ty))
.flatten()
}
}
Loading