Skip to content

Commit 23c0334

Browse files
committed
2229: Reduce the size of closures with capture_disjoint_fields
One key observation while going over the closure size profile of rustc was that we are disjointly capturing one or more fields starting at an immutable reference. Disjoint capture over immutable reference doesn't add too much value because the fields can either be borrowed immutably or copied. One possible edge case of the optimization is when a fields of a struct have a longer lifetime than the structure, therefore we can't completely get rid of all the accesses on top of sharef refs, only the rightmost one. Here is a possible example: ```rust struct MyStruct<'a> { a: &'static A, b: B, c: C<'a>, } fn foo<'a, 'b>(m: &'a MyStruct<'b>) -> impl FnMut() + 'static { let c = || drop(&*m.a.field_of_a); // Here we really do want to capture `*m.a` because that outlives `'static` // If we capture `m`, then the closure no longer outlives `'static' // it is constrained to `'a` } ```
1 parent fecc65a commit 23c0334

File tree

9 files changed

+182
-12
lines changed

9 files changed

+182
-12
lines changed

compiler/rustc_typeck/src/check/upvar.rs

+48-2
Original file line numberDiff line numberDiff line change
@@ -1609,11 +1609,17 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
16091609
"consume(place_with_id={:?}, diag_expr_id={:?}, mode={:?})",
16101610
place_with_id, diag_expr_id, mode
16111611
);
1612+
1613+
let place_with_id = PlaceWithHirId {
1614+
place: truncate_capture_for_optimization(&place_with_id.place),
1615+
..*place_with_id
1616+
};
1617+
16121618
if !self.capture_information.contains_key(&place_with_id.place) {
1613-
self.init_capture_info_for_place(place_with_id, diag_expr_id);
1619+
self.init_capture_info_for_place(&place_with_id, diag_expr_id);
16141620
}
16151621

1616-
self.adjust_upvar_borrow_kind_for_consume(place_with_id, diag_expr_id, mode);
1622+
self.adjust_upvar_borrow_kind_for_consume(&place_with_id, diag_expr_id, mode);
16171623
}
16181624

16191625
fn borrow(
@@ -1636,6 +1642,8 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
16361642
&place_with_id.place,
16371643
);
16381644

1645+
let place = truncate_capture_for_optimization(&place);
1646+
16391647
let place_with_id = PlaceWithHirId { place, ..*place_with_id };
16401648

16411649
if !self.capture_information.contains_key(&place_with_id.place) {
@@ -1967,6 +1975,44 @@ fn determine_place_ancestry_relation(
19671975
}
19681976
}
19691977

1978+
/// Reduces the precision of the captured place when the precision doesn't yeild any benefit from
1979+
/// borrow checking prespective, allowing us to save us on the size of the capture.
1980+
///
1981+
///
1982+
/// Fields that are read through a shared reference will always be read via a shared ref or a copy,
1983+
/// and therefore capturing precise paths yields no benefit. This optimization truncates the
1984+
/// rightmost deref of the capture if the deref is applied to a shared ref.
1985+
///
1986+
/// Reason we only drop the last deref is because of the following edge case:
1987+
///
1988+
/// ```rust
1989+
/// struct MyStruct<'a> {
1990+
/// a: &'static A,
1991+
/// b: B,
1992+
/// c: C<'a>,
1993+
/// }
1994+
///
1995+
/// fn foo<'a, 'b>(m: &'a MyStruct<'b>) -> impl FnMut() + 'static {
1996+
/// let c = || drop(&*m.a.field_of_a);
1997+
/// // Here we really do want to capture `*m.a` because that outlives `'static`
1998+
///
1999+
/// // If we capture `m`, then the closure no longer outlives `'static'
2000+
/// // it is constrained to `'a`
2001+
/// }
2002+
/// ```
2003+
fn truncate_capture_for_optimization<'tcx>(place: &Place<'tcx>) -> Place<'tcx> {
2004+
let is_shared_ref = |ty: Ty<'_>| matches!(ty.kind(), ty::Ref(.., hir::Mutability::Not));
2005+
2006+
let idx = place.projections.iter().rposition(|proj| ProjectionKind::Deref == proj.kind);
2007+
2008+
match idx {
2009+
Some(idx) if is_shared_ref(place.ty_before_projection(idx)) => {
2010+
Place { projections: place.projections[0..=idx].to_vec(), ..place.clone() }
2011+
}
2012+
None | Some(_) => place.clone(),
2013+
}
2014+
}
2015+
19702016
/// Precise capture is enabled if the feature gate `capture_disjoint_fields` is enabled or if
19712017
/// user is using Rust Edition 2021 or higher.
19722018
///

