Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit b6cf070

Browse files
roxeloroxelo
committed
Attempt to deal with nested closures properly
1 parent d4f8729 commit b6cf070

12 files changed

+560
-15
lines changed

‎compiler/rustc_typeck/src/check/upvar.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,7 +1153,6 @@ struct InferBorrowKind<'a, 'tcx> {
11531153
/// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow }
11541154
/// ```
11551155
capture_information: InferredCaptureInformation<'tcx>,
1156-
// [FIXME] RFC2229 Change Vec to FxHashSet
11571156
fake_reads: FxHashSet<Place<'tcx>>, // these need to be fake read.
11581157
}
11591158

@@ -1416,9 +1415,9 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
14161415
}
14171416

14181417
impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
1419-
fn fake_read(&mut self, place: PlaceWithHirId<'tcx>) {
1420-
if let PlaceBase::Upvar(_) = place.place.base {
1421-
self.fake_reads.insert(place.place);
1418+
fn fake_read(&mut self, place: Place<'tcx>) {
1419+
if let PlaceBase::Upvar(_) = place.base {
1420+
self.fake_reads.insert(place);
14221421
}
14231422
}
14241423

‎compiler/rustc_typeck/src/expr_use_visitor.rs

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
pub use self::ConsumeMode::*;
66

77
// Export these here so that Clippy can use them.
8-
pub use rustc_middle::hir::place::{PlaceBase, PlaceWithHirId, Projection};
8+
pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection};
99

1010
use rustc_hir as hir;
1111
use rustc_hir::def::Res;
@@ -54,7 +54,7 @@ pub trait Delegate<'tcx> {
5454
fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId);
5555

5656
// [FIXME] RFC2229 This should also affect clippy ref: https://github.com/sexxi-goose/rust/pull/27
57-
fn fake_read(&mut self, place: PlaceWithHirId<'tcx>);
57+
fn fake_read(&mut self, place: Place<'tcx>);
5858
}
5959

6060
#[derive(Copy, Clone, PartialEq, Debug)]
@@ -558,7 +558,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
558558
fn walk_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) {
559559
debug!("walk_pat(discr_place={:?}, pat={:?})", discr_place, pat);
560560

561-
self.delegate.fake_read(discr_place.clone());
561+
self.delegate.fake_read(discr_place.place.clone());
562562

563563
let tcx = self.tcx();
564564
let ExprUseVisitor { ref mc, body_owner: _, ref mut delegate } = *self;
@@ -620,8 +620,6 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
620620
/// - When reporting the Place back to the Delegate, ensure that the UpvarId uses the enclosing
621621
/// closure as the DefId.
622622
fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) {
623-
debug!("walk_captures({:?})", closure_expr);
624-
625623
// Over here we walk a closure that is nested inside the current body
626624
// If the current body is a closure, then we also want to report back any fake reads,
627625
// starting off of variables that are captured by our parent as well.
@@ -635,6 +633,32 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
635633
ty::Closure(..) | ty::Generator(..)
636634
);
637635

