From 945a4b18d949719a1ad737d6d39dda717d62c001 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 11:57:32 +0200 Subject: [PATCH 1/4] Fix closure migration suggestion when the body is a macro. --- compiler/rustc_typeck/src/check/upvar.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 013cb2a49b2..862b650efd7 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}; +use rustc_span::{MultiSpan, Span, Symbol, DUMMY_SP}; use rustc_trait_selection::infer::InferCtxtExt; use rustc_data_structures::stable_map::FxHashMap; @@ -644,7 +644,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } diagnostics_builder.note("for more information, see "); - let closure_body_span = self.tcx.hir().span(body_id.hir_id); + + let mut closure_body_span = self.tcx.hir().span(body_id.hir_id); + + // If the body was entirely expanded from a macro + // invocation, as the body is not contained inside the + // closure span. In that case, we walk up the expansion + // until we find the span before the expansion. + while !closure_body_span.is_dummy() && !closure_span.contains(closure_body_span) { + closure_body_span = closure_body_span.parent().unwrap_or(DUMMY_SP); + } + let (sugg, app) = match self.tcx.sess.source_map().span_to_snippet(closure_body_span) { Ok(s) => { From 99a0477f65101aa6f58b7d8a5845d953da46c91e Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 11:58:04 +0200 Subject: [PATCH 2/4] Add test for closure migration with a macro body. --- .../migrations/macro.fixed | 16 +++++++++++++ .../2229_closure_analysis/migrations/macro.rs | 16 +++++++++++++ .../migrations/macro.stderr | 24 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/macro.fixed create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/macro.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/macro.fixed b/src/test/ui/closures/2229_closure_analysis/migrations/macro.fixed new file mode 100644 index 00000000000..3d9797e6579 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/macro.fixed @@ -0,0 +1,16 @@ +// run-rustfix + +// See https://github.com/rust-lang/rust/issues/87955 + +#![deny(rust_2021_incompatible_closure_captures)] +//~^ NOTE: the lint level is defined here + +fn main() { + let a = ("hey".to_string(), "123".to_string()); + let _ = || { let _ = &a; dbg!(a.0) }; + //~^ ERROR: drop order + //~| NOTE: only captures `a.0` + //~| NOTE: for more information, see + //~| HELP: add a dummy let to cause `a` to be fully captured +} +//~^ NOTE: dropped here diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/macro.rs b/src/test/ui/closures/2229_closure_analysis/migrations/macro.rs new file mode 100644 index 00000000000..ffceaf0dd22 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/macro.rs @@ -0,0 +1,16 @@ +// run-rustfix + +// See https://github.com/rust-lang/rust/issues/87955 + +#![deny(rust_2021_incompatible_closure_captures)] +//~^ NOTE: the lint level is defined here + +fn main() { + let a = ("hey".to_string(), "123".to_string()); + let _ = || dbg!(a.0); + //~^ ERROR: drop order + //~| NOTE: only captures `a.0` + //~| NOTE: for more information, see + //~| HELP: add a dummy let to cause `a` to be fully captured +} +//~^ NOTE: dropped here diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr new file mode 100644 index 00000000000..8ce5844d490 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/macro.stderr @@ -0,0 +1,24 @@ +error: changes to closure capture in Rust 2021 will affect drop order + --> $DIR/macro.rs:10:13 + | +LL | let _ = || dbg!(a.0); + | ^^^^^^^^---^ + | | + | in Rust 2018, closure captures all of `a`, but in Rust 2021, it only captures `a.0` +... +LL | } + | - in Rust 2018, `a` would be dropped here, but in Rust 2021, only `a.0` would be dropped here alongside the closure + | +note: the lint level is defined here + --> $DIR/macro.rs:5:9 + | +LL | #![deny(rust_2021_incompatible_closure_captures)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: for more information, see +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 + From cd7f96031492397bdaba87c6768269d2d622a3b2 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 20:28:34 +0200 Subject: [PATCH 3/4] Improve comment in closure migration code. --- compiler/rustc_typeck/src/check/upvar.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 862b650efd7..472af5b44b4 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -648,9 +648,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut closure_body_span = self.tcx.hir().span(body_id.hir_id); // If the body was entirely expanded from a macro - // invocation, as the body is not contained inside the - // closure span. In that case, we walk up the expansion - // until we find the span before the expansion. + // invocation, i.e. the body is not contained inside the + // closure span, then we walk up the expansion until we + // find the span before the expansion. while !closure_body_span.is_dummy() && !closure_span.contains(closure_body_span) { closure_body_span = closure_body_span.parent().unwrap_or(DUMMY_SP); } From 26c590d1b38617602a84ccdd0878c3681d242669 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 20:35:54 +0200 Subject: [PATCH 4/4] Improve fallback span for closure migration lint. --- compiler/rustc_typeck/src/check/upvar.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 472af5b44b4..a9c0c9e65e3 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -655,7 +655,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { closure_body_span = closure_body_span.parent().unwrap_or(DUMMY_SP); } - let (sugg, app) = + let (span, sugg, app) = match self.tcx.sess.source_map().span_to_snippet(closure_body_span) { Ok(s) => { let trimmed = s.trim_start(); @@ -667,9 +667,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else { format!("{{ {}; {} }}", migration_string, s) }; - (sugg, Applicability::MachineApplicable) + (closure_body_span, sugg, Applicability::MachineApplicable) } - Err(_) => (migration_string.clone(), Applicability::HasPlaceholders), + Err(_) => (closure_span, migration_string.clone(), Applicability::HasPlaceholders), }; let diagnostic_msg = format!( @@ -678,7 +678,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); diagnostics_builder.span_suggestion( - closure_body_span, + span, &diagnostic_msg, sugg, app,