Skip to content

Commit fdfe819

Browse files
committed
Auto merge of #86701 - sexxi-goose:optimization, r=nikomatsakis
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` } ``` r? `@nikomatsakis`
2 parents 8b87e85 + 38dcae2 commit fdfe819

File tree

8 files changed

+159
-12
lines changed

8 files changed

+159
-12
lines changed

compiler/rustc_typeck/src/check/upvar.rs

+52-2
Original file line numberDiff line numberDiff line change
@@ -1618,11 +1618,17 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
16181618
"consume(place_with_id={:?}, diag_expr_id={:?}, mode={:?})",
16191619
place_with_id, diag_expr_id, mode
16201620
);
1621+
1622+
let place_with_id = PlaceWithHirId {
1623+
place: truncate_capture_for_optimization(&place_with_id.place),
1624+
..*place_with_id
1625+
};
1626+
16211627
if !self.capture_information.contains_key(&place_with_id.place) {
1622-
self.init_capture_info_for_place(place_with_id, diag_expr_id);
1628+
self.init_capture_info_for_place(&place_with_id, diag_expr_id);
16231629
}
16241630

1625-
self.adjust_upvar_borrow_kind_for_consume(place_with_id, diag_expr_id, mode);
1631+
self.adjust_upvar_borrow_kind_for_consume(&place_with_id, diag_expr_id, mode);
16261632
}
16271633

16281634
fn borrow(
@@ -1645,6 +1651,8 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
16451651
&place_with_id.place,
16461652
);
16471653

1654+
let place = truncate_capture_for_optimization(&place);
1655+
16481656
let place_with_id = PlaceWithHirId { place, ..*place_with_id };
16491657

16501658
if !self.capture_information.contains_key(&place_with_id.place) {
@@ -1980,6 +1988,48 @@ fn determine_place_ancestry_relation(
19801988
}
19811989
}
19821990

1991+
/// Reduces the precision of the captured place when the precision doesn't yeild any benefit from
1992+
/// borrow checking prespective, allowing us to save us on the size of the capture.
1993+
///
1994+
///
1995+
/// Fields that are read through a shared reference will always be read via a shared ref or a copy,
1996+
/// and therefore capturing precise paths yields no benefit. This optimization truncates the
1997+
/// rightmost deref of the capture if the deref is applied to a shared ref.
1998+
///
1999+
/// Reason we only drop the last deref is because of the following edge case:
2000+
///
2001+
/// ```rust
2002+
/// struct MyStruct<'a> {
2003+
/// a: &'static A,
2004+
/// b: B,
2005+
/// c: C<'a>,
2006+
/// }
2007+
///
2008+
/// fn foo<'a, 'b>(m: &'a MyStruct<'b>) -> impl FnMut() + 'static {
2009+
/// let c = || drop(&*m.a.field_of_a);
2010+
/// // Here we really do want to capture `*m.a` because that outlives `'static`
2011+
///
2012+
/// // If we capture `m`, then the closure no longer outlives `'static'
2013+
/// // it is constrained to `'a`
2014+
/// }
2015+
/// ```
2016+
fn truncate_capture_for_optimization<'tcx>(place: &Place<'tcx>) -> Place<'tcx> {
2017+
let is_shared_ref = |ty: Ty<'_>| matches!(ty.kind(), ty::Ref(.., hir::Mutability::Not));
2018+
2019+
// Find the right-most deref (if any). All the projections that come after this
2020+
// are fields or other "in-place pointer adjustments"; these refer therefore to
2021+
// data owned by whatever pointer is being dereferenced here.
2022+
let idx = place.projections.iter().rposition(|proj| ProjectionKind::Deref == proj.kind);
2023+
2024+
match idx {
2025+
// If that pointer is a shared reference, then we don't need those fields.
2026+
Some(idx) if is_shared_ref(place.ty_before_projection(idx)) => {
2027+
Place { projections: place.projections[0..=idx].to_vec(), ..place.clone() }
2028+
}
2029+
None | Some(_) => place.clone(),
2030+
}
2031+
}
2032+
19832033
/// Precise capture is enabled if the feature gate `capture_disjoint_fields` is enabled or if
19842034
/// user is using Rust Edition 2021 or higher.
19852035
///

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,34 @@
1+
// edition:2021
2+
3+
#![feature(rustc_attrs)]
4+
#![allow(unused)]
5+
#![allow(dead_code)]
6+
7+
struct Int(i32);
8+
struct B<'a>(&'a i32);
9+
10+
const I : Int = Int(0);
11+
const REF_I : &'static Int = &I;
12+
13+
14+
struct MyStruct<'a> {
15+
a: &'static Int,
16+
b: B<'a>,
17+
}
18+
19+
fn foo<'a, 'b>(m: &'a MyStruct<'b>) -> impl FnMut() + 'static {
20+
let c = #[rustc_capture_analysis] || drop(&m.a.0);
21+
//~^ ERROR: attributes on expressions are experimental
22+
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
23+
//~| ERROR: First Pass analysis includes:
24+
//~| ERROR: Min Capture analysis includes:
25+
//~| NOTE: Capturing m[Deref,(0, 0),Deref] -> ImmBorrow
26+
//~| NOTE: Min Capture m[Deref,(0, 0),Deref] -> ImmBorrow
27+
c
28+
}
29+
30+
fn main() {
31+
let t = 0;
32+
let s = MyStruct { a: REF_I, b: B(&t) };
33+
let _ = foo(&s);
34+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
error[E0658]: attributes on expressions are experimental
2+
--> $DIR/edge_case.rs:20: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+
error: First Pass analysis includes:
11+
--> $DIR/edge_case.rs:20:39
12+
|
13+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
14+
| ^^^^^^^^^^^^^^^
15+
|
16+
note: Capturing m[Deref,(0, 0),Deref] -> ImmBorrow
17+
--> $DIR/edge_case.rs:20:48
18+
|
19+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
20+
| ^^^^^
21+
22+
error: Min Capture analysis includes:
23+
--> $DIR/edge_case.rs:20:39
24+
|
25+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
26+
| ^^^^^^^^^^^^^^^
27+
|
28+
note: Min Capture m[Deref,(0, 0),Deref] -> ImmBorrow
29+
--> $DIR/edge_case.rs:20:48
30+
|
31+
LL | let c = #[rustc_capture_analysis] || drop(&m.a.0);
32+
| ^^^^^
33+
34+
error: aborting due to 3 previous errors
35+
36+
For more information about this error, try `rustc --explain E0658`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// edition:2021
2+
// run-pass
3+
4+
#![allow(unused)]
5+
#![allow(dead_code)]
6+
7+
struct Int(i32);
8+
struct B<'a>(&'a i32);
9+
10+
const I : Int = Int(0);
11+
const REF_I : &'static Int = &I;
12+
13+
struct MyStruct<'a> {
14+
a: &'static Int,
15+
b: B<'a>,
16+
}
17+
18+
fn foo<'a, 'b>(m: &'a MyStruct<'b>) -> impl FnMut() + 'static {
19+
let c = || drop(&m.a.0);
20+
c
21+
}
22+
23+
fn main() {
24+
let t = 0;
25+
let s = MyStruct { a: REF_I, b: B(&t) };
26+
let _ = foo(&s);
27+
}

0 commit comments

Comments
 (0)