636+
// [FIXME] RFC2229 Closures within closures don't work
637+
if let Some(fake_reads) = self.mc.typeck_results.closure_fake_reads.get(&closure_def_id) {
638+
for fake_read in fake_reads.iter() {
639+
// Use this as a reference for if we should promote the fake read
640+
match fake_read.base {
641+
PlaceBase::Upvar(upvar_id) => {
642+
if upvars.map_or(body_owner_is_closure, |upvars| {
643+
!upvars.contains_key(&upvar_id.var_path.hir_id)
644+
}) {
645+
// The nested closure might be capturing the current (enclosing) closure's local variables.
646+
// We check if the root variable is ever mentioned within the enclosing closure, if not
647+
// then for the current body (if it's a closure) these aren't captures, we will ignore them.
648+
continue;
649+
}
650+
}
651+
_ => {
652+
bug!(
653+
"Do not know how to get HirId out of Rvalue and StaticItem {:?}",
654+
fake_read.base
655+
);
656+
}
657+
};
658+
self.delegate.fake_read(fake_read.clone());
659+
}
660+
}
661+
638662
if let Some(min_captures) = self.mc.typeck_results.closure_min_captures.get(&closure_def_id)
639663
{
640664
for (var_hir_id, min_list) in min_captures.iter() {
@@ -664,12 +688,6 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
664688
place.projections.clone(),
665689
);
666690

667-
// [FIXME] RFC2229 We want to created another loop that iterates mc.typeck_results.fake_reads()
668-
// [FIXME] RFC2229 Add tests for nested closures
669-
if body_owner_is_closure {
670-
self.delegate.fake_read(place_with_id.clone());
671-
}
672-
673691
match capture_info.capture_kind {
674692
ty::UpvarCapture::ByValue(_) => {
675693
let mode = copy_or_move(&self.mc, &place_with_id);

‎src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![feature(capture_disjoint_fields)]
2+
#![feature(rustc_attrs)]
3+
4+
fn main() {
5+
let _z = 9;
6+
let t = (String::from("Hello"), String::from("World"));
7+
let g = (String::from("Mr"), String::from("Goose"));
8+
9+
let a = #[rustc_capture_analysis] || {
10+
let (_, g2) = g;
11+
println!("{}", g2);
12+
let c = #[rustc_capture_analysis] || {
13+
let (_, t2) = t;
14+
println!("{}", t2);
15+
};
16+
17+
c();
18+
};
19+
20+
a();
21+
}

‎src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
error[E0658]: attributes on expressions are experimental
2+
--> $DIR/destructure-pattern-closure-within-closure.rs:9:13
3+
|
4+
LL | let a = #[rustc_capture_analysis] || {
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[E0658]: attributes on expressions are experimental
11+
--> $DIR/destructure-pattern-closure-within-closure.rs:12:17
12+
|
13+
LL | let c = #[rustc_capture_analysis] || {
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
17+
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
18+
19+
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
20+
--> $DIR/destructure-pattern-closure-within-closure.rs:1:12
21+
|
22+
LL | #![feature(capture_disjoint_fields)]
23+
| ^^^^^^^^^^^^^^^^^^^^^^^
24+
|
25+
= note: `#[warn(incomplete_features)]` on by default
26+
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information
27+
28+
error: First Pass analysis includes:
29+
--> $DIR/destructure-pattern-closure-within-closure.rs:12:43
30+
|
31+
LL | let c = #[rustc_capture_analysis] || {
32+
| ___________________________________________^
33+
LL | | let (_, t2) = t;
34+
LL | | println!("{}", t2);
35+
LL | | };
36+
| |_________^
37+
|
38+
note: Capturing t[(1, 0)] -> ByValue
39+
--> $DIR/destructure-pattern-closure-within-closure.rs:13:27
40+
|
41+
LL | let (_, t2) = t;
42+
| ^
43+
44+
error: Min Capture analysis includes:
45+
--> $DIR/destructure-pattern-closure-within-closure.rs:12:43
46+
|
47+
LL | let c = #[rustc_capture_analysis] || {
48+
| ___________________________________________^
49+
LL | | let (_, t2) = t;
50+
LL | | println!("{}", t2);
51+
LL | | };
52+
| |_________^
53+
|
54+
note: Min Capture t[(1, 0)] -> ByValue
55+
--> $DIR/destructure-pattern-closure-within-closure.rs:13:27
56+
|
57+
LL | let (_, t2) = t;
58+
| ^
59+
60+
error: First Pass analysis includes:
61+
--> $DIR/destructure-pattern-closure-within-closure.rs:9:39
62+
|
63+
LL | let a = #[rustc_capture_analysis] || {
64+
| _______________________________________^
65+
LL | | let (_, g2) = g;
66+
LL | | println!("{}", g2);
67+
LL | | let c = #[rustc_capture_analysis] || {
68+
... |
69+
LL | | c();
70+
LL | | };
71+
| |_____^
72+
|
73+
note: Capturing g[(1, 0)] -> ByValue
74+
--> $DIR/destructure-pattern-closure-within-closure.rs:10:23
75+
|
76+
LL | let (_, g2) = g;
77+
| ^
78+
note: Capturing t[(1, 0)] -> ByValue
79+
--> $DIR/destructure-pattern-closure-within-closure.rs:13:27
80+
|
81+
LL | let (_, t2) = t;
82+
| ^
83+
84+
error: Min Capture analysis includes:
85+
--> $DIR/destructure-pattern-closure-within-closure.rs:9:39
86+
|
87+
LL | let a = #[rustc_capture_analysis] || {
88+
| _______________________________________^
89+
LL | | let (_, g2) = g;
90+
LL | | println!("{}", g2);
91+
LL | | let c = #[rustc_capture_analysis] || {
92+
... |
93+
LL | | c();
94+
LL | | };
95+
| |_____^
96+
|
97+
note: Min Capture g[(1, 0)] -> ByValue
98+
--> $DIR/destructure-pattern-closure-within-closure.rs:10:23
99+
|
100+
LL | let (_, g2) = g;
101+
| ^
102+
note: Min Capture t[(1, 0)] -> ByValue
103+
--> $DIR/destructure-pattern-closure-within-closure.rs:13:27
104+
|
105+
LL | let (_, t2) = t;
106+
| ^
107+
108+
error: aborting due to 6 previous errors; 1 warning emitted
109+
110+
For more information about this error, try `rustc --explain E0658`.

