Skip to content

Commit b86657a

Browse files
committed
Auto merge of #116040 - compiler-errors:alias-liveness, r=<try>
Consider alias bounds when computing liveness in NLL # Background Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark **all** of the regions in their args[^1] as live, leading to bugs like #42940. In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {} fn test_two_mut(mut x: ()) { let _f1 = capture(&mut x); let _f2 = capture(&mut x); //~^ ERROR cannot borrow `x` as mutable more than once at a time } ``` In the example above, we should be able to deduce from the `'static` bound on `capture`'s opaque that even though `'o` is a captured region, it *can never* show up in the opaque's hidden type, and can soundly be ignored for liveness purposes. # The Fix We apply a simple version of RFC 1214's `OutlivesProjectionEnv` and `OutlivesProjectionTraitDef` rules to NLL's `make_all_regions_live` computation. Specifically, when we encounter an alias type, we: 1. Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is `'static`, and in that case, prefer `'static`. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque. 2. Otherwise, walk through the alias's args recursively, as we do today. ## Additionally: Opaque Hidden Types A similar bug comes about when walking through the hidden type of an opaque to validate it doesn't capture lifetimes that aren't part of that opaque. For example: ```rust trait Captures<'a> {} impl<T> Captures<'_> for T {} fn a() -> impl Sized + 'static { b(&vec![]) } fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {} ``` The hidden type of `a::{opaque}` is inferred to be `b::<'?1>::{opaque}`, where `'?1` is a region local to the body of `a`. However, the `'static` bound on `b`'s opaque should allow us to deduce that the hidden type will never mention that region `'?1`, and we can ignore it (and replace it with `ReErased` in `b`'s opaque's hidden type). Similarly to liveness, we don't currently do anything other than recurse through aliases for their lifetime components. This PR ends up using the same region visitor as liveness for opaque type region captures. ## Limitations This approach has some limitations. Specifically, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice. If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of `'static`, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future. ## Read more Context: #42940 (comment) More context: #115822 (comment) Fixes #42940 [^1]: except for bivariant region args in opaques, which will become less relevant when we move onto edition 2024 capture semantics for opaques.
2 parents 93b6a36 + 1b88ace commit b86657a

File tree

12 files changed

+301
-103
lines changed

12 files changed

+301
-103
lines changed

compiler/rustc_borrowck/src/type_check/liveness/trace.rs

