From 111201d27c4262d66834e495ffa515792e3a5771 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 12:17:25 +0200 Subject: [PATCH 1/2] Use multi span suggestions for closure migrations. --- compiler/rustc_typeck/src/check/upvar.rs | 90 ++++++++++++++---------- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 46f360075c09d..f4da3d3c8704e 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -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; @@ -645,6 +645,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } diagnostics_builder.note("for more information, see "); + 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 @@ -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(); }, ); From 64310977e6e890563d9dfd6d9b2d6d6442ffd281 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 12:17:35 +0200 Subject: [PATCH 2/2] Update test output. --- .../migrations/auto_traits.stderr | 26 ++++----- .../migrations/insignificant_drop.stderr | 42 +++------------ .../insignificant_drop_attr_migrations.stderr | 12 +---- .../migrations/macro.stderr | 2 +- .../migrations/migrations_rustfix.stderr | 8 +-- .../migrations/mir_calls_to_shims.stderr | 6 +-- .../migrations/multi_diagnostics.stderr | 34 ++++-------- .../migrations/precise.stderr | 12 +---- .../migrations/significant_drop.stderr | 54 ++++--------------- 9 files changed, 44 insertions(+), 152 deletions(-) diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr index e8dd81ede2547..98396abb6ff66 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr @@ -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 @@ -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 @@ -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 diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr index b719b89f5fcbc..7989a8fa5ccae 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr index 669614fee0a93..961834aca194d 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr @@ -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 @@ -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 diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr index 0614b78a7437b..d1f959dfc520e 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr @@ -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 diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.stderr index 8d4819fe2e221..3589a6150d064 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/migrations_rustfix.stderr @@ -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 @@ -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 diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.stderr index b6ee5edb59e79..10816b7bc3adf 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.stderr @@ -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 diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr index b887b212e3d45..8bee950c13eca 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr index 51b4e11819f45..aa9b8672a0ffb 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr @@ -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/precise.rs:45:13 @@ -53,11 +49,7 @@ help: add a dummy let to cause `u` to be fully captured | LL ~ let c = || { LL + let _ = &u; -LL + -LL + -LL + -LL + let _x = u.0.0; - ... + | error: aborting due to 2 previous errors diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr index 81700e32b5f40..e9170eba3f176 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr @@ -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 + let _t = t.0; - ... + | error: changes to closure capture in Rust 2021 will affect drop order --> $DIR/significant_drop.rs:50:13 @@ -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/significant_drop.rs:71:13 @@ -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/significant_drop.rs:91:13 @@ -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/significant_drop.rs:109:13 @@ -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/significant_drop.rs:125:13 @@ -151,11 +131,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.1; - ... + | error: changes to closure capture in Rust 2021 will affect drop order --> $DIR/significant_drop.rs:143:13 @@ -179,11 +155,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/significant_drop.rs:163:21 @@ -202,11 +174,7 @@ help: add a dummy let to cause `tuple` to be fully captured | LL ~ let c = || { LL + let _ = &tuple; -LL + -LL + -LL + -LL + tuple.0; - ... + | error: changes to closure capture in Rust 2021 will affect drop order --> $DIR/significant_drop.rs:181:17 @@ -225,11 +193,7 @@ help: add a dummy let to cause `tuple` to be fully captured | LL ~ let c = || { LL + let _ = &tuple; -LL + -LL + -LL + -LL + tuple.0; - ... + | error: aborting due to 9 previous errors