Skip to content

Commit

Permalink
Rollup merge of #87958 - m-ou-se:closure-migration-multispan-suggesti…
Browse files Browse the repository at this point in the history 
…ons, r=estebank

Closure migration multispan suggestions

This changes the `rust_2021_incompatible_closure_captures` lint to only suggest inserting the parts that need to be inserted, instead of suggesting to replace the entire closure by an almost identical closure with one statement added.

Before:
```
[...]
help: add a dummy let to cause `a` to be fully captured
   |
5  ~     let _ = || {
6  +         let _ = &a;
7  +         dbg!(a.0);
8  +         println!("1");
9  +         println!("2");
10 +         println!("3");
 ...
   |

[...]
help: add a dummy let to cause `b` to be fully captured
   |
14 |     let _ = || { let _ = &b; dbg!(b.0); };
   |                ~~~~~~~~~~~~~~~~~~~~~~~~~~

[...]
help: add a dummy let to cause `c` to be fully captured
   |
16 |     let _ = || { let _ = &c; dbg!(c.0) };
   |                ~~~~~~~~~~~~~~~~~~~~~~~~~
```

After:
```
[...]
help: add a dummy let to cause `a` to be fully captured
   |
5  ~     let _ = || {
6  +         let _ = &a;
   |

[...]
help: add a dummy let to cause `b` to be fully captured
   |
14 |     let _ = || { let _ = &b; dbg!(b.0); };
   |                  +++++++++++

[...]
help: add a dummy let to cause `c` to be fully captured
   |
16 |     let _ = || { let _ = &c; dbg!(c.0) };
   |                +++++++++++++           +
```
  • Loading branch information
@m-ou-se
m-ou-se committed Aug 16, 2021
2 parents 84ca374 + 6431097 commit d7df1b1
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 189 deletions.
90 changes: 53 additions & 37 deletions compiler/rustc_typeck/src/check/upvar.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use rustc_middle::ty::{
};
use rustc_session::lint;
use rustc_span::sym;
use rustc_span::{MultiSpan, Span, Symbol, DUMMY_SP};
use rustc_span::{BytePos, MultiSpan, Pos, Span, Symbol, DUMMY_SP};
use rustc_trait_selection::infer::InferCtxtExt;

use rustc_data_structures::stable_map::FxHashMap;
Expand Down Expand Up @@ -645,6 +645,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
diagnostics_builder.note("for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>");

let diagnostic_msg = format!(
"add a dummy let to cause {} to be fully captured",
migrated_variables_concat
);

let mut closure_body_span = self.tcx.hir().span(body_id.hir_id);

// If the body was entirely expanded from a macro
Expand All @@ -655,43 +660,54 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
closure_body_span = closure_body_span.parent().unwrap_or(DUMMY_SP);
}

let (span, sugg, app) =
match self.tcx.sess.source_map().span_to_snippet(closure_body_span) {
Ok(s) => {
let trimmed = s.trim_start();
let mut lines = trimmed.lines();
let line1 = lines.next().unwrap_or_default();

// If the closure contains a block then replace the opening brace
// with "{ let _ = (..); "
let sugg = if line1.trim_end() == "{" {
// This is a multi-line closure with just a `{` on the first line,
// so we put the `let` on its own line.
// We take the indentation from the next non-empty line.
let line2 = lines.filter(|line| !line.is_empty()).next().unwrap_or_default();
let indent = line2.split_once(|c: char| !c.is_whitespace()).unwrap_or_default().0;
format!("{{\n{}{};{}", indent, migration_string, &trimmed[line1.len()..])
} else if line1.starts_with('{') {
format!("{{ {}; {}", migration_string, &trimmed[1..].trim_start())
} else {
format!("{{ {}; {} }}", migration_string, s)
};
(closure_body_span, sugg, Applicability::MachineApplicable)
}
Err(_) => (closure_span, migration_string.clone(), Applicability::HasPlaceholders),
};

