Skip to content

Commit

Permalink
Auto merge of #86965 - sexxi-goose:rfc2229-improve-lint, r=nikomatsak…
Browse files Browse the repository at this point in the history 
…is,lqd

Improves migrations lint for RFC2229

This PR improves the current disjoint capture migration lint by providing more information on why drop order or auto trait implementation for a closure is impacted by the use of the new feature.

The drop order migration lint will now look something like this:
```
error: changes to closure capture in Rust 2021 will affect drop order
  --> $DIR/significant_drop.rs:163:21
   |
LL |             let c = || {
   |                     ^^
...
LL |                 tuple.0;
   |                 ------- in Rust 2018, closure captures all of `tuple`, but in Rust 2021, it only captures `tuple.0`
...
LL |         }
   |         - in Rust 2018, `tuple` would be dropped here, but in Rust 2021, only `tuple.0` would be dropped here alongside the closure
```

The auto trait migration lint will now look something like this:
```
error: changes to closure capture in Rust 2021 will affect `Send` trait implementation for closure
  --> $DIR/auto_traits.rs:14:19
   |
LL |     thread::spawn(move || unsafe {
   |                   ^^^^^^^^^^^^^^ in Rust 2018, this closure would implement `Send` as `fptr` implements `Send`, but in Rust 2021, this closure would no longer implement `Send` as `fptr.0` does not implement `Send`
...
LL |         *fptr.0 = 20;
   |         ------- in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0`
```

r? `@nikomatsakis`

Closes rust-lang/project-rfc-2229#54
  • Loading branch information
@bors
bors committed Jul 11, 2021
2 parents 99f8efe + 08c6167 commit 9f2e753
Show file tree
Hide file tree
Showing 25 changed files with 1,274 additions and 444 deletions.
397 changes: 243 additions & 154 deletions compiler/rustc_typeck/src/check/upvar.rs
View file

Large diffs are not rendered by default.

17 changes: 14 additions & 3 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// run-rustfix
#![deny(rust_2021_incompatible_closure_captures)]
//~^ NOTE: the lint level is defined here

use std::thread;

Expand All @@ -11,9 +12,12 @@ fn test_send_trait() {
let mut f = 10;
let fptr = SendPointer(&mut f as *mut i32);
thread::spawn(move || { let _ = &fptr; unsafe {
//~^ ERROR: `Send` trait implementation
//~^ ERROR: `Send` trait implementation for closure
//~| NOTE: in Rust 2018, this closure would implement `Send` as `fptr` implements `Send`, but in Rust 2021, this closure would no longer implement `Send` as `fptr.0` does not implement `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0 = 20;
//~^ NOTE: in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0`
} });
}

Expand All @@ -28,9 +32,12 @@ fn test_sync_trait() {
let f = CustomInt(&mut f as *mut i32);
let fptr = SyncPointer(f);
thread::spawn(move || { let _ = &fptr; unsafe {
//~^ ERROR: `Sync`, `Send` trait implementation
//~^ ERROR: `Sync`, `Send` trait implementation for closure
//~| NOTE: in Rust 2018, this closure would implement `Sync`, `Send` as `fptr` implements `Sync`, `Send`, but in Rust 2021, this closure would no longer implement `Sync`, `Send` as `fptr.0.0` does not implement `Sync`, `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0.0 = 20;
//~^ NOTE: in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0.0`
} });
}

