Skip to content

fix universes in the NLL type tests #98109

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

Merged
merged 18 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from 9 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
117 changes: 102 additions & 15 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ 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,
ConstraintCategory, Local, Location, ReturnConstraint,
};
use rustc_middle::traits::ObligationCause;
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::Region;
use rustc_middle::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable};
use rustc_span::Span;

Expand Down Expand Up @@ -46,6 +48,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 +562,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 +578,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 +833,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 +850,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 +863,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 +920,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 +952,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 +1182,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 @@ -1171,7 +1193,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {

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

VerifyBound::IfEqBound(verify_if_eq_b) => {
self.eval_if_eq_bound(infcx, param_env, generic_ty, lower_bound, *verify_if_eq_b)
}

VerifyBound::IsEmpty => {
Expand All @@ -1185,33 +1211,71 @@ 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>,
generic_ty: Ty<'tcx>,
lower_bound: RegionVid,
test_ty: Ty<'tcx>,
verify_bound: &VerifyBound<'tcx>,
verify_bound: Region<'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);
let generic_ty_normalized = self.normalize_to_scc_representatives(infcx.tcx, generic_ty);
let test_ty_normalized = self.normalize_to_scc_representatives(infcx.tcx, test_ty);
if generic_ty_normalized == test_ty_normalized {
self.eval_verify_bound(tcx, body, generic_ty, lower_bound, verify_bound)
let verify_bound_vid = self.to_region_vid(verify_bound);
self.eval_outlives(verify_bound_vid, lower_bound)
} else {
false
}
}

fn eval_if_eq_bound(
&self,
infcx: &InferCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
generic_ty: Ty<'tcx>,
lower_bound: RegionVid,
verify_if_eq_b: ty::Binder<'tcx, VerifyIfEq<'tcx>>,
) -> bool {
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_bound(
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,
}
}

/// This is a conservative normalization procedure. It takes every
/// free region in `value` and replaces it with the
/// "representative" of its SCC (see `scc_representatives` field).
Expand Down Expand Up @@ -1278,6 +1342,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 +1368,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 +1379,18 @@ 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!(
"eval_outlives: returning {} because of comparison between points in sup/sub",
result
);
result
}

/// Once regions have been propagated, this method is used to see
Expand Down
25 changes: 22 additions & 3 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,24 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
min: ty::Region<'tcx>,
) -> bool {
match bound {
VerifyBound::IfEq(k, b) => {
VerifyBound::IfEq(k, r) => {
(var_values.normalize(self.region_rels.tcx, *k) == generic_ty)
&& self.bound_is_met(b, var_values, generic_ty, min)
&& self.bound_is_met(&VerifyBound::OutlivedBy(*r), var_values, generic_ty, min)
}

VerifyBound::IfEqBound(verify_if_eq_b) => {
match test_type_match::extract_verify_if_eq_bound(
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
Loading