Skip to content

Commit ec2b311

Browse files
committed
Auto merge of rust-lang#116733 - compiler-errors:alias-liveness-but-this-time-sound, r=aliemjay
Consider alias bounds when computing liveness in NLL (but this time sound hopefully) This is a revival of rust-lang#116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing `ReErased` hack we use instead. r? `@aliemjay` -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :> Fixes rust-lang#116794 as well --- (mostly copied from rust-lang#116040 and reworked slightly) # 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 rust-lang#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. ## Limitation: Multiple choices This approach has some limitations. Firstly, 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. ## Limitation: Opaque hidden types Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc rust-lang#42940) to fail: ```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 {} ``` We need to have existential regions to avoid [unsoundness](rust-lang#116040 (comment)) when an opaque captures a region which is not represented in its own substs but which outlives a region that does. ## Read more Context: rust-lang#115822 (comment) (for the liveness case) More context: rust-lang#42940 (comment) (for the opaque capture case, which this does not fix) [^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 88ae8c9 + f0e385d commit ec2b311

16 files changed

+357
-20
lines changed

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

+23-20
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use rustc_data_structures::graph::WithSuccessors;
33
use rustc_index::bit_set::{HybridBitSet, SparseBitMatrix};
44
use rustc_index::interval::IntervalSet;
55
use rustc_infer::infer::canonical::QueryRegionConstraints;
6+
use rustc_infer::infer::outlives::for_liveness;
67
use rustc_middle::mir::{BasicBlock, Body, ConstraintCategory, Local, Location};
78
use rustc_middle::traits::query::DropckOutlivesResult;
89
use rustc_middle::ty::{RegionVid, Ty, TyCtxt, TypeVisitable, TypeVisitableExt};
@@ -601,34 +602,36 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> {
601602
values::location_set_str(elements, live_at.iter()),
602603
);
603604