Expand All @@ -49,16 +56,20 @@ impl Clone for U {
fn test_clone_trait() {
let f = U(S(String::from("Hello World")), T(0));
let c = || { let _ = &f;
//~^ ERROR: `Clone` trait implementation, and drop order
//~^ ERROR: `Clone` trait implementation for closure and drop order
//~| NOTE: in Rust 2018, this closure would implement `Clone` as `f` implements `Clone`, but in Rust 2021, this closure would no longer implement `Clone` as `f.1` does not implement `Clone`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `f` to be fully captured
let f_1 = f.1;
//~^ NOTE: in Rust 2018, closure captures all of `f`, but in Rust 2021, it only captures `f.1`
println!("{:?}", f_1.0);
};

let c_clone = c.clone();

c_clone();
}
//~^ NOTE: in Rust 2018, `f` would be dropped here, but in Rust 2021, only `f.1` would be dropped here alongside the closure

fn main() {
test_send_trait();
Expand Down
17 changes: 14 additions & 3 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// run-rustfix
#![deny(rust_2021_incompatible_closure_captures)]
//~^ NOTE: the lint level is defined here

use std::thread;

Expand All @@ -11,9 +12,12 @@ fn test_send_trait() {
let mut f = 10;
let fptr = SendPointer(&mut f as *mut i32);
thread::spawn(move || unsafe {
//~^ ERROR: `Send` trait implementation
//~^ ERROR: `Send` trait implementation for closure
//~| NOTE: in Rust 2018, this closure would implement `Send` as `fptr` implements `Send`, but in Rust 2021, this closure would no longer implement `Send` as `fptr.0` does not implement `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0 = 20;
//~^ NOTE: in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0`
});
}

Expand All @@ -28,9 +32,12 @@ fn test_sync_trait() {
let f = CustomInt(&mut f as *mut i32);
let fptr = SyncPointer(f);
thread::spawn(move || unsafe {
//~^ ERROR: `Sync`, `Send` trait implementation
//~^ ERROR: `Sync`, `Send` trait implementation for closure
//~| NOTE: in Rust 2018, this closure would implement `Sync`, `Send` as `fptr` implements `Sync`, `Send`, but in Rust 2021, this closure would no longer implement `Sync`, `Send` as `fptr.0.0` does not implement `Sync`, `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0.0 = 20;
//~^ NOTE: in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0.0`
});
}

Expand All @@ -49,16 +56,20 @@ impl Clone for U {
fn test_clone_trait() {
let f = U(S(String::from("Hello World")), T(0));
let c = || {
//~^ ERROR: `Clone` trait implementation, and drop order
//~^ ERROR: `Clone` trait implementation for closure and drop order
//~| NOTE: in Rust 2018, this closure would implement `Clone` as `f` implements `Clone`, but in Rust 2021, this closure would no longer implement `Clone` as `f.1` does not implement `Clone`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `f` to be fully captured
let f_1 = f.1;
//~^ NOTE: in Rust 2018, closure captures all of `f`, but in Rust 2021, it only captures `f.1`
println!("{:?}", f_1.0);
};

let c_clone = c.clone();

c_clone();
}
//~^ NOTE: in Rust 2018, `f` would be dropped here, but in Rust 2021, only `f.1` would be dropped here alongside the closure

fn main() {
test_send_trait();
Expand Down
68 changes: 33 additions & 35 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
error: `Send` trait implementation will change in Rust 2021
--> $DIR/auto_traits.rs:13:19
error: changes to closure capture in Rust 2021 will affect `Send` trait implementation for closure
--> $DIR/auto_traits.rs:14:19
|
LL | thread::spawn(move || unsafe {
| ___________________^
LL | |
LL | |
LL | | *fptr.0 = 20;
LL | | });
| |_____^
LL | thread::spawn(move || unsafe {
| ^^^^^^^^^^^^^^ in Rust 2018, this closure would implement `Send` as `fptr` implements `Send`, but in Rust 2021, this closure would no longer implement `Send` as `fptr.0` does not implement `Send`
...
LL | *fptr.0 = 20;
| ------- in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0`
|
note: the lint level is defined here
--> $DIR/auto_traits.rs:2:9
Expand All @@ -20,53 +18,53 @@ help: add a dummy let to cause `fptr` to be fully captured
LL | thread::spawn(move || { let _ = &fptr; unsafe {
LL |
LL |
LL |
LL |
LL | *fptr.0 = 20;
LL | } });
|
...

error: `Sync`, `Send` trait implementation will change in Rust 2021
--> $DIR/auto_traits.rs:30:19
error: changes to closure capture in Rust 2021 will affect `Sync`, `Send` trait implementation for closure
--> $DIR/auto_traits.rs:34:19
|
LL | thread::spawn(move || unsafe {
| ___________________^
LL | |
LL | |
LL | | *fptr.0.0 = 20;
LL | | });
| |_____^
LL | thread::spawn(move || unsafe {
| ^^^^^^^^^^^^^^ in Rust 2018, this closure would implement `Sync`, `Send` as `fptr` implements `Sync`, `Send`, but in Rust 2021, this closure would no longer implement `Sync`, `Send` as `fptr.0.0` does not implement `Sync`, `Send`
...
LL | *fptr.0.0 = 20;
| --------- in Rust 2018, closure captures all of `fptr`, but in Rust 2021, it only captures `fptr.0.0`
|
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
help: add a dummy let to cause `fptr` to be fully captured
|
LL | thread::spawn(move || { let _ = &fptr; unsafe {
LL |
LL |
LL |
LL |
LL | *fptr.0.0 = 20;
LL | } });
|
...

error: `Clone` trait implementation, and drop order will change in Rust 2021
--> $DIR/auto_traits.rs:51:13
error: changes to closure capture in Rust 2021 will affect `Clone` trait implementation for closure and drop order
--> $DIR/auto_traits.rs:58:13
|
LL | let c = || {
| _____________^
LL | |
LL | |
LL | | let f_1 = f.1;
LL | | println!("{:?}", f_1.0);
LL | | };
| |_____^
LL | let c = || {
| ^^ in Rust 2018, this closure would implement `Clone` as `f` implements `Clone`, but in Rust 2021, this closure would no longer implement `Clone` as `f.1` does not implement `Clone`
...
LL | let f_1 = f.1;
| --- in Rust 2018, closure captures all of `f`, but in Rust 2021, it only captures `f.1`
...
LL | }
| - in Rust 2018, `f` would be dropped here, but in Rust 2021, only `f.1` would be dropped here alongside the closure
|
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
help: add a dummy let to cause `f` to be fully captured
|
LL | let c = || { let _ = &f;
LL |
LL |
LL |
LL |
LL | let f_1 = f.1;
LL | println!("{:?}", f_1.0);
LL | };
|
...

error: aborting due to 3 previous errors

22 changes: 22 additions & 0 deletions src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,18 @@ fn test1_all_need_migration() {
//~| HELP: add a dummy let to cause `t`, `t1`, `t2` to be fully captured

let _t = t.0;
//~^ NOTE: in Rust 2018, closure captures all of `t`, but in Rust 2021, it only captures `t.0`
let _t1 = t1.0;
//~^ NOTE: in Rust 2018, closure captures all of `t1`, but in Rust 2021, it only captures `t1.0`
let _t2 = t2.0;
//~^ NOTE: in Rust 2018, closure captures all of `t2`, but in Rust 2021, it only captures `t2.0`
};

c();
}
//~^ in Rust 2018, `t` would be dropped here, but in Rust 2021, only `t.0` would be dropped here alongside the closure
//~| in Rust 2018, `t1` would be dropped here, but in Rust 2021, only `t1.0` would be dropped here alongside the closure
//~| in Rust 2018, `t2` would be dropped here, but in Rust 2021, only `t2.0` would be dropped here alongside the closure

// String implements drop and therefore should be migrated.
// But in this test cases, `t2` is completely captured and when it is dropped won't be affected
Expand All @@ -37,12 +43,16 @@ fn test2_only_precise_paths_need_migration() {
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `t`, `t1` to be fully captured
let _t = t.0;
//~^ NOTE: in Rust 2018, closure captures all of `t`, but in Rust 2021, it only captures `t.0`
let _t1 = t1.0;
//~^ NOTE: in Rust 2018, closure captures all of `t1`, but in Rust 2021, it only captures `t1.0`
let _t2 = t2;
};

c();
}
//~^ in Rust 2018, `t` would be dropped here, but in Rust 2021, only `t.0` would be dropped here alongside the closure
//~| in Rust 2018, `t1` would be dropped here, but in Rust 2021, only `t1.0` would be dropped here alongside the closure

// If a variable would've not been captured by value then it would've not been
// dropped with the closure and therefore doesn't need migration.
Expand All @@ -54,11 +64,13 @@ fn test3_only_by_value_need_migration() {
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.0;
//~^ NOTE: in Rust 2018, closure captures all of `t`, but in Rust 2021, it only captures `t.0`
println!("{}", t1.1);
};

c();
}
//~^ in Rust 2018, `t` would be dropped here, but in Rust 2021, only `t.0` would be dropped here alongside the closure

// Copy types get copied into the closure instead of move. Therefore we don't need to
// migrate then as their drop order isn't tied to the closure.
Expand All @@ -73,11 +85,13 @@ fn test4_only_non_copy_types_need_migration() {
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.0;
//~^ NOTE: in Rust 2018, closure captures all of `t`, but in Rust 2021, it only captures `t.0`
let _t1 = t1.0;
};

c();
}
//~^ in Rust 2018, `t` would be dropped here, but in Rust 2021, only `t.0` would be dropped here alongside the closure

fn test5_only_drop_types_need_migration() {
struct S(i32, i32);
Expand All @@ -92,11 +106,13 @@ fn test5_only_drop_types_need_migration() {
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.0;
//~^ NOTE: in Rust 2018, closure captures all of `t`, but in Rust 2021, it only captures `t.0`
let _s = s.0;
};

c();
}
//~^ in Rust 2018, `t` would be dropped here, but in Rust 2021, only `t.0` would be dropped here alongside the closure

// Since we are using a move closure here, both `t` and `t1` get moved
// even though they are being used by ref inside the closure.
Expand All @@ -108,10 +124,14 @@ fn test6_move_closures_non_copy_types_might_need_migration() {
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `t1`, `t` to be fully captured
println!("{} {}", t1.1, t.1);
//~^ NOTE: in Rust 2018, closure captures all of `t`, but in Rust 2021, it only captures `t.1`
//~| NOTE: in Rust 2018, closure captures all of `t1`, but in Rust 2021, it only captures `t1.1`
};

c();
}
//~^ in Rust 2018, `t` would be dropped here, but in Rust 2021, only `t.1` would be dropped here alongside the closure
//~| in Rust 2018, `t1` would be dropped here, but in Rust 2021, only `t1.1` would be dropped here alongside the closure

// Test migration analysis in case of Drop + Non Drop aggregates.
// Note we need migration here only because the non-copy (because Drop type) is captured,
Expand All @@ -124,10 +144,12 @@ fn test7_drop_non_drop_aggregate_need_migration() {
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.0;
//~^ NOTE: in Rust 2018, closure captures all of `t`, but in Rust 2021, it only captures `t.0`
};

c();
}
//~^ in Rust 2018, `t` would be dropped here, but in Rust 2021, only `t.0` would be dropped here alongside the closure

fn main() {
test1_all_need_migration();
Expand Down
Loading

0 comments on commit 9f2e753

Please sign in to comment.