Skip to content

Commit

Permalink
[Compiler-V2] Check Unused Assignments (#13543)
Browse files Browse the repository at this point in the history
* add unused assignment checker

* update tests

* add checker

* update tests

* format

* add test

* reformat err msg

* refactor

* update tests

* update tests

* update tests

* update tests

* format

* add test

* import more tests

* tab to space

* add test

* remove wrong doc

* add test

* update tests

---------

Co-authored-by: Zekun Wang <zekun.wang@aptoslabs.com>
  • Loading branch information
fEst1ck and Zekun Wang committed Jun 28, 2024
1 parent ad42e03 commit 3d4d746
Show file tree
Hide file tree
Showing 108 changed files with 2,465 additions and 31 deletions.
6 changes: 6 additions & 0 deletions third_party/move/move-compiler-v2/src/experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ pub static EXPERIMENTS: Lazy<BTreeMap<String, Experiment>> = Lazy::new(|| {
description: "Whether to check for unused struct type parameters".to_string(),
default: Inherited(Experiment::CHECKS.to_string()),
},
Experiment {
name: Experiment::UNUSED_ASSIGNMENT_CHECK.to_string(),
description: "Whether to check for unused assignments".to_string(),
default: Inherited(Experiment::CHECKS.to_string()),
},
Experiment {
name: Experiment::VARIABLE_COALESCING.to_string(),
description: "Whether to run variable coalescing".to_string(),
Expand Down Expand Up @@ -214,6 +219,7 @@ impl Experiment {
pub const SPEC_REWRITE: &'static str = "spec-rewrite";
pub const SPLIT_CRITICAL_EDGES: &'static str = "split-critical-edges";
pub const UNINITIALIZED_CHECK: &'static str = "uninitialized-check";
pub const UNUSED_ASSIGNMENT_CHECK: &'static str = "unused-assignment-check";
pub const UNUSED_STRUCT_PARAMS_CHECK: &'static str = "unused-struct-params-check";
pub const USAGE_CHECK: &'static str = "usage-check";
pub const VARIABLE_COALESCING: &'static str = "variable-coalescing";
Expand Down
9 changes: 8 additions & 1 deletion third_party/move/move-compiler-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ use crate::{
split_critical_edges_processor::SplitCriticalEdgesProcessor,
uninitialized_use_checker::UninitializedUseChecker,
unreachable_code_analysis::UnreachableCodeProcessor,
unreachable_code_remover::UnreachableCodeRemover, variable_coalescing::VariableCoalescing,
unreachable_code_remover::UnreachableCodeRemover,
unused_assignment_checker::UnusedAssignmentChecker,
variable_coalescing::VariableCoalescing,
},
};
use anyhow::bail;
Expand Down Expand Up @@ -391,6 +393,11 @@ pub fn bytecode_pipeline(env: &GlobalEnv) -> FunctionTargetPipeline {
pipeline.add_processor(Box::new(UninitializedUseChecker { keep_annotations }));
}

if options.experiment_on(Experiment::UNUSED_ASSIGNMENT_CHECK) {
pipeline.add_processor(Box::new(LiveVarAnalysisProcessor::new(false)));
pipeline.add_processor(Box::new(UnusedAssignmentChecker {}));
}

// Reference check is always run, but the processor decides internally
// based on `Experiment::REFERENCE_SAFETY` whether to report errors.
pipeline.add_processor(Box::new(LiveVarAnalysisProcessor::new(false)));
Expand Down
1 change: 1 addition & 0 deletions third_party/move/move-compiler-v2/src/pipeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub mod split_critical_edges_processor;
pub mod uninitialized_use_checker;
pub mod unreachable_code_analysis;
pub mod unreachable_code_remover;
pub mod unused_assignment_checker;
pub mod variable_coalescing;
pub mod visibility_checker;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright © Aptos Foundation
// SPDX-License-Identifier: Apache-2.0

//! Implements a pipeline that checks and gives warning on unused assignments.
//! Prerequisite: live variable annotation.

use crate::pipeline::livevar_analysis_processor::LiveVarAnnotation;
use codespan_reporting::diagnostic::Severity;
use move_binary_format::file_format::CodeOffset;
use move_model::{ast::TempIndex, model::FunctionEnv};
use move_stackless_bytecode::{
function_target::{FunctionData, FunctionTarget},
function_target_pipeline::{FunctionTargetProcessor, FunctionTargetsHolder},
stackless_bytecode::{AttrId, Bytecode},
};

pub struct UnusedAssignmentChecker {}

impl UnusedAssignmentChecker {
/// Check if the assignment to `dst` at offset after the position given by `offset` and `after`.
fn check_unused_assignment(
target: &FunctionTarget,
id: AttrId,
offset: CodeOffset,
dst: TempIndex,
) {
let data = target.data;
// only check for user defined variables
if let Some(dst_name) = data.local_names.get(&dst) {
let live_var_info = target
.get_annotations()
.get::<LiveVarAnnotation>()
.expect("live variable annotation")
.get_info_at(offset);
let live_after = &live_var_info.after;
let dst_name = dst_name.display(target.func_env.symbol_pool()).to_string();
if !dst_name.starts_with('_') && live_after.get(&dst).is_none() {
let loc = target.get_bytecode_loc(id);
target
.global_env()
.diag(
Severity::Warning,
&loc,
&format!("Unused assignment to `{}`. Consider removing or prefixing with an underscore: `_{}`", dst_name, dst_name)
);
}
}
}
}

impl FunctionTargetProcessor for UnusedAssignmentChecker {
fn process(
&self,
_targets: &mut FunctionTargetsHolder,
func_env: &FunctionEnv,
data: FunctionData,
_scc_opt: Option<&[FunctionEnv]>,
) -> FunctionData {
if func_env.is_native() {
return data;
}
let target = FunctionTarget::new(func_env, &data);
for (offset, bytecode) in data.code.iter().enumerate() {
let offset = offset as u16;
use Bytecode::*;
match bytecode {
Load(id, dst, _) | Assign(id, dst, _, _) => {
UnusedAssignmentChecker::check_unused_assignment(&target, *id, offset, *dst)
},
Call(id, dsts, _, _, _) => {
for dst in dsts {
UnusedAssignmentChecker::check_unused_assignment(&target, *id, offset, *dst)
}
},
_ => {},
}
}
data
}

fn name(&self) -> String {
"UnusedAssignmentChecker".to_string()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,50 @@ fun m::f($t0: u8, $t1: &vector<u64>): u64 {
14: return $t2
}

============ after LiveVarAnalysisProcessor: ================

[variant baseline]
fun m::f($t0: u8, $t1: &vector<u64>): u64 {
var $t2: u64
var $t3: &vector<u64>
var $t4: bool
var $t5: u8
var $t6: &m::R
var $t7: address
var $t8: &u64
var $t9: u64
# live vars: $t0, $t1
0: $t5 := 0
# live vars: $t0, $t1, $t5
1: $t4 := ==($t0, $t5)
# live vars: $t1, $t4
2: if ($t4) goto 3 else goto 8
# live vars: $t1
3: label L0
# live vars:
4: $t7 := 0x1
# live vars: $t7
5: $t6 := borrow_global<m::R>($t7)
# live vars: $t6
6: $t3 := borrow_field<m::R>.data($t6)
# live vars: $t3
7: goto 10
# live vars: $t1
8: label L1
# live vars: $t1
9: $t3 := infer($t1)
# live vars: $t3
10: label L2
# live vars: $t3
11: $t9 := 0
# live vars: $t3, $t9
12: $t8 := vector::borrow<u64>($t3, $t9)
# live vars: $t8
13: $t2 := read_ref($t8)
# live vars: $t2
14: return $t2
}

============ after ReferenceSafetyProcessor: ================

[variant baseline]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,204 @@ fun <SELF>_0::check() {
80: return ()
}

============ after LiveVarAnalysisProcessor: ================

[variant baseline]
fun <SELF>_0::check() {
var $t0: bool
var $t1: u64
var $t2: bool
var $t3: u64
var $t4: bool
var $t5: u64
var $t6: u64
var $t7: u64
var $t8: bool
var $t9: vector<u8>
var $t10: vector<u8>
var $t11: u64
var $t12: &mut u64
var $t13: u64
var $t14: u64
var $t15: &mut vector<u8>
var $t16: vector<u8>
var $t17: vector<u8>
var $t18: bool
var $t19: u64
var $t20: u64
var $t21: u64
var $t22: bool
var $t23: vector<u8>
var $t24: vector<u8>
var $t25: u64
var $t26: bool
var $t27: u64
var $t28: bool
var $t29: u64
# live vars:
0: $t0 := true
# live vars: $t0
1: if ($t0) goto 2 else goto 4
# live vars:
2: label L0
# live vars:
3: goto 7
# live vars:
4: label L1
# live vars:
5: $t1 := 42
# live vars: $t1
6: abort($t1)
# live vars:
7: label L2
# live vars:
8: $t2 := true
# live vars: $t2
9: if ($t2) goto 10 else goto 12
# live vars:
10: label L3
# live vars:
11: goto 15
# live vars:
12: label L4
# live vars:
13: $t3 := 42
# live vars: $t3
14: abort($t3)
# live vars:
15: label L5
# live vars:
16: $t5 := 0
# live vars: $t5
17: $t6 := 0
# live vars: $t5, $t6
18: $t4 := ==($t5, $t6)
# live vars: $t4
19: if ($t4) goto 20 else goto 22
# live vars:
20: label L6
# live vars:
21: goto 25
# live vars:
22: label L7
# live vars:
23: $t7 := 42
# live vars: $t7
24: abort($t7)
# live vars:
25: label L8
# live vars:
26: $t9 := [104, 101, 108, 108, 111]
# live vars: $t9
27: $t10 := [104, 101, 108, 108, 111]
# live vars: $t9, $t10
28: $t8 := ==($t9, $t10)
# live vars: $t8
29: if ($t8) goto 30 else goto 32
# live vars:
30: label L9
# live vars:
31: goto 35
# live vars:
32: label L10
# live vars:
33: $t11 := 42
# live vars: $t11
34: abort($t11)
# live vars:
35: label L11
# live vars:
36: $t13 := 0
# live vars: $t13
37: $t12 := borrow_local($t13)
# live vars: $t12
38: $t14 := 1
# live vars: $t12, $t14
39: write_ref($t12, $t14)
# live vars: $t12
40: $t16 := [104, 101, 108, 108, 111]
# live vars: $t12, $t16
41: $t15 := borrow_local($t16)
# live vars: $t12, $t15
42: $t17 := [98, 121, 101]
# live vars: $t12, $t15, $t17
43: write_ref($t15, $t17)
# live vars: $t12, $t15
44: $t19 := read_ref($t12)
# live vars: $t15, $t19
45: $t20 := 1
# live vars: $t15, $t19, $t20
46: $t18 := ==($t19, $t20)
# live vars: $t15, $t18
47: if ($t18) goto 48 else goto 50
# live vars: $t15
48: label L12
# live vars: $t15
49: goto 53
# live vars: $t15
50: label L13
# live vars:
51: $t21 := 42
# live vars: $t21
52: abort($t21)
# live vars: $t15
53: label L14
# live vars: $t15
54: $t23 := read_ref($t15)
# live vars: $t23
55: $t24 := [98, 121, 101]
# live vars: $t23, $t24
56: $t22 := ==($t23, $t24)
# live vars: $t22
57: if ($t22) goto 58 else goto 60
# live vars:
58: label L15
# live vars:
59: goto 63
# live vars:
60: label L16
# live vars:
61: $t25 := 42
# live vars: $t25
62: abort($t25)
# live vars:
63: label L17
# live vars:
64: $t26 := true
# live vars: $t26
65: if ($t26) goto 66 else goto 68
# live vars:
66: label L18
# live vars:
67: goto 71
# live vars:
68: label L19
# live vars:
69: $t27 := 42
# live vars: $t27
70: abort($t27)
# live vars:
71: label L20
# live vars:
72: $t28 := true
# live vars: $t28
73: if ($t28) goto 74 else goto 76
# live vars:
74: label L21
# live vars:
75: goto 79
# live vars:
76: label L22
# live vars:
77: $t29 := 42
# live vars: $t29
78: abort($t29)
# live vars:
79: label L23
# live vars:
80: return ()
}

============ after ReferenceSafetyProcessor: ================

[variant baseline]
Expand Down
Loading

0 comments on commit 3d4d746

Please sign in to comment.