‎src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
//check-pass
2+
#![feature(capture_disjoint_fields)]
3+
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
4+
#![warn(unused)]
5+
6+
struct Point {
7+
x: u32,
8+
y: u32,
9+
}
10+
11+
fn test1() {
12+
let _z = 9;
13+
let t = (String::from("Hello"), String::from("World"));
14+
15+
let c = || {
16+
let (t1, t2) = t;
17+
println!("{} {}", t1, t2);
18+
};
19+
20+
c();
21+
}
22+
23+
fn test2() {
24+
let _z = 9;
25+
let t = (String::from("Hello"), String::from("World"));
26+
27+
let c = || {
28+
let (t1, _) = t;
29+
println!("{}", t1);
30+
};
31+
32+
c();
33+
}
34+
35+
fn test3() {
36+
let _z = 9;
37+
let t = (String::from("Hello"), String::from("World"));
38+
39+
let c = || {
40+
let (_, t2) = t;
41+
println!("{}", t2);
42+
};
43+
44+
c();
45+
}
46+
47+
fn test4() {
48+
let _z = 9;
49+
let t = (String::from("Hello"), String::from("World"));
50+
//~^ WARN unused variable: `t`
51+
52+
let c = || {
53+
let (_, _) = t;
54+
};
55+
56+
c();
57+
}
58+
59+
fn test5() {
60+
let _z = 9;
61+
let t = (String::new(), String::new());
62+
let _c = || {
63+
let _a = match t {
64+
(t1, _) => t1,
65+
};
66+
};
67+
}
68+
69+
fn test6() {
70+
let _z = 9;
71+
let t = (String::new(), String::new());
72+
let _c = || {
73+
let _a = match t {
74+
(_, t2) => t2,
75+
};
76+
};
77+
}
78+
79+
fn test7() {
80+
let x = 0;
81+
//~^ WARN unused variable: `x`
82+
let tup = (1, 2);
83+
//~^ WARN unused variable: `tup`
84+
let p = Point { x: 10, y: 20 };
85+
86+
let c = || {
87+
let _ = x;
88+
let Point { x, y } = p; // 1
89+
//~^ WARN unused variable: `x`
90+
println!("{}", y);
91+
let (_, _) = tup; // 2
92+
};
93+
94+
c();
95+
}
96+
97+
fn test8() {
98+
let _z = 9;
99+
let t = (String::from("Hello"), String::from("World"));
100+
101+
let c = || {
102+
let (_, t) = t;
103+
println!("{}", t);
104+
};
105+
106+
c();
107+
}
108+
109+
fn main() {
110+
test1();
111+
test2();
112+
test3();
113+
test4();
114+
test5();
115+
test6();
116+
test7();
117+
test8();
118+
}

‎src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
2+
--> $DIR/destructure_patterns-1.rs:2: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: unused variable: `t`
11+
--> $DIR/destructure_patterns-1.rs:49:9
12+
|
13+
LL | let t = (String::from("Hello"), String::from("World"));
14+
| ^ help: if this is intentional, prefix it with an underscore: `_t`
15+
|
16+
note: the lint level is defined here
17+
--> $DIR/destructure_patterns-1.rs:4:9
18+
|
19+
LL | #![warn(unused)]
20+
| ^^^^^^
21+
= note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`
22+
23+
warning: unused variable: `x`
24+
--> $DIR/destructure_patterns-1.rs:88:21
25+
|
26+
LL | let Point { x, y } = p; // 1
27+
| ^ help: try ignoring the field: `x: _`
28+
29+
warning: unused variable: `x`
30+
--> $DIR/destructure_patterns-1.rs:80:9
31+
|
32+
LL | let x = 0;
33+
| ^ help: if this is intentional, prefix it with an underscore: `_x`
34+
35+
warning: unused variable: `tup`
36+
--> $DIR/destructure_patterns-1.rs:82:9
37+
|
38+
LL | let tup = (1, 2);
39+
| ^^^ help: if this is intentional, prefix it with an underscore: `_tup`
40+
41+
warning: 5 warnings emitted
42+

0 commit comments

Comments
 (0)