Skip to content

Commit a721957

Browse files
arora-amanarora-aman
committed
Don't introduce a block if a block exists
1 parent da86348 commit a721957

File tree

10 files changed

+189
-63
lines changed

10 files changed

+189
-63
lines changed

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -495,10 +495,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
495495
let closure_body_span = self.tcx.hir().span(body_id.hir_id);
496496
let (sugg, app) =
497497
match self.tcx.sess.source_map().span_to_snippet(closure_body_span) {
498-
Ok(s) => (
499-
format!("{{ {}; {} }}", migration_string, s),
500-
Applicability::MachineApplicable,
501-
),
498+
Ok(s) => {
499+
let trimmed = s.trim_start();
500+
501+
// If the closure contains a block then replace the opening brace
502+
// with "{ let _ = (..); "
503+
let sugg = if let Some('{') = trimmed.chars().next() {
504+
format!("{{ {}; {}", migration_string, &trimmed[1..])
505+
} else {
506+
format!("{{ {}; {} }}", migration_string, s)
507+
};
508+
(sugg, Applicability::MachineApplicable)
509+
}
502510
Err(_) => (migration_string.clone(), Applicability::HasPlaceholders),
503511
};
504512

‎src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.fixed‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@ fn test1_all_need_migration() {
1212
let t1 = (String::new(), String::new());
1313
let t2 = (String::new(), String::new());
1414

15-
let c = || { let _ = (&t, &t1, &t2); {
15+
let c = || { let _ = (&t, &t1, &t2);
1616
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
1717
//~| HELP:` let _ = (&t, &t1, &t2)` causes `t`, `t1`, `t2` to be fully captured
1818

1919
let _t = t.0;
2020
let _t1 = t1.0;
2121
let _t2 = t2.0;
22-
} };
22+
};
2323

2424
c();
2525
}
@@ -31,13 +31,13 @@ fn test2_only_precise_paths_need_migration() {
3131
let t1 = (String::new(), String::new());
3232
let t2 = (String::new(), String::new());
3333

34-
let c = || { let _ = (&t, &t1); {
34+
let c = || { let _ = (&t, &t1);
3535
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
3636
//~| HELP:` let _ = (&t, &t1)` causes `t`, `t1` to be fully captured
3737
let _t = t.0;
3838
let _t1 = t1.0;
3939
let _t2 = t2;
40-
} };
40+
};
4141

4242
c();
4343
}
@@ -47,12 +47,12 @@ fn test2_only_precise_paths_need_migration() {
4747
fn test3_only_by_value_need_migration() {
4848
let t = (String::new(), String::new());
4949
let t1 = (String::new(), String::new());
50-
let c = || { let _ = &t; {
50+
let c = || { let _ = &t;
5151
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
5252
//~| HELP: `let _ = &t` causes `t` to be fully captured
5353
let _t = t.0;
5454
println!("{}", t1.1);
55-
} };
55+
};
5656

5757
c();
5858
}
@@ -65,12 +65,12 @@ fn test4_only_non_copy_types_need_migration() {
6565
// `t1` is Copy because all of its elements are Copy
6666
let t1 = (0i32, 0i32);
6767

68-
let c = || { let _ = &t; {
68+
let c = || { let _ = &t;
6969
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
7070
//~| HELP: `let _ = &t` causes `t` to be fully captured
7171
let _t = t.0;
7272
let _t1 = t1.0;
73-
} };
73+
};
7474

7575
c();
7676
}
@@ -83,12 +83,12 @@ fn test5_only_drop_types_need_migration() {
8383
// `s` doesn't implement Drop or any elements within it, and doesn't need migration
8484
let s = S(0i32, 0i32);
8585

86-
let c = || { let _ = &t; {
86+
let c = || { let _ = &t;
8787
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
8888
//~| HELP: `let _ = &t` causes `t` to be fully captured
8989
let _t = t.0;
9090
let _s = s.0;
91-
} };
91+
};
9292

9393
c();
9494
}
@@ -98,11 +98,11 @@ fn test5_only_drop_types_need_migration() {
9898
fn test6_move_closures_non_copy_types_might_need_migration() {
9999
let t = (String::new(), String::new());
100100
let t1 = (String::new(), String::new());
101-
let c = move || { let _ = (&t1, &t); {
101+
let c = move || { let _ = (&t1, &t);
102102
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
103103
//~| HELP: `let _ = (&t1, &t)` causes `t1`, `t` to be fully captured
104104
println!("{} {}", t1.1, t.1);
105-
} };
105+
};
106106

107107
c();
108108
}
@@ -113,11 +113,11 @@ fn test6_move_closures_non_copy_types_might_need_migration() {
113113
fn test7_drop_non_drop_aggregate_need_migration() {
114114
let t = (String::new(), String::new(), 0i32);
115115

116-
let c = || { let _ = &t; {
116+
let c = || { let _ = &t;
117117
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
118118
//~| HELP: `let _ = &t` causes `t` to be fully captured
119119
let _t = t.0;
120-
} };
120+
};
121121

122122
c();
123123
}

‎src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ LL | #![deny(disjoint_capture_drop_reorder)]
1818
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1919
help: `let _ = (&t, &t1, &t2)` causes `t`, `t1`, `t2` to be fully captured
2020
|
21-
LL | let c = || { let _ = (&t, &t1, &t2); {
21+
LL | let c = || { let _ = (&t, &t1, &t2);
2222
LL |
2323
LL |
2424
LL |
@@ -41,7 +41,7 @@ LL | | };
4141
|
4242
help: `let _ = (&t, &t1)` causes `t`, `t1` to be fully captured
4343
|
44-
LL | let c = || { let _ = (&t, &t1); {
44+
LL | let c = || { let _ = (&t, &t1);
4545
LL |
4646
LL |
4747
LL | let _t = t.0;
@@ -63,12 +63,12 @@ LL | | };
6363
|
6464
help: `let _ = &t` causes `t` to be fully captured
6565
|
66-
LL | let c = || { let _ = &t; {
66+
LL | let c = || { let _ = &t;
6767
LL |
6868
LL |
6969
LL | let _t = t.0;
7070
LL | println!("{}", t1.1);
71-
LL | } };
71+
LL | };
7272
|
7373

7474
error: drop order affected for closure because of `capture_disjoint_fields`
@@ -85,12 +85,12 @@ LL | | };
8585
|
8686
help: `let _ = &t` causes `t` to be fully captured
8787
|
88-
LL | let c = || { let _ = &t; {
88+
LL | let c = || { let _ = &t;
8989
LL |
9090
LL |
9191
LL | let _t = t.0;
9292
LL | let _t1 = t1.0;
93-
LL | } };
93+
LL | };
9494
|
9595

9696
error: drop order affected for closure because of `capture_disjoint_fields`
@@ -107,12 +107,12 @@ LL | | };
107107
|
108108
help: `let _ = &t` causes `t` to be fully captured
109109
|
110-
LL | let c = || { let _ = &t; {
110+
LL | let c = || { let _ = &t;
111111
LL |
112112
LL |
113113
LL | let _t = t.0;
114114
LL | let _s = s.0;
115-
LL | } };
115+
LL | };
116116
|
117117

118118
error: drop order affected for closure because of `capture_disjoint_fields`
@@ -128,11 +128,11 @@ LL | | };
128128
|
129129
help: `let _ = (&t1, &t)` causes `t1`, `t` to be fully captured
130130
|
131-
LL | let c = move || { let _ = (&t1, &t); {
131+
LL | let c = move || { let _ = (&t1, &t);
132132
LL |
133133
LL |
134134
LL | println!("{} {}", t1.1, t.1);
135-
LL | } };
135+
LL | };
136136
|
137137

138138
error: drop order affected for closure because of `capture_disjoint_fields`
@@ -148,11 +148,11 @@ LL | | };
148148
|
149149
help: `let _ = &t` causes `t` to be fully captured
150150
|
151-
LL | let c = || { let _ = &t; {
151+
LL | let c = || { let _ = &t;
152152
LL |
153153
LL |
154154
LL | let _t = t.0;
155-
LL | } };
155+
LL | };
156156
|
157157

158158
error: aborting due to 7 previous errors

‎src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.fixed‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// run-rustfix
2+
#![deny(disjoint_capture_drop_reorder)]
3+
//~^ NOTE: the lint level is defined here
4+
5+
// Test the two possible cases for automated migartion using rustfix
6+
// - Closure contains a block i.e. `|| { .. };`
7+
// - Closure contains just an expr `|| ..;`
8+
9+
#[derive(Debug)]
10+
struct Foo(i32);
11+
impl Drop for Foo {
12+
fn drop(&mut self) {
13+
println!("{:?} dropped", self.0);
14+
}
15+
}
16+
17+
fn closure_contains_block() {
18+
let t = (Foo(0), Foo(0));
19+
let c = || { let _ = &t;
20+
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
21+
//~| HELP: `let _ = &t` causes `t` to be fully captured
22+
let _t = t.0;
23+
};
24+
25+
c();
26+
}
27+
28+
fn closure_doesnt_contain_block() {
29+
let t = (Foo(0), Foo(0));
30+
let c = || { let _ = &t; t.0 };
31+
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
32+
//~| HELP: `let _ = &t` causes `t` to be fully captured
33+
34+
c();
35+
}
36+
37+
fn main() {
38+
closure_contains_block();
39+
closure_doesnt_contain_block();
40+
}

‎src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// run-rustfix
2+
#![deny(disjoint_capture_drop_reorder)]
3+
//~^ NOTE: the lint level is defined here
4+
5+
// Test the two possible cases for automated migartion using rustfix
6+
// - Closure contains a block i.e. `|| { .. };`
7+
// - Closure contains just an expr `|| ..;`
8+
9+
#[derive(Debug)]
10+
struct Foo(i32);
11+
impl Drop for Foo {
12+
fn drop(&mut self) {
13+
println!("{:?} dropped", self.0);
14+
}
15+
}
16+
17+
fn closure_contains_block() {
18+
let t = (Foo(0), Foo(0));
19+
let c = || {
20+
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
21+
//~| HELP: `let _ = &t` causes `t` to be fully captured
22+
let _t = t.0;
23+
};
24+
25+
c();
26+
}
27+
28+
fn closure_doesnt_contain_block() {
29+
let t = (Foo(0), Foo(0));
30+
let c = || t.0;
31+
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
32+
//~| HELP: `let _ = &t` causes `t` to be fully captured
33+
34+
c();
35+
}
36+
37+
fn main() {
38+
closure_contains_block();
39+
closure_doesnt_contain_block();
40+
}

‎src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.stderr‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
error: drop order affected for closure because of `capture_disjoint_fields`
2+
--> $DIR/migrations_rustfix.rs:19:13
3+
|
4+
LL | let c = || {
5+
| _____________^
6+
LL | |
7+
LL | |
8+
LL | | let _t = t.0;
9+
LL | | };
10+
| |_____^
11+
|
12+
note: the lint level is defined here
13+
--> $DIR/migrations_rustfix.rs:2:9
14+
|
15+
LL | #![deny(disjoint_capture_drop_reorder)]
16+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
help: `let _ = &t` causes `t` to be fully captured
18+
|
19+
LL | let c = || { let _ = &t;
20+
LL |
21+
LL |
22+
LL | let _t = t.0;
23+
LL | };
24+
|
25+
26+
error: drop order affected for closure because of `capture_disjoint_fields`
27+
--> $DIR/migrations_rustfix.rs:30:13
28+
|
29+
LL | let c = || t.0;
30+
| ^^^^^^
31+
|
32+
help: `let _ = &t` causes `t` to be fully captured
33+
|
34+
LL | let c = || { let _ = &t; t.0 };
35+
| ^^^^^^^^^^^^^^^^^^^
36+
37+
error: aborting due to 2 previous errors
38+

‎src/test/ui/closures/2229_closure_analysis/migrations/precise.fixed‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ struct ConstainsDropField(Foo, Foo);
1616
fn test_precise_analysis_drop_paths_not_captured_by_move() {
1717
let t = ConstainsDropField(Foo(10), Foo(20));
1818

19-
let c = || { let _ = &t; {
19+
let c = || { let _ = &t;
2020
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
2121
//~| HELP: `let _ = &t` causes `t` to be fully captured
2222
let _t = t.0;
2323
let _t = &t.1;
24-
} };
24+
};
2525

2626
c();
2727
}
@@ -39,13 +39,13 @@ struct U(T, T);
3939
fn test_precise_analysis_long_path_missing() {
4040
let u = U(T(S, S), T(S, S));
4141

42-
let c = || { let _ = &u; {
42+
let c = || { let _ = &u;
4343
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
4444
//~| HELP: `let _ = &u` causes `u` to be fully captured
4545
let _x = u.0.0;
4646
let _x = u.0.1;
4747
let _x = u.1.0;
48-
} };
48+
};
4949

5050
c();
5151
}

‎src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@ LL | #![deny(disjoint_capture_drop_reorder)]
1717
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1818
help: `let _ = &t` causes `t` to be fully captured
1919
|
20-
LL | let c = || { let _ = &t; {
20+
LL | let c = || { let _ = &t;
2121
LL |
2222
LL |
2323
LL | let _t = t.0;
2424
LL | let _t = &t.1;
25-
LL | } };
25+
LL | };
2626
|
2727

2828
error: drop order affected for closure because of `capture_disjoint_fields`
@@ -40,7 +40,7 @@ LL | | };
4040
|
4141
help: `let _ = &u` causes `u` to be fully captured
4242
|
43-
LL | let c = || { let _ = &u; {
43+
LL | let c = || { let _ = &u;
4444
LL |
4545
LL |
4646
LL | let _x = u.0.0;

0 commit comments

Comments
 (0)