Skip to content

Commit

Permalink
Rollup merge of rust-lang#109183 - lqd:revert-107376, r=compiler-errors
Browse files Browse the repository at this point in the history
Revert rust-lang#107376 to fix potential `bincode` breakage and `rustc-perf` benchmark.

rust-lang#107376 caused `rustc-perf`'s `webrender` benchmark to break, by regressing on the `bincode-1.3.3` crate.

~~This PR is a draft revert in case we can't land a fix soon enough, and we'd like to land the revert instead~~

(Though I myself think it'd be safer to do the revert, and run crater when relanding rust-lang#107376.)

cc `@aliemjay`
  • Loading branch information
matthiaskrgr authored Mar 15, 2023
2 parents 6e76106 + 5ad1083 commit 7805446
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 87 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ 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 @@ -727,6 +728,7 @@ 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 @@ -2056,7 +2058,8 @@ 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, implied_bounds);
let outlives_environment =
OutlivesEnvironment::with_bounds(param_env, Some(&infcx), implied_bounds);

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

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

let _ = infcx
.err_ctxt()
Expand Down Expand Up @@ -675,6 +676,7 @@ 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, implied_bounds);
let outlives_env = OutlivesEnvironment::with_bounds(param_env, Some(infcx), 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: 8 additions & 1 deletion compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,15 @@ 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. An outdated issue related to the old HIR borrowck. See the test
// 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
// `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: 3 additions & 1 deletion compiler/rustc_infer/src/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,9 +642,11 @@ 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, member_constraints } = region_constraints;
let RegionConstraintData { constraints, verifys, givens, member_constraints } =
region_constraints;

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

debug!(?constraints);

Expand Down
47 changes: 46 additions & 1 deletion 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::IndexVec;
use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::PlaceholderRegion;
use rustc_middle::ty::{self, Ty, TyCtxt};
Expand Down Expand Up @@ -132,6 +132,7 @@ 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 @@ -163,6 +164,38 @@ 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 @@ -329,6 +362,18 @@ 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: 4 additions & 0 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,10 @@ 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: 32 additions & 13 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;
use crate::infer::{GenericKind, InferCtxt};
use crate::traits::query::OutlivesBound;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::transitive_relation::TransitiveRelationBuilder;
use rustc_middle::ty::{self, Region};
use rustc_middle::ty::{self, ReEarlyBound, ReFree, ReVar, 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(explicit_outlives_bounds(param_env));
builder.add_outlives_bounds(None, explicit_outlives_bounds(param_env));

builder
}
Expand All @@ -89,10 +89,11 @@ 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(extra_bounds);
builder.add_outlives_bounds(infcx, extra_bounds);
builder.build()
}

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

/// Processes outlives bounds that are known to hold, whether from implied or other sources.
fn add_outlives_bounds<I>(&mut self, outlives_bounds: I)
///
/// 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)
where
I: IntoIterator<Item = OutlivesBound<'tcx>>,
{
Expand All @@ -136,14 +142,27 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> {
self.region_bound_pairs
.insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a));
}
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"),
},
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)
}
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,9 @@ 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: 41 additions & 3 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;
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_data_structures::intern::Interned;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::undo_log::UndoLogs;
Expand Down Expand Up @@ -104,6 +104,26 @@ 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 @@ -277,6 +297,9 @@ 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 @@ -325,6 +348,9 @@ 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 @@ -466,6 +492,15 @@ 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 @@ -769,8 +804,11 @@ 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 } = self;
constraints.is_empty() && member_constraints.is_empty() && verifys.is_empty()
let RegionConstraintData { constraints, member_constraints, verifys, givens } = self;
constraints.is_empty()
&& member_constraints.is_empty()
&& verifys.is_empty()
&& givens.is_empty()
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ 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: 1 addition & 0 deletions compiler/rustc_trait_selection/src/traits/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ 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: 5 additions & 7 deletions compiler/rustc_trait_selection/src/traits/outlives_bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ 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_infer::infer::resolve::OpportunisticRegionResolver;
use rustc_middle::ty::{self, ParamEnv, Ty, TypeFolder, TypeVisitableExt};
use rustc_middle::ty::{self, ParamEnv, Ty};
use rustc_span::def_id::LocalDefId;

pub use rustc_middle::traits::query::OutlivesBound;
Expand Down Expand Up @@ -53,10 +52,6 @@ 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);
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 @@ -110,7 +105,10 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> {
tys: FxIndexSet<Ty<'tcx>>,
) -> Bounds<'a, 'tcx> {
tys.into_iter()
.map(move |ty| self.implied_outlives_bounds(param_env, body_id, ty))
.map(move |ty| {
let ty = self.resolve_vars_if_possible(ty);
self.implied_outlives_bounds(param_env, body_id, ty)
})
.flatten()
}
}
Loading

0 comments on commit 7805446

Please sign in to comment.