diff --git a/third_party/move/move-compiler-v2/src/pipeline/ability_processor.rs b/third_party/move/move-compiler-v2/src/pipeline/ability_processor.rs index 2db12ced8982b..78870babe67b9 100644 --- a/third_party/move/move-compiler-v2/src/pipeline/ability_processor.rs +++ b/third_party/move/move-compiler-v2/src/pipeline/ability_processor.rs @@ -246,6 +246,22 @@ struct Transformer<'a> { impl<'a> Transformer<'a> { fn run(&mut self, code: Vec) { + // Check and insert drop for parameters before the first instruction if it is a return + if !code.is_empty() && code.first().unwrap().is_return() { + let instr = code.first().unwrap(); + for temp in self + .live_var + .0 + .get(&0) + .unwrap() + .released_and_unused_temps(instr) + { + if temp < self.builder.fun_env.get_parameters().len() { + self.copy_drop.get_mut(&0).unwrap().needs_drop.insert(temp); + } + } + self.check_and_add_implicit_drops(0, instr, true); + } for (offset, bc) in code.into_iter().enumerate() { self.transform_bytecode(offset as CodeOffset, bc) } @@ -290,7 +306,7 @@ impl<'a> Transformer<'a> { _ => self.check_and_emit_bytecode(code_offset, bc.clone()), } // Insert/check any drops needed after this program point - self.check_and_add_implicit_drops(code_offset, &bc) + self.check_and_add_implicit_drops(code_offset, &bc, false) } fn check_and_emit_bytecode(&mut self, _code_offset: CodeOffset, bc: Bytecode) { @@ -422,9 +438,14 @@ impl<'a> Transformer<'a> { impl<'a> Transformer<'a> { /// Add implicit drops at the given code offset. - fn check_and_add_implicit_drops(&mut self, code_offset: CodeOffset, bytecode: &Bytecode) { - // No drop after terminators - if !bytecode.is_always_branching() { + fn check_and_add_implicit_drops( + &mut self, + code_offset: CodeOffset, + bytecode: &Bytecode, + before: bool, + ) { + // No drop after terminators unless it is dropped before a return + if !bytecode.is_always_branching() || before { let copy_drop_at = self.copy_drop.get(&code_offset).expect("copy_drop"); let id = bytecode.get_attr_id(); for temp in copy_drop_at.check_drop.iter() { diff --git a/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs b/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs index c1dd410a23f01..4d66aa2cf2697 100644 --- a/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs +++ b/third_party/move/move-compiler-v2/src/pipeline/livevar_analysis_processor.rs @@ -117,8 +117,23 @@ impl FunctionTargetProcessor for LiveVarAnalysisProcessor { return data; } let target = FunctionTarget::new(fun_env, &data); - let offset_to_live_refs = LiveVarAnnotation(self.analyze(&target)); - data.annotations.set(offset_to_live_refs, true); + let mut live_info = self.analyze(&target); + // Add parameters to the before set at offset 0 + if live_info.contains_key(&0) { + let live_info_at_zero = live_info.get_mut(&0).unwrap(); + for (i, param) in fun_env.get_parameters().iter().enumerate() { + if let Entry::Vacant(e) = live_info_at_zero.before.entry(i) { + let mut usages = BTreeSet::new(); + usages.insert(param.clone().2); + let live_info = LiveVarInfo { usages }; + e.insert(live_info); + } else { + let live_info = live_info_at_zero.before.get_mut(&i).unwrap(); + live_info.usages.insert(param.clone().2); // use the location info for the parameter + } + } + } + data.annotations.set(LiveVarAnnotation(live_info), true); data } diff --git a/third_party/move/move-compiler-v2/tests/ability-check/explicit_move.move b/third_party/move/move-compiler-v2/tests/ability-check/explicit_move.move index d258aa3b39c39..1fc6ab4cceee8 100644 --- a/third_party/move/move-compiler-v2/tests/ability-check/explicit_move.move +++ b/third_party/move/move-compiler-v2/tests/ability-check/explicit_move.move @@ -1,6 +1,6 @@ module 0x42::m { - struct R has key { + struct R has key, drop { v: u64 } diff --git a/third_party/move/move-compiler-v2/tests/ability-check/inferred_copy.move b/third_party/move/move-compiler-v2/tests/ability-check/inferred_copy.move index 4f8a00d66f753..d90620a315426 100644 --- a/third_party/move/move-compiler-v2/tests/ability-check/inferred_copy.move +++ b/third_party/move/move-compiler-v2/tests/ability-check/inferred_copy.move @@ -1,6 +1,6 @@ module 0x42::m { - struct R has key, copy { + struct R has key, copy, drop { v: u64 } diff --git a/third_party/move/move-compiler-v2/tests/ability-check/unused_para_no_drop.exp b/third_party/move/move-compiler-v2/tests/ability-check/unused_para_no_drop.exp new file mode 100644 index 0000000000000..836f76fef6fd2 --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/ability-check/unused_para_no_drop.exp @@ -0,0 +1,67 @@ + +Diagnostics: +error: local `_x` of type `T` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:9:29 + │ + 9 │ public fun f1(_x: T) { + │ ╭─────────────────────────────^ +10 │ │ } + │ ╰─────^ implicitly dropped here since it is no longer used + +error: local `_x` of type `m::S` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:15:26 + │ +15 │ public fun f3(_x: S) { + │ ╭──────────────────────────^ +16 │ │ } + │ ╰─────^ implicitly dropped here since it is no longer used + +error: local `_x` of type `vector` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:21:34 + │ +21 │ public fun f5(_x: vector) { + │ ╭──────────────────────────────────^ +22 │ │ } + │ ╰─────^ implicitly dropped here since it is no longer used + +error: local `_y` of type `T` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:38:9 + │ +38 │ x + │ ^ implicitly dropped here since it is no longer used + +error: local `x` of type `m::S` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:42:9 + │ +42 │ &x == &y + │ ^^ still borrowed but will be implicitly dropped later since it is no longer used + +error: local `y` of type `m::S` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:42:15 + │ +42 │ &x == &y + │ ^^ still borrowed but will be implicitly dropped later since it is no longer used + +error: local `x` of type `T` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:46:9 + │ +46 │ &x == &y + │ ^^ still borrowed but will be implicitly dropped later since it is no longer used + +error: local `y` of type `T` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:46:15 + │ +46 │ &x == &y + │ ^^ still borrowed but will be implicitly dropped later since it is no longer used + +error: local `x` of type `m::S2` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:54:9 + │ +54 │ x.foo == y.foo + │ ^ still borrowed but will be implicitly dropped later since it is no longer used + +error: local `y` of type `m::S2` does not have the `drop` ability + ┌─ tests/ability-check/unused_para_no_drop.move:54:18 + │ +54 │ x.foo == y.foo + │ ^ still borrowed but will be implicitly dropped later since it is no longer used diff --git a/third_party/move/move-compiler-v2/tests/ability-check/unused_para_no_drop.move b/third_party/move/move-compiler-v2/tests/ability-check/unused_para_no_drop.move new file mode 100644 index 0000000000000..9a29593b4fdca --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/ability-check/unused_para_no_drop.move @@ -0,0 +1,57 @@ +module 0xc0ffee::m { + + struct S { + } + + struct G has drop { + } + + public fun f1(_x: T) { + } + + public fun f2(_x: &T) { + } + + public fun f3(_x: S) { + } + + public fun f4(_x: &S) { + } + + public fun f5(_x: vector) { + } + + public fun f6(_x: G) { + } + + public fun f7(_x: &G) { + } + + public fun f8(_x: u64) { + } + + public fun f9(_x: T) { + abort 0 // no error for this function + } + + public fun f10(x: T, _y:T): T { + x + } + + public fun f11(x: S, y: S): bool { + &x == &y + } + + public fun f12(x: T, y: T): bool { + &x == &y + } + + struct S2 { + foo: u64 + } + + public fun f13(x: S2, y: S2): bool { + x.foo == y.foo + } + +} diff --git a/third_party/move/move-compiler-v2/tests/ability-transform/mutate_return.exp b/third_party/move/move-compiler-v2/tests/ability-transform/mutate_return.exp index 1268ef79bb552..6f2eea8516789 100644 --- a/third_party/move/move-compiler-v2/tests/ability-transform/mutate_return.exp +++ b/third_party/move/move-compiler-v2/tests/ability-transform/mutate_return.exp @@ -24,7 +24,7 @@ public fun m::singleton<#0>($t0: #0): vector<#0> { [variant baseline] fun m::g<#0>($t0: &mut vector<#0>) { - # live vars: + # live vars: $t0 0: return () } @@ -53,7 +53,7 @@ public fun m::singleton<#0>($t0: #0): vector<#0> { [variant baseline] fun m::g<#0>($t0: &mut vector<#0>) { - # live vars: + # live vars: $t0 # graph: {@1000000=external[borrow(true) -> @2000000],@2000000=derived[]} # locals: {$t0=@2000000} # globals: {} @@ -111,7 +111,7 @@ public fun m::singleton<#0>($t0: #0): vector<#0> { [variant baseline] fun m::g<#0>($t0: &mut vector<#0>) { # abort state: {returns} - # live vars: + # live vars: $t0 # graph: {@1000000=external[borrow(true) -> @2000000],@2000000=derived[]} # locals: {$t0=@2000000} # globals: {} @@ -174,7 +174,8 @@ public fun m::singleton<#0>($t0: #0): vector<#0> { [variant baseline] fun m::g<#0>($t0: &mut vector<#0>) { - 0: return () + 0: drop($t0) + 1: return () } diff --git a/third_party/move/move-compiler-v2/tests/live-var/mut_ref.exp b/third_party/move/move-compiler-v2/tests/live-var/mut_ref.exp index dad0cf62faa3e..775e37a9202ca 100644 --- a/third_party/move/move-compiler-v2/tests/live-var/mut_ref.exp +++ b/third_party/move/move-compiler-v2/tests/live-var/mut_ref.exp @@ -387,13 +387,13 @@ fun m::id($t0: &mut m::R): &mut m::R { [variant baseline] fun m::some($t0: &mut m::R) { - # live vars: + # live vars: $t0 0: return () } [variant baseline] fun m::some2($t0: &mut m::R, $t1: &mut m::R) { - # live vars: + # live vars: $t0, $t1 0: return () } diff --git a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/access_control/dynamic.exp b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/access_control/dynamic.exp index 5b38fb9a25c25..8a5b7fe7e127d 100644 --- a/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/access_control/dynamic.exp +++ b/third_party/move/move-compiler-v2/transactional-tests/tests/no-v1-comparison/access_control/dynamic.exp @@ -24,6 +24,6 @@ Error: Function execution failed with VMError: { sub_status: None, location: 0x42::test, indices: [], - offsets: [(FunctionDefinitionIndex(1), 1)], + offsets: [(FunctionDefinitionIndex(1), 3)], exec_state: Some(ExecutionState { stack_trace: [] }), }