let diagnostic_msg = format!(
"add a dummy let to cause {} to be fully captured",
migrated_variables_concat
);
if let Ok(s) = self.tcx.sess.source_map().span_to_snippet(closure_body_span) {
let mut lines = s.lines();
let line1 = lines.next().unwrap_or_default();

if line1.trim_end() == "{" {
// This is a multi-line closure with just a `{` on the first line,
// so we put the `let` on its own line.
// We take the indentation from the next non-empty line.
let line2 = lines.filter(|line| !line.is_empty()).next().unwrap_or_default();
let indent = line2.split_once(|c: char| !c.is_whitespace()).unwrap_or_default().0;
diagnostics_builder.span_suggestion(
closure_body_span.with_lo(closure_body_span.lo() + BytePos::from_usize(line1.len())).shrink_to_lo(),
&diagnostic_msg,
format!("\n{}{};", indent, migration_string),
Applicability::MachineApplicable,
);
} else if line1.starts_with('{') {
// This is a closure with its body wrapped in
// braces, but with more than just the opening
// brace on the first line. We put the `let`
// directly after the `{`.
diagnostics_builder.span_suggestion(
closure_body_span.with_lo(closure_body_span.lo() + BytePos(1)).shrink_to_lo(),
&diagnostic_msg,
format!(" {};", migration_string),
Applicability::MachineApplicable,
);
} else {
// This is a closure without braces around the body.
// We add braces to add the `let` before the body.
diagnostics_builder.multipart_suggestion(
&diagnostic_msg,
vec![
(closure_body_span.shrink_to_lo(), format!("{{ {}; ", migration_string)),
(closure_body_span.shrink_to_hi(), " }".to_string()),
],
Applicability::MachineApplicable
);
}
} else {
diagnostics_builder.span_suggestion(
closure_span,
&diagnostic_msg,
migration_string,
Applicability::HasPlaceholders
);
}

diagnostics_builder.span_suggestion(
span,
&diagnostic_msg,
sugg,
app,
);
diagnostics_builder.emit();
},
);
Expand Down
26 changes: 11 additions & 15 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ LL | #![deny(rust_2021_incompatible_closure_captures)]
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 |
LL |
LL |
LL |
LL | *fptr.0 = 20;
...

error: changes to closure capture in Rust 2021 will affect `Sync`, `Send` trait implementation for closure
Expand All @@ -36,11 +36,11 @@ LL | *fptr.0.0 = 20;
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 |
LL |
LL |
LL |
LL | *fptr.0.0 = 20;
...

