From d3e85014a712e2b41ac44d71ddee3d55711a8d44 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Sat, 21 Nov 2020 04:23:42 -0500 Subject: Process mentioned upvars for analysis first pass after ExprUseVisitor - This allows us add fake information after handling migrations if needed. - Capture analysis also priortizes what we see earlier, which means fake information should go in last. --- src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr | 8 ++++---- src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr b/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr index 1a825837614..e1b446fc61f 100644 --- a/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr +++ b/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr @@ -7,10 +7,10 @@ LL | let mut closure1 = || p = &y; = note: defining type: test::{closure#0}::{closure#0} with closure substs [ i16, extern "rust-call" fn(()), - (&'_#1r i32, &'_#2r mut &'_#3r i32), + (&'_#1r mut &'_#2r i32, &'_#3r i32), ] = note: number of external vids: 4 - = note: where '_#1r: '_#3r + = note: where '_#3r: '_#2r note: external requirements --> $DIR/escape-upvar-nested.rs:20:27 @@ -25,10 +25,10 @@ LL | | }; = note: defining type: test::{closure#0} with closure substs [ i16, extern "rust-call" fn(()), - (&'_#1r i32, &'_#2r mut &'_#3r i32), + (&'_#1r mut &'_#2r i32, &'_#3r i32), ] = note: number of external vids: 4 - = note: where '_#1r: '_#3r + = note: where '_#3r: '_#2r note: no external requirements --> $DIR/escape-upvar-nested.rs:13:1 diff --git a/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr b/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr index 29fd796882b..0ea1076c32e 100644 --- a/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr +++ b/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr @@ -7,10 +7,10 @@ LL | let mut closure = || p = &y; = note: defining type: test::{closure#0} with closure substs [ i16, extern "rust-call" fn(()), - (&'_#1r i32, &'_#2r mut &'_#3r i32), + (&'_#1r mut &'_#2r i32, &'_#3r i32), ] = note: number of external vids: 4 - = note: where '_#1r: '_#3r + = note: where '_#3r: '_#2r note: no external requirements --> $DIR/escape-upvar-ref.rs:17:1 -- cgit 1.4.1-3-g733a5 From 3c71a7b60f8c661899a4e0cbe498ac67f3f02632 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Tue, 29 Dec 2020 23:43:44 -0500 Subject: Tests for 2229 lint --- .../migrations/insignificant_drop.rs | 130 +++++++++++++++++++ .../migrations/insignificant_drop.stderr | 105 ++++++++++++++++ .../migrations/no_migrations.rs | 84 +++++++++++++ .../migrations/significant_drop.rs | 137 +++++++++++++++++++++ .../migrations/significant_drop.stderr | 103 ++++++++++++++++ 5 files changed, 559 insertions(+) create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr (limited to 'src') diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs new file mode 100644 index 00000000000..37fab71be45 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs @@ -0,0 +1,130 @@ +#![deny(disjoint_capture_drop_reorder)] +//~^ NOTE: the lint level is defined here + +// Test cases for types that implement a insignificant drop (stlib defined) + +// `t` needs Drop because one of its elements needs drop, +// therefore precise capture might affect drop ordering +fn test1_all_need_migration() { + let t = (String::new(), String::new()); + let t1 = (String::new(), String::new()); + let t2 = (String::new(), String::new()); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t, t1, t2) = (t, t1, t2); + let _t = t.0; + let _t1 = t1.0; + let _t2 = t2.0; + }; + + c(); +} + +// 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 +fn test2_only_precise_paths_need_migration() { + let t = (String::new(), String::new()); + let t1 = (String::new(), String::new()); + let t2 = (String::new(), String::new()); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t, t1) = (t, t1); + let _t = t.0; + let _t1 = t1.0; + let _t2 = t2; + }; + + c(); +} + +// 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. +fn test3_only_by_value_need_migration() { + let t = (String::new(), String::new()); + let t1 = (String::new(), String::new()); + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.0; + println!("{}", t1.1); + }; + + c(); +} + +// 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. +fn test4_only_non_copy_types_need_migration() { + let t = (String::new(), String::new()); + + // `t1` is Copy because all of its elements are Copy + let t1 = (0i32, 0i32); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.0; + let _t1 = t1.0; + }; + + c(); +} + +fn test5_only_drop_types_need_migration() { + struct S(i32, i32); + + let t = (String::new(), String::new()); + + // `s` doesn't implement Drop or any elements within it, and doesn't need migration + let s = S(0i32, 0i32); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.0; + let _s = s.0; + }; + + c(); +} + +// 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. +fn test6_move_closures_non_copy_types_might_need_migration() { + let t = (String::new(), String::new()); + let t1 = (String::new(), String::new()); + let c = move || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t1, t) = (t1, t); + println!("{} {}", t1.1, t.1); + }; + + c(); +} + +// 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, +// otherwise we won't need to, since we can get away with just by ref capture in that case. +fn test7_drop_non_drop_aggregate_need_migration() { + let t = (String::new(), 0i32); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.0; + }; + + c(); +} + +fn main() { + test1_all_need_migration(); + test2_only_precise_paths_need_migration(); + test3_only_by_value_need_migration(); + test4_only_non_copy_types_need_migration(); + test5_only_drop_types_need_migration(); + test6_move_closures_non_copy_types_might_need_migration(); + test7_drop_non_drop_aggregate_need_migration(); +} 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 new file mode 100644 index 00000000000..8b35105cf82 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.stderr @@ -0,0 +1,105 @@ +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/insignificant_drop.rs:13:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | let _t1 = t1.0; +LL | | let _t2 = t2.0; +LL | | }; + | |_____^ + | +note: the lint level is defined here + --> $DIR/insignificant_drop.rs:1:9 + | +LL | #![deny(disjoint_capture_drop_reorder)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: let (t, t1, t2) = (t, t1, t2); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/insignificant_drop.rs:31:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | let _t1 = t1.0; +LL | | let _t2 = t2; +LL | | }; + | |_____^ + | + = note: let (t, t1) = (t, t1); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/insignificant_drop.rs:47:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | println!("{}", t1.1); +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/insignificant_drop.rs:65:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | let _t1 = t1.0; +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/insignificant_drop.rs:83:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | let _s = s.0; +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/insignificant_drop.rs:98:13 + | +LL | let c = move || { + | _____________^ +LL | | +LL | | +LL | | println!("{} {}", t1.1, t.1); +LL | | }; + | |_____^ + | + = note: let (t1, t) = (t1, t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/insignificant_drop.rs:113:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: aborting due to 7 previous errors + diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs b/src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs new file mode 100644 index 00000000000..73592ce04c2 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/no_migrations.rs @@ -0,0 +1,84 @@ +// run-pass + +// Set of test cases that don't need migrations + +#![deny(disjoint_capture_drop_reorder)] + + +// Copy types as copied by the closure instead of being moved into the closure +// Therefore their drop order isn't tied to the closure and won't be requiring any +// migrations. +fn test1_only_copy_types() { + let t = (0i32, 0i32); + + let c = || { + let _t = t.0; + }; + + c(); +} + +// Same as test1 but using a move closure +fn test2_only_copy_types_move_closure() { + let t = (0i32, 0i32); + + let c = move || { + println!("{}", t.0); + }; + + c(); +} + +// Don't need to migrate if captured by ref +fn test3_only_copy_types_move_closure() { + let t = (String::new(), String::new()); + + let c = || { + println!("{}", t.0); + }; + + c(); +} + +// Test migration analysis in case of Insignificant Drop + Non Drop aggregates. +// Note in this test the closure captures a non Drop type and therefore the variable +// is only captured by ref. +fn test4_insignificant_drop_non_drop_aggregate() { + let t = (String::new(), 0i32); + + let c = || { + let _t = t.1; + }; + + c(); +} + + +struct Foo(i32); +impl Drop for Foo { + fn drop(&mut self) { + println!("{:?} dropped", self.0); + } +} + +// Test migration analysis in case of Significant Drop + Non Drop aggregates. +// Note in this test the closure captures a non Drop type and therefore the variable +// is only captured by ref. +fn test5_significant_drop_non_drop_aggregate() { + let t = (Foo(0), 0i32); + + let c = || { + let _t = t.1; + }; + + c(); +} + +fn main() { + test1_only_copy_types(); + test2_only_copy_types_move_closure(); + test3_only_copy_types_move_closure(); + test4_insignificant_drop_non_drop_aggregate(); + test5_significant_drop_non_drop_aggregate(); + +} diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs new file mode 100644 index 00000000000..1818e2dcc64 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs @@ -0,0 +1,137 @@ +#![deny(disjoint_capture_drop_reorder)] +//~^ NOTE: the lint level is defined here + +// Test cases for types that implement a significant drop (user defined) + +#[derive(Debug)] +struct Foo(i32); +impl Drop for Foo { + fn drop(&mut self) { + println!("{:?} dropped", self.0); + } +} + +#[derive(Debug)] +struct ConstainsDropField(Foo, Foo); + +// `t` needs Drop because one of its elements needs drop, +// therefore precise capture might affect drop ordering +fn test1_all_need_migration() { + let t = (Foo(0), Foo(0)); + let t1 = (Foo(0), Foo(0)); + let t2 = (Foo(0), Foo(0)); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t, t1, t2) = (t, t1, t2); + let _t = t.0; + let _t1 = t1.0; + let _t2 = t2.0; + }; + + c(); +} + +// 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 +fn test2_only_precise_paths_need_migration() { + let t = (Foo(0), Foo(0)); + let t1 = (Foo(0), Foo(0)); + let t2 = (Foo(0), Foo(0)); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t, t1) = (t, t1); + let _t = t.0; + let _t1 = t1.0; + let _t2 = t2; + }; + + c(); +} + +// 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. +fn test3_only_by_value_need_migration() { + let t = (Foo(0), Foo(0)); + let t1 = (Foo(0), Foo(0)); + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.0; + println!("{:?}", t1.1); + }; + + c(); +} + +// The root variable might not implement drop themselves but some path starting +// at the root variable might implement Drop. +// +// If this path isn't captured we need to migrate for the root variable. +fn test4_type_contains_drop_need_migration() { + let t = ConstainsDropField(Foo(0), Foo(0)); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.0; + }; + + c(); +} + +// 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, +// otherwise we won't need to, since we can get away with just by ref capture in that case. +fn test5_drop_non_drop_aggregate_need_migration() { + let t = (Foo(0), 0i32); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.0; + }; + + c(); +} + +// Test migration analysis in case of Significant and Insignificant Drop aggregates. +fn test6_significant_insignificant_drop_aggregate_need_migration() { + struct S(i32, i32); + + let t = (Foo(0), String::new()); + + let c = || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t) = (t); + let _t = t.1; + }; + + c(); +} + +// 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. +fn test7_move_closures_non_copy_types_might_need_migration() { + let t = (Foo(0), Foo(0)); + let t1 = (Foo(0), Foo(0), Foo(0)); + + let c = move || { + //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` + //~| NOTE: let (t1, t) = (t1, t); + println!("{:?} {:?}", t1.1, t.1); + }; + + c(); +} + +fn main() { + test1_all_need_migration(); + test2_only_precise_paths_need_migration(); + test3_only_by_value_need_migration(); + test4_type_contains_drop_need_migration(); + test5_drop_non_drop_aggregate_need_migration(); + test6_significant_insignificant_drop_aggregate_need_migration(); + test7_move_closures_non_copy_types_might_need_migration(); +} 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 new file mode 100644 index 00000000000..52b6a628cc0 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr @@ -0,0 +1,103 @@ +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/significant_drop.rs:24:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | let _t1 = t1.0; +LL | | let _t2 = t2.0; +LL | | }; + | |_____^ + | +note: the lint level is defined here + --> $DIR/significant_drop.rs:1:9 + | +LL | #![deny(disjoint_capture_drop_reorder)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: let (t, t1, t2) = (t, t1, t2); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/significant_drop.rs:42:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | let _t1 = t1.0; +LL | | let _t2 = t2; +LL | | }; + | |_____^ + | + = note: let (t, t1) = (t, t1); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/significant_drop.rs:58:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | println!("{:?}", t1.1); +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/significant_drop.rs:75:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/significant_drop.rs:90:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.0; +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/significant_drop.rs:105:13 + | +LL | let c = || { + | _____________^ +LL | | +LL | | +LL | | let _t = t.1; +LL | | }; + | |_____^ + | + = note: let (t) = (t); + +error: drop order affected for closure because of `capture_disjoint_fields` + --> $DIR/significant_drop.rs:120:13 + | +LL | let c = move || { + | _____________^ +LL | | +LL | | +LL | | println!("{:?} {:?}", t1.1, t.1); +LL | | }; + | |_____^ + | + = note: let (t1, t) = (t1, t); + +error: aborting due to 7 previous errors + -- cgit 1.4.1-3-g733a5 From 8f15cc1d88e2a4fdc41984da6178a8c2c92b1e2b Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Tue, 19 Jan 2021 03:15:36 -0500 Subject: PR fixup --- compiler/rustc_typeck/src/check/upvar.rs | 85 ++++++++++++---------- .../migrations/insignificant_drop.rs | 2 +- .../migrations/significant_drop.rs | 2 +- 3 files changed, 48 insertions(+), 41 deletions(-) (limited to 'src') diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 16aaa48ce17..542d9a58e8a 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -162,34 +162,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id); if should_do_migration_analysis(self.tcx, closure_hir_id) { - let need_migrations = self.compute_2229_migrations_first_pass( - closure_def_id, - span, - capture_clause, - body, - self.typeck_results.borrow().closure_min_captures.get(&closure_def_id), - ); - - if !need_migrations.is_empty() { - let need_migrations_hir_id = - need_migrations.iter().map(|m| m.0).collect::>(); - - let migrations_text = - migration_suggestion_for_2229(self.tcx, &need_migrations_hir_id); - - self.tcx.struct_span_lint_hir( - lint::builtin::DISJOINT_CAPTURE_DROP_REORDER, - closure_hir_id, - span, - |lint| { - let mut diagnostics_builder = lint.build( - "drop order affected for closure because of `capture_disjoint_fields`", - ); - diagnostics_builder.note(&migrations_text); - diagnostics_builder.emit(); - }, - ); - } + self.perform_2229_migration_anaysis(closure_def_id, capture_clause, span, body); } // We now fake capture information for all variables that are mentioned within the closure @@ -555,6 +528,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { typeck_results.closure_min_captures.insert(closure_def_id, root_var_min_capture_list); } + /// Perform the migration analysis for RFC 2229, and emit lint + /// `disjoint_capture_drop_reorder` if needed. + fn perform_2229_migration_anaysis( + &self, + closure_def_id: DefId, + capture_clause: hir::CaptureBy, + span: Span, + body: &'tcx hir::Body<'tcx>, + ) { + let need_migrations = self.compute_2229_migrations_first_pass( + closure_def_id, + span, + capture_clause, + body, + self.typeck_results.borrow().closure_min_captures.get(&closure_def_id), + ); + + if !need_migrations.is_empty() { + let need_migrations_hir_id = need_migrations.iter().map(|m| m.0).collect::>(); + + let migrations_text = migration_suggestion_for_2229(self.tcx, &need_migrations_hir_id); + + let local_def_id = closure_def_id.expect_local(); + let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id); + self.tcx.struct_span_lint_hir( + lint::builtin::DISJOINT_CAPTURE_DROP_REORDER, + closure_hir_id, + span, + |lint| { + let mut diagnostics_builder = lint.build( + "drop order affected for closure because of `capture_disjoint_fields`", + ); + diagnostics_builder.note(&migrations_text); + diagnostics_builder.emit(); + }, + ); + } + } + /// Figures out the list of root variables (and their types) that aren't completely /// captured by the closure when `capture_disjoint_fields` is enabled and drop order of /// some path starting at that root variable **might** be affected. @@ -617,17 +629,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let is_moved = root_var_min_capture_list .iter() - .find(|capture| matches!(capture.info.capture_kind, ty::UpvarCapture::ByValue(_))) - .is_some(); - - // 1. If we capture more than one path starting at the root variabe then the root variable - // isn't being captured in its entirety - // 2. If we only capture one path starting at the root variable, it's still possible - // that it isn't the root variable completely. - if is_moved - && ((root_var_min_capture_list.len() > 1) - || (root_var_min_capture_list[0].place.projections.len() > 0)) - { + .any(|capture| matches!(capture.info.capture_kind, ty::UpvarCapture::ByValue(_))); + + let is_not_completely_captured = + root_var_min_capture_list.iter().any(|capture| capture.place.projections.len() > 0); + + if is_moved && is_not_completely_captured { need_migrations.push((var_hir_id, ty)); } } diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs index 37fab71be45..5b5092e9db9 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs +++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs @@ -108,7 +108,7 @@ fn test6_move_closures_non_copy_types_might_need_migration() { // Note we need migration here only because the non-copy (because Drop type) is captured, // otherwise we won't need to, since we can get away with just by ref capture in that case. fn test7_drop_non_drop_aggregate_need_migration() { - let t = (String::new(), 0i32); + let t = (String::new(), String::new(), 0i32); let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs index 1818e2dcc64..25b5539b862 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs +++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs @@ -85,7 +85,7 @@ fn test4_type_contains_drop_need_migration() { // Note we need migration here only because the non-copy (because Drop type) is captured, // otherwise we won't need to, since we can get away with just by ref capture in that case. fn test5_drop_non_drop_aggregate_need_migration() { - let t = (Foo(0), 0i32); + let t = (Foo(0), Foo(0), 0i32); let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` -- cgit 1.4.1-3-g733a5 From 84f0a0a1c6627e83408d1faf46e29d29d2bc13e9 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Tue, 26 Jan 2021 04:08:23 -0500 Subject: New migration --- compiler/rustc_typeck/src/check/upvar.rs | 2 +- .../2229_closure_analysis/migrations/insignificant_drop.rs | 14 +++++++------- .../migrations/insignificant_drop.stderr | 14 +++++++------- .../2229_closure_analysis/migrations/significant_drop.rs | 14 +++++++------- .../migrations/significant_drop.stderr | 14 +++++++------- 5 files changed, 29 insertions(+), 29 deletions(-) (limited to 'src') diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 542d9a58e8a..04a9e65e664 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -1270,7 +1270,7 @@ fn migration_suggestion_for_2229(tcx: TyCtxt<'_>, need_migrations: &Vec>(); let migrations_list_concat = need_migrations_strings.join(", "); - format!("let ({}) = ({});", migrations_list_concat, migrations_list_concat) + format!("drop(&({}));", migrations_list_concat) } /// Helper function to determine if we need to escalate CaptureKind from diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs index 5b5092e9db9..02b37362096 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs +++ b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs @@ -12,7 +12,7 @@ fn test1_all_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t, t1, t2) = (t, t1, t2); + //~| NOTE: drop(&(t, t1, t2)); let _t = t.0; let _t1 = t1.0; let _t2 = t2.0; @@ -30,7 +30,7 @@ fn test2_only_precise_paths_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t, t1) = (t, t1); + //~| NOTE: drop(&(t, t1)); let _t = t.0; let _t1 = t1.0; let _t2 = t2; @@ -46,7 +46,7 @@ fn test3_only_by_value_need_migration() { let t1 = (String::new(), String::new()); let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.0; println!("{}", t1.1); }; @@ -64,7 +64,7 @@ fn test4_only_non_copy_types_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.0; let _t1 = t1.0; }; @@ -82,7 +82,7 @@ fn test5_only_drop_types_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.0; let _s = s.0; }; @@ -97,7 +97,7 @@ fn test6_move_closures_non_copy_types_might_need_migration() { let t1 = (String::new(), String::new()); let c = move || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t1, t) = (t1, t); + //~| NOTE: drop(&(t1, t)); println!("{} {}", t1.1, t.1); }; @@ -112,7 +112,7 @@ fn test7_drop_non_drop_aggregate_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.0; }; 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 8b35105cf82..656c132c12d 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 @@ -16,7 +16,7 @@ note: the lint level is defined here | LL | #![deny(disjoint_capture_drop_reorder)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: let (t, t1, t2) = (t, t1, t2); + = note: drop(&(t, t1, t2)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/insignificant_drop.rs:31:13 @@ -31,7 +31,7 @@ LL | | let _t2 = t2; LL | | }; | |_____^ | - = note: let (t, t1) = (t, t1); + = note: drop(&(t, t1)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/insignificant_drop.rs:47:13 @@ -45,7 +45,7 @@ LL | | println!("{}", t1.1); LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/insignificant_drop.rs:65:13 @@ -59,7 +59,7 @@ LL | | let _t1 = t1.0; LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/insignificant_drop.rs:83:13 @@ -73,7 +73,7 @@ LL | | let _s = s.0; LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/insignificant_drop.rs:98:13 @@ -86,7 +86,7 @@ LL | | println!("{} {}", t1.1, t.1); LL | | }; | |_____^ | - = note: let (t1, t) = (t1, t); + = note: drop(&(t1, t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/insignificant_drop.rs:113:13 @@ -99,7 +99,7 @@ LL | | let _t = t.0; LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: aborting due to 7 previous errors diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs index 25b5539b862..ed5e4ea8be0 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs +++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs @@ -23,7 +23,7 @@ fn test1_all_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t, t1, t2) = (t, t1, t2); + //~| NOTE: drop(&(t, t1, t2)); let _t = t.0; let _t1 = t1.0; let _t2 = t2.0; @@ -41,7 +41,7 @@ fn test2_only_precise_paths_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t, t1) = (t, t1); + //~| NOTE: drop(&(t, t1)); let _t = t.0; let _t1 = t1.0; let _t2 = t2; @@ -57,7 +57,7 @@ fn test3_only_by_value_need_migration() { let t1 = (Foo(0), Foo(0)); let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.0; println!("{:?}", t1.1); }; @@ -74,7 +74,7 @@ fn test4_type_contains_drop_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.0; }; @@ -89,7 +89,7 @@ fn test5_drop_non_drop_aggregate_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.0; }; @@ -104,7 +104,7 @@ fn test6_significant_insignificant_drop_aggregate_need_migration() { let c = || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t) = (t); + //~| NOTE: drop(&(t)); let _t = t.1; }; @@ -119,7 +119,7 @@ fn test7_move_closures_non_copy_types_might_need_migration() { let c = move || { //~^ERROR: drop order affected for closure because of `capture_disjoint_fields` - //~| NOTE: let (t1, t) = (t1, t); + //~| NOTE: drop(&(t1, t)); println!("{:?} {:?}", t1.1, t.1); }; 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 52b6a628cc0..6c21b27b493 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 @@ -16,7 +16,7 @@ note: the lint level is defined here | LL | #![deny(disjoint_capture_drop_reorder)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: let (t, t1, t2) = (t, t1, t2); + = note: drop(&(t, t1, t2)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/significant_drop.rs:42:13 @@ -31,7 +31,7 @@ LL | | let _t2 = t2; LL | | }; | |_____^ | - = note: let (t, t1) = (t, t1); + = note: drop(&(t, t1)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/significant_drop.rs:58:13 @@ -45,7 +45,7 @@ LL | | println!("{:?}", t1.1); LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/significant_drop.rs:75:13 @@ -58,7 +58,7 @@ LL | | let _t = t.0; LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/significant_drop.rs:90:13 @@ -71,7 +71,7 @@ LL | | let _t = t.0; LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/significant_drop.rs:105:13 @@ -84,7 +84,7 @@ LL | | let _t = t.1; LL | | }; | |_____^ | - = note: let (t) = (t); + = note: drop(&(t)); error: drop order affected for closure because of `capture_disjoint_fields` --> $DIR/significant_drop.rs:120:13 @@ -97,7 +97,7 @@ LL | | println!("{:?} {:?}", t1.1, t.1); LL | | }; | |_____^ | - = note: let (t1, t) = (t1, t); + = note: drop(&(t1, t)); error: aborting due to 7 previous errors -- cgit 1.4.1-3-g733a5