+12-10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
22
use rustc_index::bit_set::HybridBitSet;
33
use rustc_index::interval::IntervalSet;
44
use rustc_infer::infer::canonical::QueryRegionConstraints;
5+
use rustc_infer::infer::outlives::for_liveness::FreeRegionsVisitor;
56
use rustc_middle::mir::{BasicBlock, Body, ConstraintCategory, Local, Location};
67
use rustc_middle::traits::query::DropckOutlivesResult;
78
use rustc_middle::ty::{Ty, TyCtxt, TypeVisitable, TypeVisitableExt};
@@ -554,16 +555,17 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> {
554555
"make_all_regions_live: live_at={}",
555556
values::location_set_str(elements, live_at.iter()),
556557
);
557-
558-
let tcx = typeck.tcx();
559-
tcx.for_each_free_region(&value, |live_region| {
560-
let live_region_vid =
561-
typeck.borrowck_context.universal_regions.to_region_vid(live_region);
562-
typeck
563-
.borrowck_context
564-
.constraints
565-
.liveness_constraints
566-
.add_elements(live_region_vid, live_at);
558+
value.visit_with(&mut FreeRegionsVisitor {
559+
tcx: typeck.tcx(),
560+
param_env: typeck.param_env,
561+
op: |r| {
562+
let live_region_vid = typeck.borrowck_context.universal_regions.to_region_vid(r);
563+
typeck
564+
.borrowck_context
565+
.constraints
566+
.liveness_constraints
567+
.add_elements(live_region_vid, live_at);
568+
},
567569
});
568570
}
569571

compiler/rustc_infer/src/infer/opaque_types.rs

+5-93
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use super::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
22
use super::{DefineOpaqueTypes, InferResult};
33
use crate::errors::OpaqueHiddenTypeDiag;
4+
use crate::infer::outlives::for_liveness::FreeRegionsVisitor;
45
use crate::infer::{InferCtxt, InferOk};
56
use crate::traits::{self, PredicateObligation};
67
use hir::def_id::{DefId, LocalDefId};
@@ -13,11 +14,10 @@ use rustc_middle::ty::error::{ExpectedFound, TypeError};
1314
use rustc_middle::ty::fold::BottomUpFolder;
1415
use rustc_middle::ty::GenericArgKind;
1516
use rustc_middle::ty::{
16-
self, OpaqueHiddenType, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable,
17-
TypeVisitable, TypeVisitableExt, TypeVisitor,
17+
self, OpaqueHiddenType, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable, TypeVisitable,
18+
TypeVisitableExt,
1819
};
1920
use rustc_span::Span;
20-
use std::ops::ControlFlow;
2121

2222
mod table;
2323

@@ -380,8 +380,9 @@ impl<'tcx> InferCtxt<'tcx> {
380380
.collect(),
381381
);
382382

383-
concrete_ty.visit_with(&mut ConstrainOpaqueTypeRegionVisitor {
383+
concrete_ty.visit_with(&mut FreeRegionsVisitor {
384384
tcx: self.tcx,
385+
param_env,
385386
op: |r| self.member_constraint(opaque_type_key, span, concrete_ty, r, &choice_regions),
386387
});
387388
}
@@ -415,95 +416,6 @@ impl<'tcx> InferCtxt<'tcx> {
415416
}
416417
}
417418

418-
/// Visitor that requires that (almost) all regions in the type visited outlive
419-
/// `least_region`. We cannot use `push_outlives_components` because regions in
420-
/// closure signatures are not included in their outlives components. We need to
421-
/// ensure all regions outlive the given bound so that we don't end up with,
422-
/// say, `ReVar` appearing in a return type and causing ICEs when other
423-
/// functions end up with region constraints involving regions from other
424-
/// functions.
425-
///
426-
/// We also cannot use `for_each_free_region` because for closures it includes
427-
/// the regions parameters from the enclosing item.
428-
///
429-
/// We ignore any type parameters because impl trait values are assumed to
430-
/// capture all the in-scope type parameters.
431-
pub struct ConstrainOpaqueTypeRegionVisitor<'tcx, OP: FnMut(ty::Region<'tcx>)> {
432-
pub tcx: TyCtxt<'tcx>,
433-
pub op: OP,
434-
}
435-
436-
impl<'tcx, OP> TypeVisitor<TyCtxt<'tcx>> for ConstrainOpaqueTypeRegionVisitor<'tcx, OP>
437-
where
438-
OP: FnMut(ty::Region<'tcx>),
439-
{
440-
fn visit_binder<T: TypeVisitable<TyCtxt<'tcx>>>(
441-
&mut self,
442-
t: &ty::Binder<'tcx, T>,
443-
) -> ControlFlow<Self::BreakTy> {
444-
t.super_visit_with(self);
445-
ControlFlow::Continue(())
446-
}
447-
448-
fn visit_region(&mut self, r: ty::Region<'tcx>) -> ControlFlow<Self::BreakTy> {
449-
match *r {
450-
// ignore bound regions, keep visiting
451-
ty::ReLateBound(_, _) => ControlFlow::Continue(()),
452-
_ => {
453-
(self.op)(r);
454-
ControlFlow::Continue(())
455-
}
456-
}
457-
}
458-
459-
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
460-
// We're only interested in types involving regions
461-
if !ty.flags().intersects(ty::TypeFlags::HAS_FREE_REGIONS) {
462-
return ControlFlow::Continue(());
463-
}
464-
465-
match ty.kind() {
466-
ty::Closure(_, ref args) => {
467-
// Skip lifetime parameters of the enclosing item(s)
468-
469-
for upvar in args.as_closure().upvar_tys() {
470-
upvar.visit_with(self);
471-
}
472-
args.as_closure().sig_as_fn_ptr_ty().visit_with(self);
473-
}
474-
475-
ty::Generator(_, ref args, _) => {
476-
// Skip lifetime parameters of the enclosing item(s)
477-
// Also skip the witness type, because that has no free regions.
478-
479-
for upvar in args.as_generator().upvar_tys() {
480-
upvar.visit_with(self);
481-
}
482-
args.as_generator().return_ty().visit_with(self);
483-
args.as_generator().yield_ty().visit_with(self);
484-
args.as_generator().resume_ty().visit_with(self);
485-
}
486-
487-
ty::Alias(ty::Opaque, ty::AliasTy { def_id, ref args, .. }) => {
488-
// Skip lifetime parameters that are not captures.
489-
let variances = self.tcx.variances_of(*def_id);
490-
491-
for (v, s) in std::iter::zip(variances, args.iter()) {
492-
if *v != ty::Variance::Bivariant {
493-
s.visit_with(self);
494-
}
495-
}
496-
}
497-
498-
_ => {
499-
ty.super_visit_with(self);
500-
}
501-
}
502-
503-
ControlFlow::Continue(())
504-
}
505-
}
506-
507419
pub enum UseKind {
508420
DefiningUse,
509421
OpaqueUse,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor};
2+
3+
use std::ops::ControlFlow;
4+
5+
use crate::infer::outlives::test_type_match;
6+
use crate::infer::region_constraints::VerifyIfEq;
7+
8+
/// Visits free regions in the type that are relevant for liveness computation.
9+
/// These regions are passed to `OP`.
10+
///
11+
/// Specifically, we visit all of the regions of types recursively, except:
12+
///
13+
/// 1. If the type is an alias, we look at the outlives bounds in the param-env
14+
/// and alias's item bounds. If there is a unique outlives bound, then visit
15+
/// that instead. If there is not a unique but there is a `'static` outlives
16+
/// bound, then don't visit anything. Otherwise, walk through the opaque's
17+
/// regions structurally.
18+
///
19+
/// 2. If the type is a closure, only walk through the signature of the closure
20+
/// and the upvars. Similarly, if the type is a generator, walk through the
21+
/// three "signature" types (yield/resume/return) and its upvars. All generator
22+
/// interior regions are bound regions, so ther is no need to walk through
23+
/// that type.
24+
///
25+
/// # How does this differ from other region visitors?
26+
///
27+
/// We cannot use `push_outlives_components` because regions in closure
28+
/// signatures are not included in their outlives components. We need to
29+
/// ensure all regions outlive the given bound so that we don't end up with,
30+
/// say, `ReVar` appearing in a return type and causing ICEs when other
31+
/// functions end up with region constraints involving regions from other
32+
/// functions.
33+
///
34+
/// We also cannot use `for_each_free_region` because for closures it includes
35+
/// the regions parameters from the enclosing item, which we know are always
36+
/// universal, so we don't particularly care about since they're not relevant
37+
/// for opaque type captures or computing liveness.
38+
pub struct FreeRegionsVisitor<'tcx, OP: FnMut(ty::Region<'tcx>)> {
39+
pub tcx: TyCtxt<'tcx>,
40+
pub param_env: ty::ParamEnv<'tcx>,
41+
pub op: OP,
42+
}
43+
44+
impl<'tcx, OP> TypeVisitor<TyCtxt<'tcx>> for FreeRegionsVisitor<'tcx, OP>
45+
where
46+
OP: FnMut(ty::Region<'tcx>),
47+
{
48+
fn visit_binder<T: TypeVisitable<TyCtxt<'tcx>>>(
49+
&mut self,
50+
t: &ty::Binder<'tcx, T>,
51+
) -> ControlFlow<Self::BreakTy> {
52+
t.super_visit_with(self);
53+
ControlFlow::Continue(())
54+
}
55+
56+
fn visit_region(&mut self, r: ty::Region<'tcx>) -> ControlFlow<Self::BreakTy> {
57+
match *r {
58+
// ignore bound regions, keep visiting
59+
ty::ReLateBound(_, _) => ControlFlow::Continue(()),
60+
_ => {
61+
(self.op)(r);
62+
ControlFlow::Continue(())
63+
}
64+
}
65+
}
66+
67+
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
68+
// We're only interested in types involving regions
69+
if !ty.flags().intersects(ty::TypeFlags::HAS_FREE_REGIONS) {
70+
return ControlFlow::Continue(());
71+
}
72+
73+
match ty.kind() {
74+
ty::Closure(_, ref args) => {
75+
// Skip lifetime parameters of the enclosing item(s)
76+
77+
for upvar in args.as_closure().upvar_tys() {
78+
upvar.visit_with(self);
79+
}
80+
args.as_closure().sig_as_fn_ptr_ty().visit_with(self);
81+
}
82+
83+
ty::Generator(_, ref args, _) => {
84+
// Skip lifetime parameters of the enclosing item(s)
85+
// Also skip the witness type, because that has no free regions.
86+
87+
for upvar in args.as_generator().upvar_tys() {
88+
upvar.visit_with(self);
89+
}
90+
args.as_generator().return_ty().visit_with(self);
91+
args.as_generator().yield_ty().visit_with(self);
92+
args.as_generator().resume_ty().visit_with(self);
93+
}
94+
95+
// We can prove that an alias is live two ways:
96+
// 1. All the components are live.
97+
//
98+
// 2. There is a known outlives bound or where-clause, and that
99+
// region is live.
100+
//
101+
// We search through the item bounds and where clauses for
102+
// either `'static` or a unique outlives region, and if one is
103+
// found, we just need to prove that that region is still live.
104+
// If one is not found, then we continue to walk through the alias.
105+
ty::Alias(kind, ty::AliasTy { def_id, args, .. }) => {
106+
let tcx = self.tcx;
107+
let param_env = self.param_env;
108+
let outlives_bounds: Vec<_> = tcx
109+
.item_bounds(def_id)
110+
.iter_instantiated(tcx, args)
111+
.chain(param_env.caller_bounds())
112+
.filter_map(|clause| {
113+
let outlives = clause.as_type_outlives_clause()?;
114+
if let Some(outlives) = outlives.no_bound_vars()
115+
&& outlives.0 == ty
116+
{
117+
Some(outlives.1)
118+
} else {
119+
test_type_match::extract_verify_if_eq(
120+
tcx,
121+
param_env,
122+
&outlives.map_bound(|ty::OutlivesPredicate(ty, bound)| {
123+
VerifyIfEq { ty, bound }
124+
}),
125+
ty,
126+
)
127+
}
128+
})
129+
.collect();
130+
// If we find `'static`, then we know the alias doesn't capture *any* regions.
131+
// Otherwise, all of the outlives regions should be equal -- if they're not,
132+
// we don't really know how to proceed, so we continue recursing through the
133+
// alias.
134+
if outlives_bounds.contains(&tcx.lifetimes.re_static) {
135+
// no
136+
} else if let Some(r) = outlives_bounds.first()
137+
&& outlives_bounds[1..].iter().all(|other_r| other_r == r)
138+
{
139+
assert!(r.type_flags().intersects(ty::TypeFlags::HAS_FREE_REGIONS));
140+
r.visit_with(self)?;
141+
} else {
142+
// Skip lifetime parameters that are not captures.
143+
let variances = match kind {
144+
ty::Opaque => Some(self.tcx.variances_of(*def_id)),
145+
_ => None,
146+
};
147+
148+
for (idx, s) in args.iter().enumerate() {
149+
if variances.map(|variances| variances[idx]) != Some(ty::Variance::Bivariant) {
150+
s.visit_with(self)?;
151+
}
152+
}
153+
}
154+
}
155+
156+
_ => {
157+
ty.super_visit_with(self)?;
158+
}
159+
}
160+
161+
ControlFlow::Continue(())
162+
}
163+
}

