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 universes in the NLL type tests #98109

Merged
merged 18 commits into from
Jun 24, 2022
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
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>(

// Solve the region constraints.
let (closure_region_requirements, nll_errors) =
regioncx.solve(infcx, &body, polonius_output.clone());
regioncx.solve(infcx, param_env, &body, polonius_output.clone());

if !nll_errors.is_empty() {
// Suppress unhelpful extra errors in `infer_opaque_types`.
Expand Down
101 changes: 81 additions & 20 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
use rustc_hir::CRATE_HIR_ID;
use rustc_index::vec::IndexVec;
use rustc_infer::infer::canonical::QueryOutlivesConstraint;
use rustc_infer::infer::region_constraints::{GenericKind, VarInfos, VerifyBound};
use rustc_infer::infer::outlives::test_type_match;
use rustc_infer::infer::region_constraints::{GenericKind, VarInfos, VerifyBound, VerifyIfEq};
use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin, RegionVariableOrigin};
use rustc_middle::mir::{
Body, ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements,
Expand Down Expand Up @@ -46,6 +47,7 @@ pub mod values;

pub struct RegionInferenceContext<'tcx> {
pub var_infos: VarInfos,

/// Contains the definition for every region variable. Region
/// variables are identified by their index (`RegionVid`). The
/// definition contains information about where the region came
Expand Down Expand Up @@ -559,6 +561,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
pub(super) fn solve(
&mut self,
infcx: &InferCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
body: &Body<'tcx>,
polonius_output: Option<Rc<PoloniusOutput>>,
) -> (Option<ClosureRegionRequirements<'tcx>>, RegionErrors<'tcx>) {
Expand All @@ -574,7 +577,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// eagerly.
let mut outlives_requirements = infcx.tcx.is_typeck_child(mir_def_id).then(Vec::new);

self.check_type_tests(infcx, body, outlives_requirements.as_mut(), &mut errors_buffer);
self.check_type_tests(
infcx,
param_env,
body,
outlives_requirements.as_mut(),
&mut errors_buffer,
);

// In Polonius mode, the errors about missing universal region relations are in the output
// and need to be emitted or propagated. Otherwise, we need to check whether the
Expand Down Expand Up @@ -823,6 +832,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
fn check_type_tests(
&self,
infcx: &InferCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
body: &Body<'tcx>,
mut propagated_outlives_requirements: Option<&mut Vec<ClosureOutlivesRequirement<'tcx>>>,
errors_buffer: &mut RegionErrors<'tcx>,
Expand All @@ -839,7 +849,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {

let generic_ty = type_test.generic_kind.to_ty(tcx);
if self.eval_verify_bound(
tcx,
infcx,
param_env,
body,
generic_ty,
type_test.lower_bound,
Expand All @@ -851,6 +862,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
if let Some(propagated_outlives_requirements) = &mut propagated_outlives_requirements {
if self.try_promote_type_test(
infcx,
param_env,
body,
type_test,
propagated_outlives_requirements,
Expand Down Expand Up @@ -907,6 +919,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
fn try_promote_type_test(
&self,
infcx: &InferCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
body: &Body<'tcx>,
type_test: &TypeTest<'tcx>,
propagated_outlives_requirements: &mut Vec<ClosureOutlivesRequirement<'tcx>>,
Expand Down Expand Up @@ -938,7 +951,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// where `ur` is a local bound -- we are sometimes in a
// position to prove things that our caller cannot. See
// #53570 for an example.
if self.eval_verify_bound(tcx, body, generic_ty, ur, &type_test.verify_bound) {
if self.eval_verify_bound(
infcx,
param_env,
body,
generic_ty,
ur,
&type_test.verify_bound,
) {
continue;
}

Expand Down Expand Up @@ -1161,7 +1181,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
/// `point`.
fn eval_verify_bound(
&self,
tcx: TyCtxt<'tcx>,
infcx: &InferCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
body: &Body<'tcx>,
generic_ty: Ty<'tcx>,
lower_bound: RegionVid,
Expand All @@ -1170,8 +1191,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
debug!("eval_verify_bound(lower_bound={:?}, verify_bound={:?})", lower_bound, verify_bound);

match verify_bound {
VerifyBound::IfEq(test_ty, verify_bound1) => {
self.eval_if_eq(tcx, body, generic_ty, lower_bound, *test_ty, verify_bound1)
VerifyBound::IfEq(verify_if_eq_b) => {
self.eval_if_eq(infcx, param_env, generic_ty, lower_bound, *verify_if_eq_b)
}

VerifyBound::IsEmpty => {
Expand All @@ -1185,30 +1206,50 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}

VerifyBound::AnyBound(verify_bounds) => verify_bounds.iter().any(|verify_bound| {
self.eval_verify_bound(tcx, body, generic_ty, lower_bound, verify_bound)
self.eval_verify_bound(
infcx,
param_env,
body,
generic_ty,
lower_bound,
verify_bound,
)
}),

VerifyBound::AllBounds(verify_bounds) => verify_bounds.iter().all(|verify_bound| {
self.eval_verify_bound(tcx, body, generic_ty, lower_bound, verify_bound)
self.eval_verify_bound(
infcx,
param_env,
body,
generic_ty,
lower_bound,
verify_bound,
)
}),
}
}

fn eval_if_eq(
&self,
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
infcx: &InferCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
generic_ty: Ty<'tcx>,
lower_bound: RegionVid,
test_ty: Ty<'tcx>,
verify_bound: &VerifyBound<'tcx>,
verify_if_eq_b: ty::Binder<'tcx, VerifyIfEq<'tcx>>,
) -> bool {
let generic_ty_normalized = self.normalize_to_scc_representatives(tcx, generic_ty);
let test_ty_normalized = self.normalize_to_scc_representatives(tcx, test_ty);
if generic_ty_normalized == test_ty_normalized {
self.eval_verify_bound(tcx, body, generic_ty, lower_bound, verify_bound)
} else {
false
let generic_ty = self.normalize_to_scc_representatives(infcx.tcx, generic_ty);
let verify_if_eq_b = self.normalize_to_scc_representatives(infcx.tcx, verify_if_eq_b);
match test_type_match::extract_verify_if_eq(
infcx.tcx,
param_env,
&verify_if_eq_b,
generic_ty,
) {
Some(r) => {
let r_vid = self.to_region_vid(r);
self.eval_outlives(r_vid, lower_bound)
}
None => false,
}
}

Expand Down Expand Up @@ -1278,6 +1319,18 @@ impl<'tcx> RegionInferenceContext<'tcx> {
let sub_region_scc = self.constraint_sccs.scc(sub_region);
let sup_region_scc = self.constraint_sccs.scc(sup_region);

// If we are checking that `'sup: 'sub`, and `'sub` contains
// some placeholder that `'sup` cannot name, then this is only
// true if `'sup` outlives static.
if !self.universe_compatible(sub_region_scc, sup_region_scc) {
debug!(
"eval_outlives: sub universe `{sub_region_scc:?}` is not nameable \
by super `{sup_region_scc:?}`, promoting to static",
);

return self.eval_outlives(sup_region, self.universal_regions.fr_static);
}

// Both the `sub_region` and `sup_region` consist of the union
// of some number of universal regions (along with the union
// of various points in the CFG; ignore those points for
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious if the logic below (let universal_outlives = ...) is necessary due to the added logic above. I don't know enough about borrowck to make a judgement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it is still relevant. I think that the code below is meant to deal with "top-level" generics declared on the function, where we do have finer-grained outlives data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In other words, it deals with relationships between universals and variables that are in the same universe.

Expand All @@ -1292,6 +1345,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
});

if !universal_outlives {
debug!(
"eval_outlives: returning false because sub region contains a universal region not present in super"
);
return false;
}

Expand All @@ -1300,10 +1356,15 @@ impl<'tcx> RegionInferenceContext<'tcx> {

if self.universal_regions.is_universal_region(sup_region) {
// Micro-opt: universal regions contain all points.
debug!(
"eval_outlives: returning true because super is universal and hence contains all points"
);
return true;
}

self.scc_values.contains_points(sup_region_scc, sub_region_scc)
let result = self.scc_values.contains_points(sup_region_scc, sub_region_scc);
debug!("returning {} because of comparison between points in sup/sub", result);
result
}

/// Once regions have been propagated, this method is used to see
Expand Down
23 changes: 19 additions & 4 deletions compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,22 @@ use rustc_middle::ty::{Region, RegionVid};
use rustc_span::Span;
use std::fmt;

use super::outlives::test_type_match;

/// This function performs lexical region resolution given a complete
/// set of constraints and variable origins. It performs a fixed-point
/// iteration to find region values which satisfy all constraints,
/// assuming such values can be found. It returns the final values of
/// all the variables as well as a set of errors that must be reported.
#[instrument(level = "debug", skip(region_rels, var_infos, data))]
pub(crate) fn resolve<'tcx>(
param_env: ty::ParamEnv<'tcx>,
region_rels: &RegionRelations<'_, 'tcx>,
var_infos: VarInfos,
data: RegionConstraintData<'tcx>,
) -> (LexicalRegionResolutions<'tcx>, Vec<RegionResolutionError<'tcx>>) {
let mut errors = vec![];
let mut resolver = LexicalResolver { region_rels, var_infos, data };
let mut resolver = LexicalResolver { param_env, region_rels, var_infos, data };
let values = resolver.infer_variable_values(&mut errors);
(values, errors)
}
Expand Down Expand Up @@ -100,6 +103,7 @@ struct RegionAndOrigin<'tcx> {
type RegionGraph<'tcx> = Graph<(), Constraint<'tcx>>;

struct LexicalResolver<'cx, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
region_rels: &'cx RegionRelations<'cx, 'tcx>,
var_infos: VarInfos,
data: RegionConstraintData<'tcx>,
Expand Down Expand Up @@ -818,9 +822,20 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
min: ty::Region<'tcx>,
) -> bool {
match bound {
VerifyBound::IfEq(k, b) => {
(var_values.normalize(self.region_rels.tcx, *k) == generic_ty)
&& self.bound_is_met(b, var_values, generic_ty, min)
VerifyBound::IfEq(verify_if_eq_b) => {
let verify_if_eq_b = var_values.normalize(self.region_rels.tcx, *verify_if_eq_b);
match test_type_match::extract_verify_if_eq(
self.tcx(),
self.param_env,
&verify_if_eq_b,
generic_ty,
) {
Some(r) => {
self.bound_is_met(&VerifyBound::OutlivedBy(r), var_values, generic_ty, min)
}

None => false,
}
}

VerifyBound::OutlivedBy(r) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1290,7 +1290,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
&RegionRelations::new(self.tcx, region_context, outlives_env.free_region_map());

let (lexical_region_resolutions, errors) =
lexical_region_resolve::resolve(region_rels, var_infos, data);
lexical_region_resolve::resolve(outlives_env.param_env, region_rels, var_infos, data);

let old_value = self.lexical_region_resolutions.replace(Some(lexical_region_resolutions));
assert!(old_value.is_none());
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/outlives/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
pub mod components;
pub mod env;
pub mod obligations;
pub mod test_type_match;
pub mod verify;

use rustc_middle::traits::query::OutlivesBound;
Expand Down
41 changes: 27 additions & 14 deletions compiler/rustc_infer/src/infer/outlives/obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,17 +318,13 @@ where
self.delegate.push_verify(origin, generic, region, verify_bound);
}

#[tracing::instrument(level = "debug", skip(self))]
fn projection_must_outlive(
&mut self,
origin: infer::SubregionOrigin<'tcx>,
region: ty::Region<'tcx>,
projection_ty: ty::ProjectionTy<'tcx>,
) {
debug!(
"projection_must_outlive(region={:?}, projection_ty={:?}, origin={:?})",
region, projection_ty, origin
);

// This case is thorny for inference. The fundamental problem is
// that there are many cases where we have choice, and inference
// doesn't like choice (the current region inference in
Expand Down Expand Up @@ -363,13 +359,21 @@ where
// #55756) in cases where you have e.g., `<T as Foo<'a>>::Item:
// 'a` in the environment but `trait Foo<'b> { type Item: 'b
// }` in the trait definition.
approx_env_bounds.retain(|bound| match *bound.0.kind() {
ty::Projection(projection_ty) => self
.verify_bound
.projection_declared_bounds_from_trait(projection_ty)
.all(|r| r != bound.1),

_ => panic!("expected only projection types from env, not {:?}", bound.0),
approx_env_bounds.retain(|bound_outlives| {
// OK to skip binder because we only manipulate and compare against other
// values from the same binder. e.g. if we have (e.g.) `for<'a> <T as Trait<'a>>::Item: 'a`
// in `bound`, the `'a` will be a `^1` (bound, debruijn index == innermost) region.
// If the declaration is `trait Trait<'b> { type Item: 'b; }`, then `projection_declared_bounds_from_trait`
// will be invoked with `['b => ^1]` and so we will get `^1` returned.
let bound = bound_outlives.skip_binder();
match *bound.0.kind() {
ty::Projection(projection_ty) => self
.verify_bound
.projection_declared_bounds_from_trait(projection_ty)
.all(|r| r != bound.1),

_ => panic!("expected only projection types from env, not {:?}", bound.0),
}
});

// If declared bounds list is empty, the only applicable rule is
Expand Down Expand Up @@ -420,8 +424,16 @@ where
if !trait_bounds.is_empty()
&& trait_bounds[1..]
.iter()
.chain(approx_env_bounds.iter().map(|b| &b.1))
.all(|b| *b == trait_bounds[0])
.map(|r| Some(*r))
.chain(
// NB: The environment may contain `for<'a> T: 'a` style bounds.
// In that case, we don't know if they are equal to the trait bound
// or not (since we don't *know* whether the environment bound even applies),
// so just map to `None` here if there are bound vars, ensuring that
// the call to `all` will fail below.
approx_env_bounds.iter().map(|b| b.map_bound(|b| b.1).no_bound_vars()),
)
.all(|b| b == Some(trait_bounds[0]))
{
let unique_bound = trait_bounds[0];
debug!("projection_must_outlive: unique trait bound = {:?}", unique_bound);
Expand All @@ -437,6 +449,7 @@ where
// even though a satisfactory solution exists.
let generic = GenericKind::Projection(projection_ty);
let verify_bound = self.verify_bound.generic_bound(generic);
debug!("projection_must_outlive: pushing {:?}", verify_bound);
self.delegate.push_verify(origin, generic, region, verify_bound);
}
}
Expand Down
Loading