error: changes to closure capture in Rust 2021 will affect `Clone` trait implementation for closure and drop order
Expand All @@ -60,11 +60,7 @@ help: add a dummy let to cause `f` to be fully captured
|
LL ~ let c = || {
LL + let _ = &f;
LL +
LL +
LL +
LL +
...
|

error: aborting due to 3 previous errors

42 changes: 7 additions & 35 deletions src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ help: add a dummy let to cause `t`, `t1`, `t2` to be fully captured
|
LL ~ let c = || {
LL + let _ = (&t, &t1, &t2);
LL +
LL +
LL +
LL +
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/insignificant_drop.rs:41:13
Expand All @@ -59,11 +55,7 @@ help: add a dummy let to cause `t`, `t1` to be fully captured
|
LL ~ let c = || {
LL + let _ = (&t, &t1);
LL +
LL +
LL +
LL + let _t = t.0;
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/insignificant_drop.rs:62:13
Expand All @@ -82,11 +74,7 @@ help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = || {
LL + let _ = &t;
LL +
LL +
LL +
LL + let _t = t.0;
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/insignificant_drop.rs:83:13
Expand All @@ -105,11 +93,7 @@ help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = || {
LL + let _ = &t;
LL +
LL +
LL +
LL + let _t = t.0;
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/insignificant_drop.rs:104:13
Expand All @@ -128,11 +112,7 @@ help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = || {
LL + let _ = &t;
LL +
LL +
LL +
LL + let _t = t.0;
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/insignificant_drop.rs:122:13
Expand All @@ -156,11 +136,7 @@ help: add a dummy let to cause `t1`, `t` to be fully captured
|
LL ~ let c = move || {
LL + let _ = (&t1, &t);
LL +
LL +
LL +
LL + println!("{} {}", t1.1, t.1);
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/insignificant_drop.rs:142:13
Expand All @@ -179,11 +155,7 @@ help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = || {
LL + let _ = &t;
LL +
LL +
LL +
LL + let _t = t.0;
...
|

error: aborting due to 7 previous errors

12 changes: 2 additions & 10 deletions ...st/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = || {
LL + let _ = &t;
LL +
LL +
LL +
LL + let _t = t.0;
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/insignificant_drop_attr_migrations.rs:57:13
Expand All @@ -43,11 +39,7 @@ help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = move || {
LL + let _ = &t;
LL +
LL +
LL +
LL + let _t = t.1;
...
|

error: aborting due to 2 previous errors

2 changes: 1 addition & 1 deletion src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ LL | #![deny(rust_2021_incompatible_closure_captures)]
help: add a dummy let to cause `a` to be fully captured
|
LL | let _ = || { let _ = &a; dbg!(a.0) };
| ~~~~~~~~~~~~~~~~~~~~~~~~~
| +++++++++++++ +

error: aborting due to previous error

8 changes: 2 additions & 6 deletions src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = || {
LL + let _ = &t;
LL +
LL +
LL +
LL + let _t = t.0;
...
|

error: changes to closure capture in Rust 2021 will affect drop order
--> $DIR/migrations_rustfix.rs:33:13
Expand All @@ -41,7 +37,7 @@ LL | }
help: add a dummy let to cause `t` to be fully captured
|
LL | let c = || { let _ = &t; t.0 };
| ~~~~~~~~~~~~~~~~~~~
| +++++++++++++ +

error: aborting due to 2 previous errors

6 changes: 1 addition & 5 deletions src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ help: add a dummy let to cause `f` to be fully captured
|
LL ~ let result = panic::catch_unwind(move || {
LL + let _ = &f;
LL +
LL +
LL +
LL +
...
|

error: aborting due to previous error

34 changes: 9 additions & 25 deletions src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,7 @@ help: add a dummy let to cause `f1`, `f2` to be fully captured
|
LL ~ let c = || {
LL + let _ = (&f1, &f2);
LL +
LL +
LL +
LL +
...
|

error: changes to closure capture in Rust 2021 will affect `Clone` trait implementation for closure
--> $DIR/multi_diagnostics.rs:42:13
Expand All @@ -43,11 +39,7 @@ help: add a dummy let to cause `f1` to be fully captured
|
LL ~ let c = || {
LL + let _ = &f1;
LL +
LL +
LL +
LL +
...
|

error: changes to closure capture in Rust 2021 will affect `Clone` trait implementation for closure
--> $DIR/multi_diagnostics.rs:67:13
Expand All @@ -69,11 +61,7 @@ help: add a dummy let to cause `f1` to be fully captured
|
LL ~ let c = || {
LL + let _ = &f1;
LL +
LL +
LL +
LL +
...
|

error: changes to closure capture in Rust 2021 will affect `Clone` trait implementation for closure and drop order
--> $DIR/multi_diagnostics.rs:86:13
Expand All @@ -98,11 +86,7 @@ help: add a dummy let to cause `f1` to be fully captured
|
LL ~ let c = || {
LL + let _ = &f1;
LL +
LL +
LL +
LL +
...
|

error: changes to closure capture in Rust 2021 will affect `Sync`, `Send` trait implementation for closure
--> $DIR/multi_diagnostics.rs:119:19
Expand All @@ -123,11 +107,11 @@ LL | *fptr2.0 = 20;
help: add a dummy let to cause `fptr1`, `fptr2` to be fully captured
|
LL ~ thread::spawn(move || { let _ = (&fptr1, &fptr2); unsafe {
LL +
LL +
LL +
LL +
LL +
LL |
LL |
LL |
LL |
LL |
...

error: aborting due to 5 previous errors
Expand Down
Loading

0 comments on commit d7df1b1

Please sign in to comment.