compiler/rustc_infer/src/infer/outlives/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use rustc_middle::ty;
99

1010
pub mod components;
1111
pub mod env;
12+
pub mod for_liveness;
1213
pub mod obligations;
1314
pub mod test_type_match;
1415
pub mod verify;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// check-pass
2+
3+
trait Foo {
4+
type Assoc<'a>
5+
where
6+
Self: 'a;
7+
8+
fn assoc(&mut self) -> Self::Assoc<'_>;
9+
}
10+
11+
fn test<T>(mut t: T)
12+
where
13+
T: Foo,
14+
for<'a> T::Assoc<'a>: 'static,
15+
{
16+
let a = t.assoc();
17+
let b = t.assoc();
18+
}
19+
20+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// check-pass
2+
3+
trait Captures<'a> {}
4+
impl<T> Captures<'_> for T {}
5+
6+
trait Outlives<'a>: 'a {}
7+
impl<'a, T: 'a> Outlives<'a> for T {}
8+
9+
// Test that we treat `for<'a> Opaque: 'a` as `Opaque: 'static`
10+
fn test<'o>(v: &'o Vec<i32>) -> impl Captures<'o> + for<'a> Outlives<'a> {}
11+
12+
fn statik() -> impl Sized {
13+
test(&vec![])
14+
}
15+
16+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// check-pass
2+
3+
trait Captures<'a> {}
4+
impl<T> Captures<'_> for T {}
5+
6+
fn captures_temp_early<'a>(x: &'a Vec<i32>) -> impl Sized + Captures<'a> + 'static {}
7+
fn captures_temp_late<'a: 'a>(x: &'a Vec<i32>) -> impl Sized + Captures<'a> + 'static {}
8+
9+
fn test() {
10+
let x = captures_temp_early(&vec![]);
11+
let y = captures_temp_late(&vec![]);
12+
}
13+
14+
fn main() {}

0 commit comments

Comments
 (0)