604-
let tcx = typeck.tcx();
605-
let borrowck_context = &mut typeck.borrowck_context;
606-
607605
// When using `-Zpolonius=next`, we want to record the loans that flow into this value's
608606
// regions as being live at the given `live_at` points: this will be used to compute the
609607
// location where a loan goes out of scope.
610-
let num_loans = borrowck_context.borrow_set.len();
611-
let mut value_loans = HybridBitSet::new_empty(num_loans);
612-
613-
tcx.for_each_free_region(&value, |live_region| {
614-
let live_region_vid = borrowck_context.universal_regions.to_region_vid(live_region);
615-
616-
borrowck_context
617-
.constraints
618-
.liveness_constraints
619-
.add_elements(live_region_vid, live_at);
620-
621-
// There can only be inflowing loans for this region when we are using
622-
// `-Zpolonius=next`.
623-
if let Some(inflowing) = inflowing_loans.row(live_region_vid) {
624-
value_loans.union(inflowing);
625-
}
608+
let num_loans = typeck.borrowck_context.borrow_set.len();
609+
let value_loans = &mut HybridBitSet::new_empty(num_loans);
610+
611+
value.visit_with(&mut for_liveness::FreeRegionsVisitor {
612+
tcx: typeck.tcx(),
613+
param_env: typeck.param_env,
614+
op: |r| {
615+
let live_region_vid = typeck.borrowck_context.universal_regions.to_region_vid(r);
616+
617+
typeck
618+
.borrowck_context
619+
.constraints
620+
.liveness_constraints
621+
.add_elements(live_region_vid, live_at);
622+
623+
// There can only be inflowing loans for this region when we are using
624+
// `-Zpolonius=next`.
625+
if let Some(inflowing) = inflowing_loans.row(live_region_vid) {
626+
value_loans.union(inflowing);
627+
}
628+
},
626629
});
627630

628631
// Record the loans reaching the value.
629632
if !value_loans.is_empty() {
630633
for point in live_at.iter() {
631-
borrowck_context.live_loans.union_row(point, &value_loans);
634+
typeck.borrowck_context.live_loans.union_row(point, value_loans);
632635
}
633636
}
634637
}

compiler/rustc_infer/src/infer/opaque_types.rs

+2
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,8 @@ impl<'tcx> InferCtxt<'tcx> {
362362
.collect(),
363363
);
364364

365+
// FIXME(#42940): This should use the `FreeRegionsVisitor`, but that's
366+
// not currently sound until we have existential regions.
365367
concrete_ty.visit_with(&mut ConstrainOpaqueTypeRegionVisitor {
366368
tcx: self.tcx,
367369
op: |r| self.member_constraint(opaque_type_key, span, concrete_ty, r, &choice_regions),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
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 if
12+
/// the type is an alias, we look at the outlives bounds in the param-env
13+
/// and alias's item bounds. If there is a unique outlives bound, then visit
14+
/// that instead. If there is not a unique but there is a `'static` outlives
15+
/// bound, then don't visit anything. Otherwise, walk through the opaque's
16+
/// regions structurally.
17+
pub struct FreeRegionsVisitor<'tcx, OP: FnMut(ty::Region<'tcx>)> {
18+
pub tcx: TyCtxt<'tcx>,
19+
pub param_env: ty::ParamEnv<'tcx>,
20+
pub op: OP,
21+
}
22+
23+
impl<'tcx, OP> TypeVisitor<TyCtxt<'tcx>> for FreeRegionsVisitor<'tcx, OP>
24+
where
25+
OP: FnMut(ty::Region<'tcx>),
26+
{
27+
fn visit_binder<T: TypeVisitable<TyCtxt<'tcx>>>(
28+
&mut self,
29+
t: &ty::Binder<'tcx, T>,
30+
) -> ControlFlow<Self::BreakTy> {
31+
t.super_visit_with(self);
32+
ControlFlow::Continue(())
33+
}
34+
35+
fn visit_region(&mut self, r: ty::Region<'tcx>) -> ControlFlow<Self::BreakTy> {
36+
match *r {
37+
// ignore bound regions, keep visiting
38+
ty::ReLateBound(_, _) => ControlFlow::Continue(()),
39+
_ => {
40+
(self.op)(r);
41+
ControlFlow::Continue(())
42+
}
43+
}
44+
}
45+
46+
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
47+
// We're only interested in types involving regions
48+
if !ty.flags().intersects(ty::TypeFlags::HAS_FREE_REGIONS) {
49+
return ControlFlow::Continue(());
50+
}
51+
52+
match ty.kind() {
53+
// We can prove that an alias is live two ways:
54+
// 1. All the components are live.
55+
//
56+
// 2. There is a known outlives bound or where-clause, and that
57+
// region is live.
58+
//
59+
// We search through the item bounds and where clauses for
60+
// either `'static` or a unique outlives region, and if one is
61+
// found, we just need to prove that that region is still live.
62+
// If one is not found, then we continue to walk through the alias.
63+
ty::Alias(kind, ty::AliasTy { def_id, args, .. }) => {
64+
let tcx = self.tcx;
65+
let param_env = self.param_env;
66+
let outlives_bounds: Vec<_> = tcx
67+
.item_bounds(def_id)
68+
.iter_instantiated(tcx, args)
69+
.chain(param_env.caller_bounds())
70+
.filter_map(|clause| {
71+
let outlives = clause.as_type_outlives_clause()?;
72+
if let Some(outlives) = outlives.no_bound_vars()
73+
&& outlives.0 == ty
74+
{
75+
Some(outlives.1)
76+
} else {
77+
test_type_match::extract_verify_if_eq(
78+
tcx,
79+
param_env,
80+
&outlives.map_bound(|ty::OutlivesPredicate(ty, bound)| {
81+
VerifyIfEq { ty, bound }
82+
}),
83+
ty,
84+
)
85+
}
86+
})
87+
.collect();
88+
// If we find `'static`, then we know the alias doesn't capture *any* regions.
89+
// Otherwise, all of the outlives regions should be equal -- if they're not,
90+
// we don't really know how to proceed, so we continue recursing through the
91+
// alias.
92+
if outlives_bounds.contains(&tcx.lifetimes.re_static) {
93+
// no
94+
} else if let Some(r) = outlives_bounds.first()
95+
&& outlives_bounds[1..].iter().all(|other_r| other_r == r)
96+
{
97+
assert!(r.type_flags().intersects(ty::TypeFlags::HAS_FREE_REGIONS));
98+
r.visit_with(self)?;
99+
} else {
100+
// Skip lifetime parameters that are not captures.
101+
let variances = match kind {
102+
ty::Opaque => Some(self.tcx.variances_of(*def_id)),
103+
_ => None,
104+
};
105+
106+
for (idx, s) in args.iter().enumerate() {
107+
if variances.map(|variances| variances[idx]) != Some(ty::Variance::Bivariant) {
108+
s.visit_with(self)?;
109+
}
110+
}
111+
}
112+
}
113+
114+
_ => {
115+
ty.super_visit_with(self)?;
116+
}
117+
}
118+
119+
ControlFlow::Continue(())
120+
}
121+
}

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,29 @@
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 overlapping_mut<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 live_past_borrow<T>(mut t: T)
21+
where
22+
T: Foo,
23+
for<'a> T::Assoc<'a>: 'static {
24+
let x = t.assoc();
25+
drop(t);
26+
drop(x);
27+
}
28+
29+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// known-bug: #42940
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,16 @@
1+
error[E0716]: temporary value dropped while borrowed
2+
--> $DIR/higher-ranked-outlives-for-capture.rs:13:11
3+
|
4+
LL | test(&vec![])
5+
| ------^^^^^^-
6+
| | |
7+
| | creates a temporary value which is freed while still in use
8+
| argument requires that borrow lasts for `'static`
9+
LL | }
10+
| - temporary value is freed at the end of this statement
11+
|
12+
= note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)
13+
14+
error: aborting due to previous error
15+
16+
For more information about this error, try `rustc --explain E0716`.
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 opaque_doesnt_use_temporary() {
13+
let a = test(&vec![]);
14+
}
15+
16+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// check-pass
2+
3+
// Check that opaques capturing early and late-bound vars correctly mark
4+
// regions required to be live using the item bounds.
5+
6+
trait Captures<'a> {}
7+
impl<T> Captures<'_> for T {}
8+
9+
fn captures_temp_late<'a>(x: &'a Vec<i32>) -> impl Sized + Captures<'a> + 'static {}
10+
fn captures_temp_early<'a: 'a>(x: &'a Vec<i32>) -> impl Sized + Captures<'a> + 'static {}
11+
12+
fn test() {
13+
let x = captures_temp_early(&vec![]);
14+
let y = captures_temp_late(&vec![]);
15+
}
16+
17+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// known-bug: #42940
2+
3+
trait Trait {}
4+
impl Trait for () {}
5+
6+
fn foo<'a>(s: &'a str) -> impl Trait + 'static {
7+
bar(s)
8+
}
9+
10+
fn bar<P: AsRef<str>>(s: P) -> impl Trait + 'static {
11+
()
12+
}
13+
14+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error[E0700]: hidden type for `impl Trait + 'static` captures lifetime that does not appear in bounds
2+
--> $DIR/opaque-type-param.rs:7:5
3+
|
4+
LL | fn foo<'a>(s: &'a str) -> impl Trait + 'static {
5+
| -- -------------------- opaque type defined here
6+
| |
7+
| hidden type `impl Trait + 'static` captures the lifetime `'a` as defined here
8+
LL | bar(s)
9+
| ^^^^^^
10+
11+
error: aborting due to previous error
12+
13+
For more information about this error, try `rustc --explain E0700`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// check-pass
2+
3+
trait Captures<'a> {}
4+
impl<T> Captures<'_> for T {}
5+
6+
fn foo(x: &mut i32) -> impl Sized + Captures<'_> + 'static {}
7+
8+
fn overlapping_mut() {
9+
let i = &mut 1;
10+
let x = foo(i);
11+
let y = foo(i);
12+
}
13+
14+
fn live_past_borrow() {
15+
let y;
16+
{
17+
let x = &mut 1;
18+
y = foo(x);
19+
}
20+
}
21+
22+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// check-pass
2+
3+
trait Foo {
4+
fn rpitit(&mut self) -> impl Sized + 'static;
5+
}
6+
7+
fn live_past_borrow<T: Foo>(mut t: T) {
8+
let x = t.rpitit();
9+
drop(t);
10+
drop(x);
11+
}
12+
13+
fn overlapping_mut<T: Foo>(mut t: T) {
14+
let a = t.rpitit();
15+
let b = t.rpitit();
16+
}
17+
18+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// check-pass
2+
3+
#![feature(return_type_notation)]
4+
//~^ WARN the feature `return_type_notation` is incomplete
5+
6+
trait Foo {
7+
fn borrow(&mut self) -> impl Sized + '_;
8+
}
9+
10+
fn live_past_borrow<T: Foo<borrow(): 'static>>(mut t: T) {
11+
let x = t.borrow();
12+
drop(t);
13+
drop(x);
14+
}
15+
16+
// Test that the `'_` item bound in `borrow` does not cause us to
17+
// overlook the `'static` RTN bound.
18+
fn overlapping_mut<T: Foo<borrow(): 'static>>(mut t: T) {
19+
let x = t.borrow();
20+
let x = t.borrow();
21+
}
22+
23+
fn main() {}

0 commit comments

Comments
 (0)