src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ fn main() {
88
let mut y = (&x, "Y");
99
let z = (&mut y, "Z");
1010

11-
// `x.0` is mutable but we access `x` via `z.0.0`, which is an immutable reference and
11+
// `x.0` is mutable but we access `x` via `*z.0.0`, which is an immutable reference and
1212
// therefore can't be mutated.
1313
let mut c = || {
14-
//~^ ERROR: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference
14+
//~^ ERROR: cannot borrow `*z.0.0` as mutable, as it is behind a `&` reference
1515
z.0.0.0 = format!("X1");
1616
};
1717

src/test/ui/closures/2229_closure_analysis/diagnostics/cant-mutate-imm-borrow.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
error[E0596]: cannot borrow `z.0.0.0` as mutable, as it is behind a `&` reference
1+
error[E0596]: cannot borrow `*z.0.0` as mutable, as it is behind a `&` reference
22
--> $DIR/cant-mutate-imm-borrow.rs:13:17
33
|
44
LL | let mut c = || {
55
| ^^ cannot borrow as mutable
66
LL |
77
LL | z.0.0.0 = format!("X1");
8-
| ------- mutable borrow occurs due to use of `z.0.0.0` in closure
8+
| ------- mutable borrow occurs due to use of `*z.0.0` in closure
99

1010
error: aborting due to previous error
1111

src/test/ui/closures/2229_closure_analysis/move_closure.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ fn struct_contains_ref_to_another_struct_2() {
7878
//~^ ERROR: First Pass analysis includes:
7979
//~| ERROR: Min Capture analysis includes:
8080
let _t = t.0.0;
81-
//~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow
82-
//~| NOTE: Min Capture t[(0, 0),Deref,(0, 0)] -> ImmBorrow
81+
//~^ NOTE: Capturing t[(0, 0),Deref] -> ImmBorrow
82+
//~| NOTE: Min Capture t[(0, 0),Deref] -> ImmBorrow
8383
};
8484

8585
c();
@@ -100,7 +100,7 @@ fn struct_contains_ref_to_another_struct_3() {
100100
//~^ ERROR: First Pass analysis includes:
101101
//~| ERROR: Min Capture analysis includes:
102102
let _t = t.0.0;
103-
//~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow
103+
//~^ NOTE: Capturing t[(0, 0),Deref] -> ImmBorrow
104104
//~| NOTE: Capturing t[(0, 0)] -> ByValue
105105
//~| NOTE: Min Capture t[(0, 0)] -> ByValue
106106
};

src/test/ui/closures/2229_closure_analysis/move_closure.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ LL | |
190190
LL | | };
191191
| |_____^
192192
|
193-
note: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow
193+
note: Capturing t[(0, 0),Deref] -> ImmBorrow
194194
--> $DIR/move_closure.rs:80:18
195195
|
196196
LL | let _t = t.0.0;
@@ -208,7 +208,7 @@ LL | |
208208
LL | | };
209209
| |_____^
210210
|
211-
note: Min Capture t[(0, 0),Deref,(0, 0)] -> ImmBorrow
211+
note: Min Capture t[(0, 0),Deref] -> ImmBorrow
212212
--> $DIR/move_closure.rs:80:18
213213
|
214214
LL | let _t = t.0.0;
@@ -226,7 +226,7 @@ LL | |
226226
LL | | };
227227
| |_____^
228228
|
229-
note: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow
229+
note: Capturing t[(0, 0),Deref] -> ImmBorrow
230230
--> $DIR/move_closure.rs:102:18
231231
|
232232
LL | let _t = t.0.0;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#![feature(capture_disjoint_fields)]
2+
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
3+
//~| NOTE: `#[warn(incomplete_features)]` on by default
4+
//~| NOTE: see issue #53488 <https://github.com/rust-lang/rust/issues/53488>
5+
6+
#![feature(rustc_attrs)]
7+
#![allow(unused)]
8+
#![allow(dead_code)]
9+
10+
struct Int(i32);
11+
struct B<'a>(&'a i32);
12+
13+
const I : Int = Int(0);
14+
const REF_I : &'static Int = &I;
15+
16+
17+
struct MyStruct<'a> {
18+
a: &'static Int,
19+
b: B<'a>,
20+
}
21+
22+
fn foo<'a, 'b>(m: &'a MyStruct<'b>) -> impl FnMut() + 'static {
23+
let c = #[rustc_capture_analysis] || drop(&m.a.0);
24+
//~^ ERROR: attributes on expressions are experimental
25+
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
26+
//~| ERROR: First Pass analysis includes:
27+
//~| ERROR: Min Capture analysis includes:
28+
//~| NOTE: Capturing m[Deref,(0, 0),Deref] -> ImmBorrow
29+
//~| NOTE: Min Capture m[Deref,(0, 0),Deref] -> ImmBorrow
30+
c
31+
}
32+
33+
fn main() {
34+
let t = 0;
35+
let s = MyStruct { a: REF_I, b: B(&t) };
36+
let _ = foo(&s);
37+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
error[E0658]: attributes on expressions are experimental
2+
--> $DIR/edge_case.rs:23:13
3+
|
4+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
8+
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
9+
10+
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
11+
--> $DIR/edge_case.rs:1:12
12+
|
13+
LL | #![feature(capture_disjoint_fields)]
14+
| ^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= note: `#[warn(incomplete_features)]` on by default
17+
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
18+
19+
error: First Pass analysis includes:
20+
--> $DIR/edge_case.rs:23:39
21+
|
22+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
23+
| ^^^^^^^^^^^^^^^
24+
|
25+
note: Capturing m[Deref,(0, 0),Deref] -> ImmBorrow
26+
--> $DIR/edge_case.rs:23:48
27+
|
28+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
29+
| ^^^^^
30+
31+
error: Min Capture analysis includes:
32+
--> $DIR/edge_case.rs:23:39
33+
|
34+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
35+
| ^^^^^^^^^^^^^^^
36+
|
37+
note: Min Capture m[Deref,(0, 0),Deref] -> ImmBorrow
38+
--> $DIR/edge_case.rs:23:48
39+
|
40+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
41+
| ^^^^^
42+
43+
error: aborting due to 3 previous errors; 1 warning emitted
44+
45+
For more information about this error, try `rustc --explain E0658`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#![feature(capture_disjoint_fields)]
2+
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
3+
//~| NOTE: `#[warn(incomplete_features)]` on by default
4+
//~| NOTE: see issue #53488 <https://github.com/rust-lang/rust/issues/53488>
5+
6+
// run-pass
7+
8+
#![allow(unused)]
9+
#![allow(dead_code)]
10+
11+
struct Int(i32);
12+
struct B<'a>(&'a i32);
13+
14+
const I : Int = Int(0);
15+
const REF_I : &'static Int = &I;
16+
17+
struct MyStruct<'a> {
18+
a: &'static Int,
19+
b: B<'a>,
20+
}
21+
22+
fn foo<'a, 'b>(m: &'a MyStruct<'b>) -> impl FnMut() + 'static {
23+
let c = || drop(&m.a.0);
24+
c
25+
}
26+
27+
fn main() {
28+
let t = 0;
29+
let s = MyStruct { a: REF_I, b: B(&t) };
30+
let _ = foo(&s);
31+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
2+
--> $DIR/edge_case_run_pass.rs:1:12
3+
|
4+
LL | #![feature(capture_disjoint_fields)]
5+
| ^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(incomplete_features)]` on by default
8+
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
9+
10+
warning: 1 warning emitted
11+

0 commit comments

Comments
 (0)