From 96e3c2c1acbb711c9b936b558d6a0232a7aca626 Mon Sep 17 00:00:00 2001 From: omahs <73983677+omahs@users.noreply.github.com> Date: Sun, 17 Mar 2024 14:25:06 +0100 Subject: [PATCH 01/10] fix typo --- compiler/rustc_hir_typeck/src/method/probe.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index bdc796aca3a46..e4d5ebb82e8c8 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -1935,7 +1935,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { } } - /// Determine if the associated item withe the given DefId matches + /// Determine if the associated item with the given DefId matches /// the desired name via a doc alias. fn matches_by_doc_alias(&self, def_id: DefId) -> bool { let Some(name) = self.method_name else { From 758f642c29043630fd987afd74b008fa9c49a51d Mon Sep 17 00:00:00 2001 From: omahs <73983677+omahs@users.noreply.github.com> Date: Sun, 17 Mar 2024 14:25:24 +0100 Subject: [PATCH 02/10] fix typo --- compiler/rustc_mir_build/src/build/scope.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 88ac05cabb659..aef63896dde1e 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -774,7 +774,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let (current_root, parent_root) = if self.tcx.sess.opts.unstable_opts.maximal_hir_to_mir_coverage { // Some consumers of rustc need to map MIR locations back to HIR nodes. Currently - // the the only part of rustc that tracks MIR -> HIR is the + // the only part of rustc that tracks MIR -> HIR is the // `SourceScopeLocalData::lint_root` field that tracks lint levels for MIR // locations. Normally the number of source scopes is limited to the set of nodes // with lint annotations. The -Zmaximal-hir-to-mir-coverage flag changes this From 6828f0ca74da04537e03af63a7ef3a9394ed9202 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 17 Mar 2024 11:23:24 -0400 Subject: [PATCH 03/10] Remove some only- clauses from mir-opt tests --- tests/mir-opt/asm_unwind_panic_abort.rs | 1 - tests/mir-opt/pre-codegen/checked_ops.rs | 1 - tests/mir-opt/pre-codegen/intrinsics.rs | 1 - tests/mir-opt/pre-codegen/loops.rs | 1 - tests/mir-opt/pre-codegen/mem_replace.rs | 1 - tests/mir-opt/pre-codegen/range_iter.rs | 1 - .../pre-codegen/simple_option_map.ezmap.PreCodegen.after.mir | 4 ++-- tests/mir-opt/pre-codegen/simple_option_map.rs | 1 - tests/mir-opt/pre-codegen/slice_index.rs | 1 - tests/mir-opt/pre-codegen/slice_iter.rs | 1 - tests/mir-opt/pre-codegen/try_identity.rs | 1 - 11 files changed, 2 insertions(+), 12 deletions(-) diff --git a/tests/mir-opt/asm_unwind_panic_abort.rs b/tests/mir-opt/asm_unwind_panic_abort.rs index d6830e12287e5..8d607bc72262e 100644 --- a/tests/mir-opt/asm_unwind_panic_abort.rs +++ b/tests/mir-opt/asm_unwind_panic_abort.rs @@ -1,7 +1,6 @@ //! Tests that unwinding from an asm block is caught and forced to abort //! when `-C panic=abort`. -//@ only-x86_64 //@ compile-flags: -C panic=abort //@ no-prefer-dynamic diff --git a/tests/mir-opt/pre-codegen/checked_ops.rs b/tests/mir-opt/pre-codegen/checked_ops.rs index d36502d354784..3ff1123d0b1e5 100644 --- a/tests/mir-opt/pre-codegen/checked_ops.rs +++ b/tests/mir-opt/pre-codegen/checked_ops.rs @@ -1,7 +1,6 @@ // skip-filecheck //@ compile-flags: -O -Zmir-opt-level=2 -Cdebuginfo=2 //@ needs-unwind -//@ only-x86_64 #![crate_type = "lib"] #![feature(step_trait)] diff --git a/tests/mir-opt/pre-codegen/intrinsics.rs b/tests/mir-opt/pre-codegen/intrinsics.rs index ed7320cd3c4d5..e5c059cda12e5 100644 --- a/tests/mir-opt/pre-codegen/intrinsics.rs +++ b/tests/mir-opt/pre-codegen/intrinsics.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -//@ only-64bit // Checks that we do not have any branches in the MIR for the two tested functions. diff --git a/tests/mir-opt/pre-codegen/loops.rs b/tests/mir-opt/pre-codegen/loops.rs index 2d179abc9f31b..d0b8cc8db7a90 100644 --- a/tests/mir-opt/pre-codegen/loops.rs +++ b/tests/mir-opt/pre-codegen/loops.rs @@ -1,7 +1,6 @@ // skip-filecheck //@ compile-flags: -O -Zmir-opt-level=2 -g //@ needs-unwind -//@ only-64bit #![crate_type = "lib"] diff --git a/tests/mir-opt/pre-codegen/mem_replace.rs b/tests/mir-opt/pre-codegen/mem_replace.rs index 535c1062669a3..9cb3a83995654 100644 --- a/tests/mir-opt/pre-codegen/mem_replace.rs +++ b/tests/mir-opt/pre-codegen/mem_replace.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -Zinline-mir -//@ only-64bit //@ ignore-debug the standard library debug assertions leak into this test // EMIT_MIR_FOR_EACH_PANIC_STRATEGY diff --git a/tests/mir-opt/pre-codegen/range_iter.rs b/tests/mir-opt/pre-codegen/range_iter.rs index fe7d0e67f7a10..5aa617227ce6b 100644 --- a/tests/mir-opt/pre-codegen/range_iter.rs +++ b/tests/mir-opt/pre-codegen/range_iter.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -//@ only-64bit // EMIT_MIR_FOR_EACH_PANIC_STRATEGY #![crate_type = "lib"] diff --git a/tests/mir-opt/pre-codegen/simple_option_map.ezmap.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/simple_option_map.ezmap.PreCodegen.after.mir index 718dba21a95de..7265a4fc942d8 100644 --- a/tests/mir-opt/pre-codegen/simple_option_map.ezmap.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/simple_option_map.ezmap.PreCodegen.after.mir @@ -3,9 +3,9 @@ fn ezmap(_1: Option) -> Option { debug x => _1; let mut _0: std::option::Option; - scope 1 (inlined map::) { + scope 1 (inlined map::) { debug slf => _1; - debug f => const ZeroSized: {closure@$DIR/simple_option_map.rs:18:12: 18:15}; + debug f => const ZeroSized: {closure@$DIR/simple_option_map.rs:17:12: 17:15}; let mut _2: isize; let _3: i32; let mut _4: i32; diff --git a/tests/mir-opt/pre-codegen/simple_option_map.rs b/tests/mir-opt/pre-codegen/simple_option_map.rs index c563f6af2a539..0c432be0419bc 100644 --- a/tests/mir-opt/pre-codegen/simple_option_map.rs +++ b/tests/mir-opt/pre-codegen/simple_option_map.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -//@ only-64bit #[inline(always)] fn map(slf: Option, f: F) -> Option diff --git a/tests/mir-opt/pre-codegen/slice_index.rs b/tests/mir-opt/pre-codegen/slice_index.rs index 80bbffbd097d3..1d977ee92148f 100644 --- a/tests/mir-opt/pre-codegen/slice_index.rs +++ b/tests/mir-opt/pre-codegen/slice_index.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -//@ only-64bit //@ ignore-debug the standard library debug assertions leak into this test // EMIT_MIR_FOR_EACH_PANIC_STRATEGY diff --git a/tests/mir-opt/pre-codegen/slice_iter.rs b/tests/mir-opt/pre-codegen/slice_iter.rs index 0269eb39ddf06..0fbd370654485 100644 --- a/tests/mir-opt/pre-codegen/slice_iter.rs +++ b/tests/mir-opt/pre-codegen/slice_iter.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -//@ only-64bit //@ ignore-debug the standard library debug assertions leak into this test // EMIT_MIR_FOR_EACH_PANIC_STRATEGY diff --git a/tests/mir-opt/pre-codegen/try_identity.rs b/tests/mir-opt/pre-codegen/try_identity.rs index 9da02d65e1598..2e17a3ae6e7e4 100644 --- a/tests/mir-opt/pre-codegen/try_identity.rs +++ b/tests/mir-opt/pre-codegen/try_identity.rs @@ -1,6 +1,5 @@ // skip-filecheck //@ compile-flags: -O -C debuginfo=0 -Zmir-opt-level=2 -//@ only-64bit // Track the status of MIR optimizations simplifying `Ok(res?)` for both the old and new desugarings // of that syntax. From 872781b22697882051481a4ed7c764551a8b2cf2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 19:32:03 +0100 Subject: [PATCH 04/10] interpret/memory: explain why we use == on bool --- compiler/rustc_const_eval/src/interpret/memory.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 86aad2e1642d6..a6ca4b2e0de43 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -949,6 +949,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Runs the close in "validation" mode, which means the machine's memory read hooks will be /// suppressed. Needless to say, this must only be set with great care! Cannot be nested. pub(super) fn run_for_validation(&self, f: impl FnOnce() -> R) -> R { + // This deliberately uses `==` on `bool` to follow the pattern + // `assert!(val.replace(new) == old)`. assert!( self.memory.validation_in_progress.replace(true) == false, "`validation_in_progress` was already set" From 23a4ad12ce047a8e175f1298000553435ab835a0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 19:36:55 +0100 Subject: [PATCH 05/10] simplify_cfg: rename some passes so that they make more sense --- compiler/rustc_mir_transform/src/lib.rs | 4 ++-- compiler/rustc_mir_transform/src/simplify.rs | 11 +++++++---- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- tests/mir-opt/array_index_is_temporary.rs | 4 ++-- ...lice.main.SimplifyCfg-pre-optimizations.after.mir} | 2 +- tests/mir-opt/byte_slice.rs | 2 +- ...omoted[0].SimplifyCfg-pre-optimizations.after.mir} | 2 +- ...omoted[0].SimplifyCfg-pre-optimizations.after.mir} | 2 +- tests/mir-opt/const_promotion_extern_static.rs | 4 ++-- tests/mir-opt/no_drop_for_inactive_variant.rs | 2 +- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- tests/mir-opt/packed_struct_drop_aligned.rs | 2 +- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- tests/mir-opt/retag.rs | 10 +++++----- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- ...mplifyCfg-pre-optimizations.after.panic-abort.mir} | 2 +- ...plifyCfg-pre-optimizations.after.panic-unwind.mir} | 2 +- ... simplify_cfg.main.SimplifyCfg-post-analysis.diff} | 4 ++-- tests/mir-opt/simplify_cfg.rs | 2 +- 29 files changed, 43 insertions(+), 40 deletions(-) rename tests/mir-opt/{array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir => array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (96%) rename tests/mir-opt/{array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (96%) rename tests/mir-opt/{byte_slice.main.SimplifyCfg-elaborate-drops.after.mir => byte_slice.main.SimplifyCfg-pre-optimizations.after.mir} (90%) rename tests/mir-opt/{const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-elaborate-drops.after.mir => const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir} (85%) rename tests/mir-opt/{const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-elaborate-drops.after.mir => const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-pre-optimizations.after.mir} (82%) rename tests/mir-opt/{no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-abort.mir => no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (92%) rename tests/mir-opt/{no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (93%) rename tests/mir-opt/{packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir => packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (94%) rename tests/mir-opt/{packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (95%) rename tests/mir-opt/{retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-abort.mir => retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (98%) rename tests/mir-opt/{retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (98%) rename tests/mir-opt/{retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (85%) rename tests/mir-opt/{retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-abort.mir => retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (85%) rename tests/mir-opt/{retag.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir => retag.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (98%) rename tests/mir-opt/{retag.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => retag.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (98%) rename tests/mir-opt/{retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (94%) rename tests/mir-opt/{retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-abort.mir => retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (94%) rename tests/mir-opt/{retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-abort.mir => retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-abort.mir} (91%) rename tests/mir-opt/{retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-unwind.mir => retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-unwind.mir} (91%) rename tests/mir-opt/{simplify_cfg.main.SimplifyCfg-early-opt.diff => simplify_cfg.main.SimplifyCfg-post-analysis.diff} (88%) diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 4551380152217..3e5ec323d2746 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -507,7 +507,7 @@ fn run_analysis_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let passes: &[&dyn MirPass<'tcx>] = &[ &cleanup_post_borrowck::CleanupPostBorrowck, &remove_noop_landing_pads::RemoveNoopLandingPads, - &simplify::SimplifyCfg::EarlyOpt, + &simplify::SimplifyCfg::PostAnalysis, &deref_separator::Derefer, ]; @@ -544,7 +544,7 @@ fn run_runtime_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let passes: &[&dyn MirPass<'tcx>] = &[ &lower_intrinsics::LowerIntrinsics, &remove_place_mention::RemovePlaceMention, - &simplify::SimplifyCfg::ElaborateDrops, + &simplify::SimplifyCfg::PreOptimizations, ]; pm::run_passes(tcx, body, passes, Some(MirPhase::Runtime(RuntimePhase::PostCleanup))); diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index 8c8818bd68e6f..574330cc355c4 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -37,8 +37,11 @@ pub enum SimplifyCfg { Initial, PromoteConsts, RemoveFalseEdges, - EarlyOpt, - ElaborateDrops, + /// Runs at the beginning of "analysis to runtime" lowering, *before* drop elaboration. + PostAnalysis, + /// Runs at the end of "analysis to runtime" lowering, *after* drop elaboration. + /// This is before the main optimization passes on runtime MIR kick in. + PreOptimizations, Final, MakeShim, AfterUninhabitedEnumBranching, @@ -50,8 +53,8 @@ impl SimplifyCfg { SimplifyCfg::Initial => "SimplifyCfg-initial", SimplifyCfg::PromoteConsts => "SimplifyCfg-promote-consts", SimplifyCfg::RemoveFalseEdges => "SimplifyCfg-remove-false-edges", - SimplifyCfg::EarlyOpt => "SimplifyCfg-early-opt", - SimplifyCfg::ElaborateDrops => "SimplifyCfg-elaborate-drops", + SimplifyCfg::PostAnalysis => "SimplifyCfg-post-analysis", + SimplifyCfg::PreOptimizations => "SimplifyCfg-pre-optimizations", SimplifyCfg::Final => "SimplifyCfg-final", SimplifyCfg::MakeShim => "SimplifyCfg-make_shim", SimplifyCfg::AfterUninhabitedEnumBranching => { diff --git a/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 96% rename from tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 9b4c221df73d3..3b7c4b8796ea0 100644 --- a/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `main` after SimplifyCfg-elaborate-drops +// MIR for `main` after SimplifyCfg-pre-optimizations fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 96% rename from tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 4b05610f73105..3dcddea0353f2 100644 --- a/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `main` after SimplifyCfg-elaborate-drops +// MIR for `main` after SimplifyCfg-pre-optimizations fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/array_index_is_temporary.rs b/tests/mir-opt/array_index_is_temporary.rs index 3e5d5d5dd27f8..500b8b7f7c74e 100644 --- a/tests/mir-opt/array_index_is_temporary.rs +++ b/tests/mir-opt/array_index_is_temporary.rs @@ -1,4 +1,4 @@ -//@ unit-test: SimplifyCfg-elaborate-drops +//@ unit-test: SimplifyCfg-pre-optimizations // EMIT_MIR_FOR_EACH_PANIC_STRATEGY // Retagging (from Stacked Borrows) relies on the array index being a fresh // temporary, so that side-effects cannot change it. @@ -10,7 +10,7 @@ unsafe fn foo(z: *mut usize) -> u32 { } -// EMIT_MIR array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR array_index_is_temporary.main.SimplifyCfg-pre-optimizations.after.mir fn main() { // CHECK-LABEL: fn main( // CHECK: debug x => [[x:_.*]]; diff --git a/tests/mir-opt/byte_slice.main.SimplifyCfg-elaborate-drops.after.mir b/tests/mir-opt/byte_slice.main.SimplifyCfg-pre-optimizations.after.mir similarity index 90% rename from tests/mir-opt/byte_slice.main.SimplifyCfg-elaborate-drops.after.mir rename to tests/mir-opt/byte_slice.main.SimplifyCfg-pre-optimizations.after.mir index 09a65e6e6a6ea..c53b5e48610f6 100644 --- a/tests/mir-opt/byte_slice.main.SimplifyCfg-elaborate-drops.after.mir +++ b/tests/mir-opt/byte_slice.main.SimplifyCfg-pre-optimizations.after.mir @@ -1,4 +1,4 @@ -// MIR for `main` after SimplifyCfg-elaborate-drops +// MIR for `main` after SimplifyCfg-pre-optimizations fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/byte_slice.rs b/tests/mir-opt/byte_slice.rs index c064e2945fd22..fa616b62ad0b2 100644 --- a/tests/mir-opt/byte_slice.rs +++ b/tests/mir-opt/byte_slice.rs @@ -1,7 +1,7 @@ // skip-filecheck //@ compile-flags: -Z mir-opt-level=0 -// EMIT_MIR byte_slice.main.SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR byte_slice.main.SimplifyCfg-pre-optimizations.after.mir fn main() { let x = b"foo"; let y = [5u8, b'x']; diff --git a/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-elaborate-drops.after.mir b/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir similarity index 85% rename from tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-elaborate-drops.after.mir rename to tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir index b4ae8386add3d..7affbf6dd403f 100644 --- a/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-elaborate-drops.after.mir +++ b/tests/mir-opt/const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir @@ -1,4 +1,4 @@ -// MIR for `BAR::promoted[0]` after SimplifyCfg-elaborate-drops +// MIR for `BAR::promoted[0]` after SimplifyCfg-pre-optimizations const BAR::promoted[0]: &[&i32; 1] = { let mut _0: &[&i32; 1]; diff --git a/tests/mir-opt/const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-elaborate-drops.after.mir b/tests/mir-opt/const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-pre-optimizations.after.mir similarity index 82% rename from tests/mir-opt/const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-elaborate-drops.after.mir rename to tests/mir-opt/const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-pre-optimizations.after.mir index 8d4bfa711e414..72cb64e275e36 100644 --- a/tests/mir-opt/const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-elaborate-drops.after.mir +++ b/tests/mir-opt/const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-pre-optimizations.after.mir @@ -1,4 +1,4 @@ -// MIR for `FOO::promoted[0]` after SimplifyCfg-elaborate-drops +// MIR for `FOO::promoted[0]` after SimplifyCfg-pre-optimizations const FOO::promoted[0]: &[&i32; 1] = { let mut _0: &[&i32; 1]; diff --git a/tests/mir-opt/const_promotion_extern_static.rs b/tests/mir-opt/const_promotion_extern_static.rs index 077e74e91f43f..fe258f5e8fdb3 100644 --- a/tests/mir-opt/const_promotion_extern_static.rs +++ b/tests/mir-opt/const_promotion_extern_static.rs @@ -6,11 +6,11 @@ extern "C" { static Y: i32 = 42; // EMIT_MIR const_promotion_extern_static.BAR.PromoteTemps.diff -// EMIT_MIR const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR const_promotion_extern_static.BAR-promoted[0].SimplifyCfg-pre-optimizations.after.mir static mut BAR: *const &i32 = [&Y].as_ptr(); // EMIT_MIR const_promotion_extern_static.FOO.PromoteTemps.diff -// EMIT_MIR const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR const_promotion_extern_static.FOO-promoted[0].SimplifyCfg-pre-optimizations.after.mir static mut FOO: *const &i32 = [unsafe { &X }].as_ptr(); // EMIT_MIR const_promotion_extern_static.BOP.built.after.mir diff --git a/tests/mir-opt/no_drop_for_inactive_variant.rs b/tests/mir-opt/no_drop_for_inactive_variant.rs index dd20e4a548eb1..c94b36971ca2e 100644 --- a/tests/mir-opt/no_drop_for_inactive_variant.rs +++ b/tests/mir-opt/no_drop_for_inactive_variant.rs @@ -4,7 +4,7 @@ // Ensure that there are no drop terminators in `unwrap` (except the one along the cleanup // path). -// EMIT_MIR no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.mir fn unwrap(opt: Option) -> T { match opt { Some(x) => x, diff --git a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 92% rename from tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 31a6a1d8b3daa..fa6c6ce8e5750 100644 --- a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `unwrap` after SimplifyCfg-elaborate-drops +// MIR for `unwrap` after SimplifyCfg-pre-optimizations fn unwrap(_1: Option) -> T { debug opt => _1; diff --git a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 93% rename from tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 53352fbb19f4c..54fec3c0f9847 100644 --- a/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/no_drop_for_inactive_variant.unwrap.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `unwrap` after SimplifyCfg-elaborate-drops +// MIR for `unwrap` after SimplifyCfg-pre-optimizations fn unwrap(_1: Option) -> T { debug opt => _1; diff --git a/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 94% rename from tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 089adff0c565e..57f3c614afa5e 100644 --- a/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `main` after SimplifyCfg-elaborate-drops +// MIR for `main` after SimplifyCfg-pre-optimizations fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 95% rename from tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 0ef19180459dc..d5d466a1c30a5 100644 --- a/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `main` after SimplifyCfg-elaborate-drops +// MIR for `main` after SimplifyCfg-pre-optimizations fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/packed_struct_drop_aligned.rs b/tests/mir-opt/packed_struct_drop_aligned.rs index 079c4e68f50ca..dff941c4fa0c0 100644 --- a/tests/mir-opt/packed_struct_drop_aligned.rs +++ b/tests/mir-opt/packed_struct_drop_aligned.rs @@ -2,7 +2,7 @@ // EMIT_MIR_FOR_EACH_PANIC_STRATEGY -// EMIT_MIR packed_struct_drop_aligned.main.SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR packed_struct_drop_aligned.main.SimplifyCfg-pre-optimizations.after.mir fn main() { let mut x = Packed(Aligned(Droppy(0))); x.0 = Aligned(Droppy(0)); diff --git a/tests/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 98% rename from tests/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 0580bf3e2d0fa..7124b4c1cd873 100644 --- a/tests/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `array_casts` after SimplifyCfg-elaborate-drops +// MIR for `array_casts` after SimplifyCfg-pre-optimizations fn array_casts() -> () { let mut _0: (); diff --git a/tests/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 98% rename from tests/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 6f3cc9051d340..be04757f2a3cd 100644 --- a/tests/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `array_casts` after SimplifyCfg-elaborate-drops +// MIR for `array_casts` after SimplifyCfg-pre-optimizations fn array_casts() -> () { let mut _0: (); diff --git a/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 85% rename from tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 7f3310919cade..2620929e896be 100644 --- a/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `main::{closure#0}` after SimplifyCfg-elaborate-drops +// MIR for `main::{closure#0}` after SimplifyCfg-pre-optimizations fn main::{closure#0}(_1: &{closure@main::{closure#0}}, _2: &i32) -> &i32 { debug x => _2; diff --git a/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 85% rename from tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 7f3310919cade..2620929e896be 100644 --- a/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `main::{closure#0}` after SimplifyCfg-elaborate-drops +// MIR for `main::{closure#0}` after SimplifyCfg-pre-optimizations fn main::{closure#0}(_1: &{closure@main::{closure#0}}, _2: &i32) -> &i32 { debug x => _2; diff --git a/tests/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/retag.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 98% rename from tests/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/retag.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 96a76cc66abd6..d7372a0508254 100644 --- a/tests/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/retag.main.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `main` after SimplifyCfg-elaborate-drops +// MIR for `main` after SimplifyCfg-pre-optimizations fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/retag.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 98% rename from tests/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/retag.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 2cedf4c3f5f91..6ec62bfcf8ccb 100644 --- a/tests/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/retag.main.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `main` after SimplifyCfg-elaborate-drops +// MIR for `main` after SimplifyCfg-pre-optimizations fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/retag.rs b/tests/mir-opt/retag.rs index 0f2659ebfe853..9cee96e429498 100644 --- a/tests/mir-opt/retag.rs +++ b/tests/mir-opt/retag.rs @@ -8,8 +8,8 @@ struct Test(i32); -// EMIT_MIR retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.mir -// EMIT_MIR retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.mir +// EMIT_MIR retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.mir impl Test { // Make sure we run the pass on a method, not just on bare functions. fn foo<'x>(&self, x: &'x mut i32) -> &'x mut i32 { @@ -26,8 +26,8 @@ impl Drop for Test { fn drop(&mut self) {} } -// EMIT_MIR retag.main.SimplifyCfg-elaborate-drops.after.mir -// EMIT_MIR retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR retag.main.SimplifyCfg-pre-optimizations.after.mir +// EMIT_MIR retag.main-{closure#0}.SimplifyCfg-pre-optimizations.after.mir pub fn main() { let mut x = 0; { @@ -55,7 +55,7 @@ pub fn main() { } /// Casting directly to an array should also go through `&raw` and thus add appropriate retags. -// EMIT_MIR retag.array_casts.SimplifyCfg-elaborate-drops.after.mir +// EMIT_MIR retag.array_casts.SimplifyCfg-pre-optimizations.after.mir fn array_casts() { let mut x: [usize; 2] = [0, 0]; let p = &mut x as *mut usize; diff --git a/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 94% rename from tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 285db435f5a0a..b656656919e1f 100644 --- a/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `::foo` after SimplifyCfg-elaborate-drops +// MIR for `::foo` after SimplifyCfg-pre-optimizations fn ::foo(_1: &Test, _2: &mut i32) -> &mut i32 { debug self => _1; diff --git a/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 94% rename from tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 285db435f5a0a..b656656919e1f 100644 --- a/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/retag.{impl#0}-foo.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `::foo` after SimplifyCfg-elaborate-drops +// MIR for `::foo` after SimplifyCfg-pre-optimizations fn ::foo(_1: &Test, _2: &mut i32) -> &mut i32 { debug self => _1; diff --git a/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-abort.mir b/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-abort.mir similarity index 91% rename from tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-abort.mir rename to tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-abort.mir index 9ad607b2fe291..4f90413e38bfe 100644 --- a/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-abort.mir +++ b/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-abort.mir @@ -1,4 +1,4 @@ -// MIR for `::foo_shr` after SimplifyCfg-elaborate-drops +// MIR for `::foo_shr` after SimplifyCfg-pre-optimizations fn ::foo_shr(_1: &Test, _2: &i32) -> &i32 { debug self => _1; diff --git a/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-unwind.mir b/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-unwind.mir similarity index 91% rename from tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-unwind.mir rename to tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-unwind.mir index 9ad607b2fe291..4f90413e38bfe 100644 --- a/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-elaborate-drops.after.panic-unwind.mir +++ b/tests/mir-opt/retag.{impl#0}-foo_shr.SimplifyCfg-pre-optimizations.after.panic-unwind.mir @@ -1,4 +1,4 @@ -// MIR for `::foo_shr` after SimplifyCfg-elaborate-drops +// MIR for `::foo_shr` after SimplifyCfg-pre-optimizations fn ::foo_shr(_1: &Test, _2: &i32) -> &i32 { debug self => _1; diff --git a/tests/mir-opt/simplify_cfg.main.SimplifyCfg-early-opt.diff b/tests/mir-opt/simplify_cfg.main.SimplifyCfg-post-analysis.diff similarity index 88% rename from tests/mir-opt/simplify_cfg.main.SimplifyCfg-early-opt.diff rename to tests/mir-opt/simplify_cfg.main.SimplifyCfg-post-analysis.diff index f20ab869b7bcc..0e41950d6267d 100644 --- a/tests/mir-opt/simplify_cfg.main.SimplifyCfg-early-opt.diff +++ b/tests/mir-opt/simplify_cfg.main.SimplifyCfg-post-analysis.diff @@ -1,5 +1,5 @@ -- // MIR for `main` before SimplifyCfg-early-opt -+ // MIR for `main` after SimplifyCfg-early-opt +- // MIR for `main` before SimplifyCfg-post-analysis ++ // MIR for `main` after SimplifyCfg-post-analysis fn main() -> () { let mut _0: (); diff --git a/tests/mir-opt/simplify_cfg.rs b/tests/mir-opt/simplify_cfg.rs index 8dea0e50a61bb..b1fdc5e64a0e2 100644 --- a/tests/mir-opt/simplify_cfg.rs +++ b/tests/mir-opt/simplify_cfg.rs @@ -4,7 +4,7 @@ //@ no-prefer-dynamic // EMIT_MIR simplify_cfg.main.SimplifyCfg-initial.diff -// EMIT_MIR simplify_cfg.main.SimplifyCfg-early-opt.diff +// EMIT_MIR simplify_cfg.main.SimplifyCfg-post-analysis.diff fn main() { loop { if bar() { From 14473adf425623268252a4d10b5dd0d4f458daad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 26 Feb 2024 20:06:19 +0000 Subject: [PATCH 06/10] Detect when move of `!Copy` value occurs within `loop` and should likely not be cloned When encountering a move error on a value within a loop of any kind, identify if the moved value belongs to a call expression that should not be cloned and avoid the semantically incorrect suggestion. Also try to suggest moving the call expression outside of the loop instead. ``` error[E0382]: use of moved value: `vec` --> $DIR/recreating-value-in-loop-condition.rs:6:33 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | while let Some(item) = iter(vec).next() { | ----------------------------^^^-------- | | | | | value moved here, in previous iteration of loop | inside of this loop | note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary --> $DIR/recreating-value-in-loop-condition.rs:1:17 | LL | fn iter(vec: Vec) -> impl Iterator { | ---- ^^^^^^ this parameter takes ownership of the value | | | in this function help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ while let Some(item) = value.next() { | ``` We use the presence of a `break` in the loop that would be affected by the moved value as a heuristic for "shouldn't be cloned". Fix #121466. --- .../src/diagnostics/conflict_errors.rs | 162 +++++++++++++++++- compiler/rustc_hir/src/hir.rs | 2 +- tests/ui/borrowck/mut-borrow-in-loop-2.fixed | 35 ---- tests/ui/borrowck/mut-borrow-in-loop-2.rs | 1 - tests/ui/borrowck/mut-borrow-in-loop-2.stderr | 10 +- .../liveness/liveness-move-call-arg-2.stderr | 6 + .../ui/liveness/liveness-move-call-arg.stderr | 6 + .../moves/borrow-closures-instead-of-move.rs | 2 +- .../borrow-closures-instead-of-move.stderr | 6 + .../nested-loop-moved-value-wrong-continue.rs | 24 +++ ...ted-loop-moved-value-wrong-continue.stderr | 35 ++++ .../recreating-value-in-loop-condition.rs | 57 ++++++ .../recreating-value-in-loop-condition.stderr | 133 ++++++++++++++ 13 files changed, 437 insertions(+), 42 deletions(-) delete mode 100644 tests/ui/borrowck/mut-borrow-in-loop-2.fixed create mode 100644 tests/ui/moves/nested-loop-moved-value-wrong-continue.rs create mode 100644 tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr create mode 100644 tests/ui/moves/recreating-value-in-loop-condition.rs create mode 100644 tests/ui/moves/recreating-value-in-loop-condition.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index f81c74f3482c3..8a8fffe087671 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -447,8 +447,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { err.span_note( span, format!( - "consider changing this parameter type in {descr} `{ident}` to \ - borrow instead if owning the value isn't necessary", + "consider changing this parameter type in {descr} `{ident}` to borrow \ + instead if owning the value isn't necessary", ), ); } @@ -747,8 +747,166 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { true } + /// In a move error that occurs on a call wihtin a loop, we try to identify cases where cloning + /// the value would lead to a logic error. We infer these cases by seeing if the moved value is + /// part of the logic to break the loop, either through an explicit `break` or if the expression + /// is part of a `while let`. + fn suggest_hoisting_call_outside_loop(&self, err: &mut Diag<'_>, expr: &hir::Expr<'_>) -> bool { + let tcx = self.infcx.tcx; + let mut can_suggest_clone = true; + + // If the moved value is a locally declared binding, we'll look upwards on the expression + // tree until the scope where it is defined, and no further, as suggesting to move the + // expression beyond that point would be illogical. + let local_hir_id = if let hir::ExprKind::Path(hir::QPath::Resolved( + _, + hir::Path { res: hir::def::Res::Local(local_hir_id), .. }, + )) = expr.kind + { + Some(local_hir_id) + } else { + // This case would be if the moved value comes from an argument binding, we'll just + // look within the entire item, that's fine. + None + }; + + /// This will allow us to look for a specific `HirId`, in our case `local_hir_id` where the + /// binding was declared, within any other expression. We'll use it to search for the + /// binding declaration within every scope we inspect. + struct Finder { + hir_id: hir::HirId, + found: bool, + } + impl<'hir> Visitor<'hir> for Finder { + fn visit_pat(&mut self, pat: &'hir hir::Pat<'hir>) { + if pat.hir_id == self.hir_id { + self.found = true; + } + hir::intravisit::walk_pat(self, pat); + } + fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) { + if ex.hir_id == self.hir_id { + self.found = true; + } + hir::intravisit::walk_expr(self, ex); + } + } + // The immediate HIR parent of the moved expression. We'll look for it to be a call. + let mut parent = None; + // The top-most loop where the moved expression could be moved to a new binding. + let mut outer_most_loop: Option<&hir::Expr<'_>> = None; + for (_, node) in tcx.hir().parent_iter(expr.hir_id) { + let e = match node { + hir::Node::Expr(e) => e, + hir::Node::Local(hir::Local { els: Some(els), .. }) => { + let mut finder = BreakFinder { found_break: false }; + finder.visit_block(els); + if finder.found_break { + // Don't suggest clone as it could be will likely end in an infinite + // loop. + // let Some(_) = foo(non_copy.clone()) else { break; } + // --- ^^^^^^^^ ----- + can_suggest_clone = false; + } + continue; + } + _ => continue, + }; + if let Some(&hir_id) = local_hir_id { + let mut finder = Finder { hir_id, found: false }; + finder.visit_expr(e); + if finder.found { + // The current scope includes the declaration of the binding we're accessing, we + // can't look up any further for loops. + break; + } + } + if parent.is_none() { + parent = Some(e); + } + match e.kind { + hir::ExprKind::Let(_) => { + match tcx.parent_hir_node(e.hir_id) { + hir::Node::Expr(hir::Expr { + kind: hir::ExprKind::If(cond, ..), .. + }) => { + let mut finder = Finder { hir_id: expr.hir_id, found: false }; + finder.visit_expr(cond); + if finder.found { + // The expression where the move error happened is in a `while let` + // condition Don't suggest clone as it will likely end in an + // infinite loop. + // while let Some(_) = foo(non_copy.clone()) { } + // --------- ^^^^^^^^ + can_suggest_clone = false; + } + } + _ => {} + } + } + hir::ExprKind::Loop(..) => { + outer_most_loop = Some(e); + } + _ => {} + } + } + + /// Look for `break` expressions within any arbitrary expressions. We'll do this to infer + /// whether this is a case where the moved value would affect the exit of a loop, making it + /// unsuitable for a `.clone()` suggestion. + struct BreakFinder { + found_break: bool, + } + impl<'hir> Visitor<'hir> for BreakFinder { + fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) { + if let hir::ExprKind::Break(..) = ex.kind { + self.found_break = true; + } + hir::intravisit::walk_expr(self, ex); + } + } + if let Some(in_loop) = outer_most_loop + && let Some(parent) = parent + && let hir::ExprKind::MethodCall(..) | hir::ExprKind::Call(..) = parent.kind + { + // FIXME: We could check that the call's *parent* takes `&mut val` to make the + // suggestion more targeted to the `mk_iter(val).next()` case. Maybe do that only to + // check for wheter to suggest `let value` or `let mut value`. + + let span = in_loop.span; + let mut finder = BreakFinder { found_break: false }; + finder.visit_expr(in_loop); + let sm = tcx.sess.source_map(); + if (finder.found_break || true) + && let Ok(value) = sm.span_to_snippet(parent.span) + { + // We know with high certainty that this move would affect the early return of a + // loop, so we suggest moving the expression with the move out of the loop. + let indent = if let Some(indent) = sm.indentation_before(span) { + format!("\n{indent}") + } else { + " ".to_string() + }; + err.multipart_suggestion( + "consider moving the expression out of the loop so it is only moved once", + vec![ + (parent.span, "value".to_string()), + (span.shrink_to_lo(), format!("let mut value = {value};{indent}")), + ], + Applicability::MaybeIncorrect, + ); + } + } + can_suggest_clone + } + fn suggest_cloning(&self, err: &mut Diag<'_>, ty: Ty<'tcx>, expr: &hir::Expr<'_>, span: Span) { let tcx = self.infcx.tcx; + if !self.suggest_hoisting_call_outside_loop(err, expr) { + // The place where the the type moves would be misleading to suggest clone. (#121466) + return; + } + // Try to find predicates on *generic params* that would allow copying `ty` let suggestion = if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) { diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index e21d31238e0dd..186b8716d9a51 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2049,7 +2049,7 @@ impl LoopSource { } } -#[derive(Copy, Clone, Debug, HashStable_Generic)] +#[derive(Copy, Clone, Debug, PartialEq, HashStable_Generic)] pub enum LoopIdError { OutsideLoopScope, UnlabeledCfInWhileCondition, diff --git a/tests/ui/borrowck/mut-borrow-in-loop-2.fixed b/tests/ui/borrowck/mut-borrow-in-loop-2.fixed deleted file mode 100644 index cff3c372cdb90..0000000000000 --- a/tests/ui/borrowck/mut-borrow-in-loop-2.fixed +++ /dev/null @@ -1,35 +0,0 @@ -//@ run-rustfix -#![allow(dead_code)] - -struct Events(R); - -struct Other; - -pub trait Trait { - fn handle(value: T) -> Self; -} - -// Blanket impl. (If you comment this out, compiler figures out that it -// is passing an `&mut` to a method that must be expecting an `&mut`, -// and injects an auto-reborrow.) -impl Trait for T where T: From { - fn handle(_: U) -> Self { unimplemented!() } -} - -impl<'a, R> Trait<&'a mut Events> for Other { - fn handle(_: &'a mut Events) -> Self { unimplemented!() } -} - -fn this_compiles<'a, R>(value: &'a mut Events) { - for _ in 0..3 { - Other::handle(&mut *value); - } -} - -fn this_does_not<'a, R>(value: &'a mut Events) { - for _ in 0..3 { - Other::handle(&mut *value); //~ ERROR use of moved value: `value` - } -} - -fn main() {} diff --git a/tests/ui/borrowck/mut-borrow-in-loop-2.rs b/tests/ui/borrowck/mut-borrow-in-loop-2.rs index ba79b12042fba..f530dfca1a3ff 100644 --- a/tests/ui/borrowck/mut-borrow-in-loop-2.rs +++ b/tests/ui/borrowck/mut-borrow-in-loop-2.rs @@ -1,4 +1,3 @@ -//@ run-rustfix #![allow(dead_code)] struct Events(R); diff --git a/tests/ui/borrowck/mut-borrow-in-loop-2.stderr b/tests/ui/borrowck/mut-borrow-in-loop-2.stderr index 7b9a946f3ca3e..7a569d1da41c4 100644 --- a/tests/ui/borrowck/mut-borrow-in-loop-2.stderr +++ b/tests/ui/borrowck/mut-borrow-in-loop-2.stderr @@ -1,5 +1,5 @@ error[E0382]: use of moved value: `value` - --> $DIR/mut-borrow-in-loop-2.rs:31:23 + --> $DIR/mut-borrow-in-loop-2.rs:30:23 | LL | fn this_does_not<'a, R>(value: &'a mut Events) { | ----- move occurs because `value` has type `&mut Events`, which does not implement the `Copy` trait @@ -9,12 +9,18 @@ LL | Other::handle(value); | ^^^^^ value moved here, in previous iteration of loop | note: consider changing this parameter type in function `handle` to borrow instead if owning the value isn't necessary - --> $DIR/mut-borrow-in-loop-2.rs:9:22 + --> $DIR/mut-borrow-in-loop-2.rs:8:22 | LL | fn handle(value: T) -> Self; | ------ ^ this parameter takes ownership of the value | | | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = Other::handle(value); +LL ~ for _ in 0..3 { +LL ~ value; + | help: consider creating a fresh reborrow of `value` here | LL | Other::handle(&mut *value); diff --git a/tests/ui/liveness/liveness-move-call-arg-2.stderr b/tests/ui/liveness/liveness-move-call-arg-2.stderr index f4252e1ac33c9..9b036541e8de6 100644 --- a/tests/ui/liveness/liveness-move-call-arg-2.stderr +++ b/tests/ui/liveness/liveness-move-call-arg-2.stderr @@ -16,6 +16,12 @@ LL | fn take(_x: Box) {} | ---- ^^^^^^^^^^ this parameter takes ownership of the value | | | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = take(x); +LL ~ loop { +LL ~ value; + | help: consider cloning the value if the performance cost is acceptable | LL | take(x.clone()); diff --git a/tests/ui/liveness/liveness-move-call-arg.stderr b/tests/ui/liveness/liveness-move-call-arg.stderr index 9697ed5cb114b..b1d831b9ef95f 100644 --- a/tests/ui/liveness/liveness-move-call-arg.stderr +++ b/tests/ui/liveness/liveness-move-call-arg.stderr @@ -16,6 +16,12 @@ LL | fn take(_x: Box) {} | ---- ^^^^^^^^^^ this parameter takes ownership of the value | | | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = take(x); +LL ~ loop { +LL ~ value; + | help: consider cloning the value if the performance cost is acceptable | LL | take(x.clone()); diff --git a/tests/ui/moves/borrow-closures-instead-of-move.rs b/tests/ui/moves/borrow-closures-instead-of-move.rs index 51771ced7f22f..ab8a280221358 100644 --- a/tests/ui/moves/borrow-closures-instead-of-move.rs +++ b/tests/ui/moves/borrow-closures-instead-of-move.rs @@ -1,5 +1,5 @@ fn takes_fn(f: impl Fn()) { - loop { + loop { //~ HELP consider moving the expression out of the loop so it is only computed once takes_fnonce(f); //~^ ERROR use of moved value //~| HELP consider borrowing diff --git a/tests/ui/moves/borrow-closures-instead-of-move.stderr b/tests/ui/moves/borrow-closures-instead-of-move.stderr index 9a84ddef7e64e..b2787fd5c72f3 100644 --- a/tests/ui/moves/borrow-closures-instead-of-move.stderr +++ b/tests/ui/moves/borrow-closures-instead-of-move.stderr @@ -15,6 +15,12 @@ LL | fn takes_fnonce(_: impl FnOnce()) {} | ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value | | | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = takes_fnonce(f); +LL ~ loop { +LL ~ value; + | help: consider borrowing `f` | LL | takes_fnonce(&f); diff --git a/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs b/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs new file mode 100644 index 0000000000000..618d820a2abea --- /dev/null +++ b/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs @@ -0,0 +1,24 @@ +fn main() { + let foos = vec![String::new()]; + let bars = vec![""]; + let mut baz = vec![]; + let mut qux = vec![]; + for foo in foos { + //~^ NOTE this reinitialization might get skipped + //~| NOTE move occurs because `foo` has type `String` + for bar in &bars { + //~^ NOTE inside of this loop + //~| HELP consider moving the expression out of the loop + //~| NOTE in this expansion of desugaring of `for` loop + if foo == *bar { + baz.push(foo); + //~^ NOTE value moved here + //~| HELP consider cloning the value + continue; + } + } + qux.push(foo); + //~^ ERROR use of moved value + //~| NOTE value used here + } +} diff --git a/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr b/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr new file mode 100644 index 0000000000000..f04b5ecb935ce --- /dev/null +++ b/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr @@ -0,0 +1,35 @@ +error[E0382]: use of moved value: `foo` + --> $DIR/nested-loop-moved-value-wrong-continue.rs:20:18 + | +LL | for foo in foos { + | --- + | | + | this reinitialization might get skipped + | move occurs because `foo` has type `String`, which does not implement the `Copy` trait +... +LL | for bar in &bars { + | ---------------- inside of this loop +... +LL | baz.push(foo); + | --- value moved here, in previous iteration of loop +... +LL | qux.push(foo); + | ^^^ value used here after move + | +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = baz.push(foo); +LL ~ for bar in &bars { +LL | + ... +LL | if foo == *bar { +LL ~ value; + | +help: consider cloning the value if the performance cost is acceptable + | +LL | baz.push(foo.clone()); + | ++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0382`. diff --git a/tests/ui/moves/recreating-value-in-loop-condition.rs b/tests/ui/moves/recreating-value-in-loop-condition.rs new file mode 100644 index 0000000000000..03df8102410d6 --- /dev/null +++ b/tests/ui/moves/recreating-value-in-loop-condition.rs @@ -0,0 +1,57 @@ +fn iter(vec: Vec) -> impl Iterator { + vec.into_iter() +} +fn foo() { + let vec = vec!["one", "two", "three"]; + while let Some(item) = iter(vec).next() { //~ ERROR use of moved value + println!("{:?}", item); + } +} +fn bar() { + let vec = vec!["one", "two", "three"]; + loop { + let Some(item) = iter(vec).next() else { //~ ERROR use of moved value + break; + }; + println!("{:?}", item); + } +} +fn baz() { + let vec = vec!["one", "two", "three"]; + loop { + let item = iter(vec).next(); //~ ERROR use of moved value + if item.is_none() { + break; + } + println!("{:?}", item); + } +} +fn qux() { + let vec = vec!["one", "two", "three"]; + loop { + if let Some(item) = iter(vec).next() { //~ ERROR use of moved value + println!("{:?}", item); + } + } +} +fn zap() { + loop { + let vec = vec!["one", "two", "three"]; + loop { + loop { + loop { + if let Some(item) = iter(vec).next() { //~ ERROR use of moved value + println!("{:?}", item); + } + } + } + } + } +} +fn main() { + foo(); + bar(); + baz(); + qux(); + zap(); +} diff --git a/tests/ui/moves/recreating-value-in-loop-condition.stderr b/tests/ui/moves/recreating-value-in-loop-condition.stderr new file mode 100644 index 0000000000000..ee2a68b72c103 --- /dev/null +++ b/tests/ui/moves/recreating-value-in-loop-condition.stderr @@ -0,0 +1,133 @@ +error[E0382]: use of moved value: `vec` + --> $DIR/recreating-value-in-loop-condition.rs:6:33 + | +LL | let vec = vec!["one", "two", "three"]; + | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait +LL | while let Some(item) = iter(vec).next() { + | ----------------------------^^^-------- + | | | + | | value moved here, in previous iteration of loop + | inside of this loop + | +note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary + --> $DIR/recreating-value-in-loop-condition.rs:1:17 + | +LL | fn iter(vec: Vec) -> impl Iterator { + | ---- ^^^^^^ this parameter takes ownership of the value + | | + | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = iter(vec); +LL ~ while let Some(item) = value.next() { + | + +error[E0382]: use of moved value: `vec` + --> $DIR/recreating-value-in-loop-condition.rs:13:31 + | +LL | let vec = vec!["one", "two", "three"]; + | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait +LL | loop { + | ---- inside of this loop +LL | let Some(item) = iter(vec).next() else { + | ^^^ value moved here, in previous iteration of loop + | +note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary + --> $DIR/recreating-value-in-loop-condition.rs:1:17 + | +LL | fn iter(vec: Vec) -> impl Iterator { + | ---- ^^^^^^ this parameter takes ownership of the value + | | + | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = iter(vec); +LL ~ loop { +LL ~ let Some(item) = value.next() else { + | + +error[E0382]: use of moved value: `vec` + --> $DIR/recreating-value-in-loop-condition.rs:22:25 + | +LL | let vec = vec!["one", "two", "three"]; + | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait +LL | loop { + | ---- inside of this loop +LL | let item = iter(vec).next(); + | ^^^ value moved here, in previous iteration of loop + | +note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary + --> $DIR/recreating-value-in-loop-condition.rs:1:17 + | +LL | fn iter(vec: Vec) -> impl Iterator { + | ---- ^^^^^^ this parameter takes ownership of the value + | | + | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = iter(vec); +LL ~ loop { +LL ~ let item = value.next(); + | +help: consider cloning the value if the performance cost is acceptable + | +LL | let item = iter(vec.clone()).next(); + | ++++++++ + +error[E0382]: use of moved value: `vec` + --> $DIR/recreating-value-in-loop-condition.rs:32:34 + | +LL | let vec = vec!["one", "two", "three"]; + | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait +LL | loop { + | ---- inside of this loop +LL | if let Some(item) = iter(vec).next() { + | ^^^ value moved here, in previous iteration of loop + | +note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary + --> $DIR/recreating-value-in-loop-condition.rs:1:17 + | +LL | fn iter(vec: Vec) -> impl Iterator { + | ---- ^^^^^^ this parameter takes ownership of the value + | | + | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = iter(vec); +LL ~ loop { +LL ~ if let Some(item) = value.next() { + | + +error[E0382]: use of moved value: `vec` + --> $DIR/recreating-value-in-loop-condition.rs:43:46 + | +LL | let vec = vec!["one", "two", "three"]; + | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait +LL | loop { + | ---- inside of this loop +LL | loop { + | ---- inside of this loop +LL | loop { + | ---- inside of this loop +LL | if let Some(item) = iter(vec).next() { + | ^^^ value moved here, in previous iteration of loop + | +note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary + --> $DIR/recreating-value-in-loop-condition.rs:1:17 + | +LL | fn iter(vec: Vec) -> impl Iterator { + | ---- ^^^^^^ this parameter takes ownership of the value + | | + | in this function +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = iter(vec); +LL ~ loop { +LL | loop { +LL | loop { +LL ~ if let Some(item) = value.next() { + | + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0382`. From 78d29ad8d6483ab01b8a380ff3a9598ae23cfa05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 26 Feb 2024 22:54:34 +0000 Subject: [PATCH 07/10] Point at `continue` and `break` that might be in the wrong place Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix #92531. --- .../src/diagnostics/conflict_errors.rs | 163 ++++++++++++++---- .../liveness/liveness-move-call-arg-2.stderr | 6 - .../ui/liveness/liveness-move-call-arg.stderr | 6 - .../moves/borrow-closures-instead-of-move.rs | 2 +- .../borrow-closures-instead-of-move.stderr | 6 - .../nested-loop-moved-value-wrong-continue.rs | 26 +++ ...ted-loop-moved-value-wrong-continue.stderr | 52 +++++- .../recreating-value-in-loop-condition.rs | 2 + .../recreating-value-in-loop-condition.stderr | 17 +- 9 files changed, 225 insertions(+), 55 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 8a8fffe087671..5b61223a67f96 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -9,7 +9,7 @@ use rustc_data_structures::fx::FxIndexSet; use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag, MultiSpan}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::intravisit::{walk_block, walk_expr, Visitor}; +use rustc_hir::intravisit::{walk_block, walk_expr, Map, Visitor}; use rustc_hir::{CoroutineDesugaring, PatField}; use rustc_hir::{CoroutineKind, CoroutineSource, LangItem}; use rustc_middle::hir::nested_filter::OnlyBodies; @@ -799,9 +799,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let e = match node { hir::Node::Expr(e) => e, hir::Node::Local(hir::Local { els: Some(els), .. }) => { - let mut finder = BreakFinder { found_break: false }; + let mut finder = BreakFinder { found_breaks: vec![], found_continues: vec![] }; finder.visit_block(els); - if finder.found_break { + if !finder.found_breaks.is_empty() { // Don't suggest clone as it could be will likely end in an infinite // loop. // let Some(_) = foo(non_copy.clone()) else { break; } @@ -850,51 +850,148 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { _ => {} } } + let loop_count: usize = tcx + .hir() + .parent_iter(expr.hir_id) + .map(|(_, node)| match node { + hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Loop(..), .. }) => 1, + _ => 0, + }) + .sum(); /// Look for `break` expressions within any arbitrary expressions. We'll do this to infer /// whether this is a case where the moved value would affect the exit of a loop, making it /// unsuitable for a `.clone()` suggestion. struct BreakFinder { - found_break: bool, + found_breaks: Vec<(hir::Destination, Span)>, + found_continues: Vec<(hir::Destination, Span)>, } impl<'hir> Visitor<'hir> for BreakFinder { fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) { - if let hir::ExprKind::Break(..) = ex.kind { - self.found_break = true; + match ex.kind { + hir::ExprKind::Break(destination, _) => { + self.found_breaks.push((destination, ex.span)); + } + hir::ExprKind::Continue(destination) => { + self.found_continues.push((destination, ex.span)); + } + _ => {} } hir::intravisit::walk_expr(self, ex); } } - if let Some(in_loop) = outer_most_loop - && let Some(parent) = parent - && let hir::ExprKind::MethodCall(..) | hir::ExprKind::Call(..) = parent.kind - { - // FIXME: We could check that the call's *parent* takes `&mut val` to make the - // suggestion more targeted to the `mk_iter(val).next()` case. Maybe do that only to - // check for wheter to suggest `let value` or `let mut value`. - let span = in_loop.span; - let mut finder = BreakFinder { found_break: false }; + let sm = tcx.sess.source_map(); + if let Some(in_loop) = outer_most_loop { + let mut finder = BreakFinder { found_breaks: vec![], found_continues: vec![] }; finder.visit_expr(in_loop); - let sm = tcx.sess.source_map(); - if (finder.found_break || true) - && let Ok(value) = sm.span_to_snippet(parent.span) - { - // We know with high certainty that this move would affect the early return of a - // loop, so we suggest moving the expression with the move out of the loop. - let indent = if let Some(indent) = sm.indentation_before(span) { - format!("\n{indent}") - } else { - " ".to_string() + // All of the spans for `break` and `continue` expressions. + let spans = finder + .found_breaks + .iter() + .chain(finder.found_continues.iter()) + .map(|(_, span)| *span) + .filter(|span| { + !matches!( + span.desugaring_kind(), + Some(DesugaringKind::ForLoop | DesugaringKind::WhileLoop) + ) + }) + .collect::>(); + // All of the spans for the loops above the expression with the move error. + let loop_spans: Vec<_> = tcx + .hir() + .parent_iter(expr.hir_id) + .filter_map(|(_, node)| match node { + hir::Node::Expr(hir::Expr { span, kind: hir::ExprKind::Loop(..), .. }) => { + Some(*span) + } + _ => None, + }) + .collect(); + // It is possible that a user written `break` or `continue` is in the wrong place. We + // point them out at the user for them to make a determination. (#92531) + if !spans.is_empty() && loop_count > 1 { + // Getting fancy: if the spans of the loops *do not* overlap, we only use the line + // number when referring to them. If there *are* overlaps (multiple loops on the + // same line) then we use the more verbose span output (`file.rs:col:ll`). + let mut lines: Vec<_> = + loop_spans.iter().map(|sp| sm.lookup_char_pos(sp.lo()).line).collect(); + lines.sort(); + lines.dedup(); + let fmt_span = |span: Span| { + if lines.len() == loop_spans.len() { + format!("line {}", sm.lookup_char_pos(span.lo()).line) + } else { + sm.span_to_diagnostic_string(span) + } }; - err.multipart_suggestion( - "consider moving the expression out of the loop so it is only moved once", - vec![ - (parent.span, "value".to_string()), - (span.shrink_to_lo(), format!("let mut value = {value};{indent}")), - ], - Applicability::MaybeIncorrect, - ); + let mut spans: MultiSpan = spans.clone().into(); + // Point at all the `continue`s and explicit `break`s in the relevant loops. + for (desc, elements) in [ + ("`break` exits", &finder.found_breaks), + ("`continue` advances", &finder.found_continues), + ] { + for (destination, sp) in elements { + if let Ok(hir_id) = destination.target_id + && let hir::Node::Expr(expr) = tcx.hir().hir_node(hir_id) + && !matches!( + sp.desugaring_kind(), + Some(DesugaringKind::ForLoop | DesugaringKind::WhileLoop) + ) + { + spans.push_span_label( + *sp, + format!("this {desc} the loop at {}", fmt_span(expr.span)), + ); + } + } + } + // Point at all the loops that are between this move and the parent item. + for span in loop_spans { + spans.push_span_label(sm.guess_head_span(span), ""); + } + + // note: verify that your loop breaking logic is correct + // --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 + // | + // 28 | for foo in foos { + // | --------------- + // ... + // 33 | for bar in &bars { + // | ---------------- + // ... + // 41 | continue; + // | ^^^^^^^^ this `continue` advances the loop at line 33 + err.span_note(spans, "verify that your loop breaking logic is correct"); + } + if let Some(parent) = parent + && let hir::ExprKind::MethodCall(..) | hir::ExprKind::Call(..) = parent.kind + { + // FIXME: We could check that the call's *parent* takes `&mut val` to make the + // suggestion more targeted to the `mk_iter(val).next()` case. Maybe do that only to + // check for wheter to suggest `let value` or `let mut value`. + + let span = in_loop.span; + if !finder.found_breaks.is_empty() + && let Ok(value) = sm.span_to_snippet(parent.span) + { + // We know with high certainty that this move would affect the early return of a + // loop, so we suggest moving the expression with the move out of the loop. + let indent = if let Some(indent) = sm.indentation_before(span) { + format!("\n{indent}") + } else { + " ".to_string() + }; + err.multipart_suggestion( + "consider moving the expression out of the loop so it is only moved once", + vec![ + (parent.span, "value".to_string()), + (span.shrink_to_lo(), format!("let mut value = {value};{indent}")), + ], + Applicability::MaybeIncorrect, + ); + } } } can_suggest_clone diff --git a/tests/ui/liveness/liveness-move-call-arg-2.stderr b/tests/ui/liveness/liveness-move-call-arg-2.stderr index 9b036541e8de6..f4252e1ac33c9 100644 --- a/tests/ui/liveness/liveness-move-call-arg-2.stderr +++ b/tests/ui/liveness/liveness-move-call-arg-2.stderr @@ -16,12 +16,6 @@ LL | fn take(_x: Box) {} | ---- ^^^^^^^^^^ this parameter takes ownership of the value | | | in this function -help: consider moving the expression out of the loop so it is only moved once - | -LL ~ let mut value = take(x); -LL ~ loop { -LL ~ value; - | help: consider cloning the value if the performance cost is acceptable | LL | take(x.clone()); diff --git a/tests/ui/liveness/liveness-move-call-arg.stderr b/tests/ui/liveness/liveness-move-call-arg.stderr index b1d831b9ef95f..9697ed5cb114b 100644 --- a/tests/ui/liveness/liveness-move-call-arg.stderr +++ b/tests/ui/liveness/liveness-move-call-arg.stderr @@ -16,12 +16,6 @@ LL | fn take(_x: Box) {} | ---- ^^^^^^^^^^ this parameter takes ownership of the value | | | in this function -help: consider moving the expression out of the loop so it is only moved once - | -LL ~ let mut value = take(x); -LL ~ loop { -LL ~ value; - | help: consider cloning the value if the performance cost is acceptable | LL | take(x.clone()); diff --git a/tests/ui/moves/borrow-closures-instead-of-move.rs b/tests/ui/moves/borrow-closures-instead-of-move.rs index ab8a280221358..51771ced7f22f 100644 --- a/tests/ui/moves/borrow-closures-instead-of-move.rs +++ b/tests/ui/moves/borrow-closures-instead-of-move.rs @@ -1,5 +1,5 @@ fn takes_fn(f: impl Fn()) { - loop { //~ HELP consider moving the expression out of the loop so it is only computed once + loop { takes_fnonce(f); //~^ ERROR use of moved value //~| HELP consider borrowing diff --git a/tests/ui/moves/borrow-closures-instead-of-move.stderr b/tests/ui/moves/borrow-closures-instead-of-move.stderr index b2787fd5c72f3..9a84ddef7e64e 100644 --- a/tests/ui/moves/borrow-closures-instead-of-move.stderr +++ b/tests/ui/moves/borrow-closures-instead-of-move.stderr @@ -15,12 +15,6 @@ LL | fn takes_fnonce(_: impl FnOnce()) {} | ------------ ^^^^^^^^^^^^^ this parameter takes ownership of the value | | | in this function -help: consider moving the expression out of the loop so it is only moved once - | -LL ~ let mut value = takes_fnonce(f); -LL ~ loop { -LL ~ value; - | help: consider borrowing `f` | LL | takes_fnonce(&f); diff --git a/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs b/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs index 618d820a2abea..0235b291df540 100644 --- a/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs +++ b/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs @@ -1,3 +1,27 @@ +fn foo() { + let foos = vec![String::new()]; + let bars = vec![""]; + let mut baz = vec![]; + let mut qux = vec![]; + for foo in foos { for bar in &bars { if foo == *bar { + //~^ NOTE this reinitialization might get skipped + //~| NOTE move occurs because `foo` has type `String` + //~| NOTE inside of this loop + //~| HELP consider moving the expression out of the loop + //~| NOTE in this expansion of desugaring of `for` loop + baz.push(foo); + //~^ NOTE value moved here + //~| HELP consider cloning the value + continue; + //~^ NOTE verify that your loop breaking logic is correct + //~| NOTE this `continue` advances the loop at $DIR/nested-loop-moved-value-wrong-continue.rs:6:23 + } } + qux.push(foo); + //~^ ERROR use of moved value + //~| NOTE value used here + } +} + fn main() { let foos = vec![String::new()]; let bars = vec![""]; @@ -15,6 +39,8 @@ fn main() { //~^ NOTE value moved here //~| HELP consider cloning the value continue; + //~^ NOTE verify that your loop breaking logic is correct + //~| NOTE this `continue` advances the loop at line 33 } } qux.push(foo); diff --git a/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr b/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr index f04b5ecb935ce..3247513d42cb8 100644 --- a/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr +++ b/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr @@ -1,5 +1,42 @@ error[E0382]: use of moved value: `foo` - --> $DIR/nested-loop-moved-value-wrong-continue.rs:20:18 + --> $DIR/nested-loop-moved-value-wrong-continue.rs:19:14 + | +LL | for foo in foos { for bar in &bars { if foo == *bar { + | --- ---------------- inside of this loop + | | + | this reinitialization might get skipped + | move occurs because `foo` has type `String`, which does not implement the `Copy` trait +... +LL | baz.push(foo); + | --- value moved here, in previous iteration of loop +... +LL | qux.push(foo); + | ^^^ value used here after move + | +note: verify that your loop breaking logic is correct + --> $DIR/nested-loop-moved-value-wrong-continue.rs:15:9 + | +LL | for foo in foos { for bar in &bars { if foo == *bar { + | --------------- ---------------- +... +LL | continue; + | ^^^^^^^^ this `continue` advances the loop at $DIR/nested-loop-moved-value-wrong-continue.rs:6:23: 18:8 +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ for foo in foos { let mut value = baz.push(foo); +LL ~ for bar in &bars { if foo == *bar { +LL | + ... +LL | +LL ~ value; + | +help: consider cloning the value if the performance cost is acceptable + | +LL | baz.push(foo.clone()); + | ++++++++ + +error[E0382]: use of moved value: `foo` + --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- @@ -16,6 +53,17 @@ LL | baz.push(foo); LL | qux.push(foo); | ^^^ value used here after move | +note: verify that your loop breaking logic is correct + --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 + | +LL | for foo in foos { + | --------------- +... +LL | for bar in &bars { + | ---------------- +... +LL | continue; + | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); @@ -30,6 +78,6 @@ help: consider cloning the value if the performance cost is acceptable LL | baz.push(foo.clone()); | ++++++++ -error: aborting due to 1 previous error +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0382`. diff --git a/tests/ui/moves/recreating-value-in-loop-condition.rs b/tests/ui/moves/recreating-value-in-loop-condition.rs index 03df8102410d6..000a8471e866f 100644 --- a/tests/ui/moves/recreating-value-in-loop-condition.rs +++ b/tests/ui/moves/recreating-value-in-loop-condition.rs @@ -31,6 +31,7 @@ fn qux() { loop { if let Some(item) = iter(vec).next() { //~ ERROR use of moved value println!("{:?}", item); + break; } } } @@ -42,6 +43,7 @@ fn zap() { loop { if let Some(item) = iter(vec).next() { //~ ERROR use of moved value println!("{:?}", item); + break; } } } diff --git a/tests/ui/moves/recreating-value-in-loop-condition.stderr b/tests/ui/moves/recreating-value-in-loop-condition.stderr index ee2a68b72c103..fbf76c780d78c 100644 --- a/tests/ui/moves/recreating-value-in-loop-condition.stderr +++ b/tests/ui/moves/recreating-value-in-loop-condition.stderr @@ -99,7 +99,7 @@ LL ~ if let Some(item) = value.next() { | error[E0382]: use of moved value: `vec` - --> $DIR/recreating-value-in-loop-condition.rs:43:46 + --> $DIR/recreating-value-in-loop-condition.rs:44:46 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait @@ -119,6 +119,21 @@ LL | fn iter(vec: Vec) -> impl Iterator { | ---- ^^^^^^ this parameter takes ownership of the value | | | in this function +note: verify that your loop breaking logic is correct + --> $DIR/recreating-value-in-loop-condition.rs:46:25 + | +LL | loop { + | ---- +LL | let vec = vec!["one", "two", "three"]; +LL | loop { + | ---- +LL | loop { + | ---- +LL | loop { + | ---- +... +LL | break; + | ^^^^^ this `break` exits the loop at line 43 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); From f216bac8618e6387cdc9cd25791347fc93889147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 17 Mar 2024 21:45:03 +0000 Subject: [PATCH 08/10] Add `HELP` to test --- .../recreating-value-in-loop-condition.rs | 6 ++++++ .../recreating-value-in-loop-condition.stderr | 21 +++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/ui/moves/recreating-value-in-loop-condition.rs b/tests/ui/moves/recreating-value-in-loop-condition.rs index 000a8471e866f..ad012ff297813 100644 --- a/tests/ui/moves/recreating-value-in-loop-condition.rs +++ b/tests/ui/moves/recreating-value-in-loop-condition.rs @@ -4,12 +4,14 @@ fn iter(vec: Vec) -> impl Iterator { fn foo() { let vec = vec!["one", "two", "three"]; while let Some(item) = iter(vec).next() { //~ ERROR use of moved value + //~^ HELP consider moving the expression out of the loop so it is only moved once println!("{:?}", item); } } fn bar() { let vec = vec!["one", "two", "three"]; loop { + //~^ HELP consider moving the expression out of the loop so it is only moved once let Some(item) = iter(vec).next() else { //~ ERROR use of moved value break; }; @@ -19,7 +21,9 @@ fn bar() { fn baz() { let vec = vec!["one", "two", "three"]; loop { + //~^ HELP consider moving the expression out of the loop so it is only moved once let item = iter(vec).next(); //~ ERROR use of moved value + //~^ HELP consider cloning if item.is_none() { break; } @@ -29,6 +33,7 @@ fn baz() { fn qux() { let vec = vec!["one", "two", "three"]; loop { + //~^ HELP consider moving the expression out of the loop so it is only moved once if let Some(item) = iter(vec).next() { //~ ERROR use of moved value println!("{:?}", item); break; @@ -39,6 +44,7 @@ fn zap() { loop { let vec = vec!["one", "two", "three"]; loop { + //~^ HELP consider moving the expression out of the loop so it is only moved once loop { loop { if let Some(item) = iter(vec).next() { //~ ERROR use of moved value diff --git a/tests/ui/moves/recreating-value-in-loop-condition.stderr b/tests/ui/moves/recreating-value-in-loop-condition.stderr index fbf76c780d78c..75c9633bc0ae8 100644 --- a/tests/ui/moves/recreating-value-in-loop-condition.stderr +++ b/tests/ui/moves/recreating-value-in-loop-condition.stderr @@ -23,12 +23,13 @@ LL ~ while let Some(item) = value.next() { | error[E0382]: use of moved value: `vec` - --> $DIR/recreating-value-in-loop-condition.rs:13:31 + --> $DIR/recreating-value-in-loop-condition.rs:15:31 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | loop { | ---- inside of this loop +LL | LL | let Some(item) = iter(vec).next() else { | ^^^ value moved here, in previous iteration of loop | @@ -43,16 +44,18 @@ help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ loop { +LL | LL ~ let Some(item) = value.next() else { | error[E0382]: use of moved value: `vec` - --> $DIR/recreating-value-in-loop-condition.rs:22:25 + --> $DIR/recreating-value-in-loop-condition.rs:25:25 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | loop { | ---- inside of this loop +LL | LL | let item = iter(vec).next(); | ^^^ value moved here, in previous iteration of loop | @@ -67,6 +70,7 @@ help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ loop { +LL | LL ~ let item = value.next(); | help: consider cloning the value if the performance cost is acceptable @@ -75,12 +79,13 @@ LL | let item = iter(vec.clone()).next(); | ++++++++ error[E0382]: use of moved value: `vec` - --> $DIR/recreating-value-in-loop-condition.rs:32:34 + --> $DIR/recreating-value-in-loop-condition.rs:37:34 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | loop { | ---- inside of this loop +LL | LL | if let Some(item) = iter(vec).next() { | ^^^ value moved here, in previous iteration of loop | @@ -95,16 +100,18 @@ help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ loop { +LL | LL ~ if let Some(item) = value.next() { | error[E0382]: use of moved value: `vec` - --> $DIR/recreating-value-in-loop-condition.rs:44:46 + --> $DIR/recreating-value-in-loop-condition.rs:50:46 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | loop { | ---- inside of this loop +LL | LL | loop { | ---- inside of this loop LL | loop { @@ -120,24 +127,26 @@ LL | fn iter(vec: Vec) -> impl Iterator { | | | in this function note: verify that your loop breaking logic is correct - --> $DIR/recreating-value-in-loop-condition.rs:46:25 + --> $DIR/recreating-value-in-loop-condition.rs:52:25 | LL | loop { | ---- LL | let vec = vec!["one", "two", "three"]; LL | loop { | ---- +LL | LL | loop { | ---- LL | loop { | ---- ... LL | break; - | ^^^^^ this `break` exits the loop at line 43 + | ^^^^^ this `break` exits the loop at line 49 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ loop { +LL | LL | loop { LL | loop { LL ~ if let Some(item) = value.next() { From da2364d746291a5a1f68106d64df7402b6276df5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 17 Mar 2024 21:46:52 +0000 Subject: [PATCH 09/10] Move `Visitor` impl out to the `mod` level --- .../src/diagnostics/conflict_errors.rs | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 5b61223a67f96..85e3788d1e3f2 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -859,28 +859,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { }) .sum(); - /// Look for `break` expressions within any arbitrary expressions. We'll do this to infer - /// whether this is a case where the moved value would affect the exit of a loop, making it - /// unsuitable for a `.clone()` suggestion. - struct BreakFinder { - found_breaks: Vec<(hir::Destination, Span)>, - found_continues: Vec<(hir::Destination, Span)>, - } - impl<'hir> Visitor<'hir> for BreakFinder { - fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) { - match ex.kind { - hir::ExprKind::Break(destination, _) => { - self.found_breaks.push((destination, ex.span)); - } - hir::ExprKind::Continue(destination) => { - self.found_continues.push((destination, ex.span)); - } - _ => {} - } - hir::intravisit::walk_expr(self, ex); - } - } - let sm = tcx.sess.source_map(); if let Some(in_loop) = outer_most_loop { let mut finder = BreakFinder { found_breaks: vec![], found_continues: vec![] }; @@ -3943,6 +3921,28 @@ impl<'a, 'v> Visitor<'v> for ReferencedStatementsVisitor<'a> { } } +/// Look for `break` expressions within any arbitrary expressions. We'll do this to infer +/// whether this is a case where the moved value would affect the exit of a loop, making it +/// unsuitable for a `.clone()` suggestion. +struct BreakFinder { + found_breaks: Vec<(hir::Destination, Span)>, + found_continues: Vec<(hir::Destination, Span)>, +} +impl<'hir> Visitor<'hir> for BreakFinder { + fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) { + match ex.kind { + hir::ExprKind::Break(destination, _) => { + self.found_breaks.push((destination, ex.span)); + } + hir::ExprKind::Continue(destination) => { + self.found_continues.push((destination, ex.span)); + } + _ => {} + } + hir::intravisit::walk_expr(self, ex); + } +} + /// Given a set of spans representing statements initializing the relevant binding, visit all the /// function expressions looking for branching code paths that *do not* initialize the binding. struct ConditionVisitor<'b> { From 3b237d7d8ad46259856a42043ae37b25c6496f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 17 Mar 2024 21:52:12 +0000 Subject: [PATCH 10/10] Move `suggest_hoisting_call_outside_loop` out of `suggest_cloning` --- .../rustc_borrowck/src/diagnostics/conflict_errors.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 85e3788d1e3f2..0109f188772bc 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -463,7 +463,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } else if let UseSpans::FnSelfUse { kind: CallKind::Normal { .. }, .. } = move_spans { // We already suggest cloning for these cases in `explain_captures`. - } else { + } else if self.suggest_hoisting_call_outside_loop(err, expr) { + // The place where the the type moves would be misleading to suggest clone. + // #121466 self.suggest_cloning(err, ty, expr, move_span); } } @@ -977,11 +979,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fn suggest_cloning(&self, err: &mut Diag<'_>, ty: Ty<'tcx>, expr: &hir::Expr<'_>, span: Span) { let tcx = self.infcx.tcx; - if !self.suggest_hoisting_call_outside_loop(err, expr) { - // The place where the the type moves would be misleading to suggest clone. (#121466) - return; - } - // Try to find predicates on *generic params* that would allow copying `ty` let suggestion = if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {