Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sui-mode][move-compiler] Greatly improve shared_owned lint accuracy #19330

Merged
merged 5 commits into from
Sep 12, 2024
Merged

[sui-mode][move-compiler] Greatly improve shared_owned lint accuracy #19330

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bfcdd76
refine share_owned lint
tnowacki Aug 28, 2024
3f5c63a
fix tests
tnowacki Sep 5, 2024
4710001
clippy
tnowacki Sep 6, 2024
cbf7c12
fix tests
tnowacki Sep 11, 2024
90f9d28
comments
tnowacki Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
clippy
  • Loading branch information
@tnowacki
tnowacki committed Sep 11, 2024
commit 4710001f39d0ab7aed1b74d92a3d1eb1b6d062e1
75 changes: 74 additions & 1 deletion external-crates/move/crates/move-compiler/src/cfgir/visitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub trait AbstractInterpreterVisitor {
}

//**************************************************************************************************
// simple ast visitor
// CFGIR visitor
//**************************************************************************************************

pub trait CFGIRVisitorConstructor {
Expand Down Expand Up @@ -768,3 +768,76 @@ impl<V: SimpleAbsIntConstructor> AbstractInterpreterVisitor for V {
SimpleAbsIntConstructor::verify(self, env, context, cfg)
}
}

//**************************************************************************************************
// utils
//**************************************************************************************************

pub fn calls_special_function(special: &[(&str, &str, &str)], cfg: &ImmForwardCFG) -> bool {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These will be generalized a bit in the next PR

cfg.blocks().values().any(|block| {
block
.iter()
.any(|cmd| calls_special_function_command(special, cmd))
})
}

pub fn calls_special_function_command(
special: &[(&str, &str, &str)],
sp!(_, cmd_): &Command,
) -> bool {
use H::Command_ as C;
match cmd_ {
C::Assign(_, _, e)
| C::Abort(_, e)
| C::Return { exp: e, .. }
| C::IgnoreAndPop { exp: e, .. }
| C::JumpIf { cond: e, .. }
| C::VariantSwitch { subject: e, .. } => calls_special_function_exp(special, e),
C::Mutate(el, er) => {
calls_special_function_exp(special, el) || calls_special_function_exp(special, er)
}
C::Jump { .. } => false,
C::Break(_) | C::Continue(_) => panic!("ICE break/continue not translated to jumps"),
}
}

#[growing_stack]
pub fn calls_special_function_exp(special: &[(&str, &str, &str)], e: &Exp) -> bool {
use H::UnannotatedExp_ as E;
match &e.exp.value {
E::Unit { .. }
| E::Move { .. }
| E::Copy { .. }
| E::Constant(_)
| E::ErrorConstant { .. }
| E::BorrowLocal(_, _)
| E::Unreachable
| E::UnresolvedError
| E::Value(_) => false,

E::Freeze(e)
| E::Dereference(e)
| E::UnaryExp(_, e)
| E::Borrow(_, e, _, _)
| E::Cast(e, _) => calls_special_function_exp(special, e),

E::BinopExp(el, _, er) => {
calls_special_function_exp(special, el) || calls_special_function_exp(special, er)
}

E::ModuleCall(call) => {
special.iter().any(|(a, m, f)| call.is(*a, *m, *f))
|| call
.arguments
.iter()
.any(|arg| calls_special_function_exp(special, arg))
}
E::Vector(_, _, _, es) | E::Multiple(es) => {
es.iter().any(|e| calls_special_function_exp(special, e))
}

E::Pack(_, _, es) | E::PackVariant(_, _, _, es) => es
.iter()
.any(|(_, _, e)| calls_special_function_exp(special, e)),
}
}
81 changes: 8 additions & 73 deletions external-crates/move/crates/move-compiler/src/sui_mode/linters/share_owned.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use crate::{
absint::JoinResult,
cfg::ImmForwardCFG,
visitor::{
LocalState, SimpleAbsInt, SimpleAbsIntConstructor, SimpleDomain, SimpleExecutionContext,
calls_special_function, LocalState, SimpleAbsInt, SimpleAbsIntConstructor,
SimpleDomain, SimpleExecutionContext,
},
CFGContext,
},
Expand All @@ -24,8 +25,8 @@ use crate::{
},
expansion::ast::ModuleIdent,
hlir::ast::{
BaseType, BaseType_, Command, Command_, Exp, LValue, LValue_, Label, ModuleCall,
SingleType, SingleType_, Type, TypeName_, Type_, UnannotatedExp_, Var,
BaseType, BaseType_, Exp, LValue, LValue_, Label, ModuleCall, SingleType, SingleType_,
Type, TypeName_, Type_, UnannotatedExp_, Var,
},
naming::ast::BuiltinTypeName_,
parser::ast::{Ability_, DatatypeName},
Expand Down Expand Up @@ -116,72 +117,6 @@ impl SimpleAbsIntConstructor for ShareOwnedVerifier {
}
}

fn calls_special_function(special: &[(&str, &str, &str)], cfg: &ImmForwardCFG) -> bool {
cfg.blocks().values().any(|block| {
block
.iter()
.any(|cmd| calls_special_function_command(special, cmd))
})
}

fn calls_special_function_command(special: &[(&str, &str, &str)], sp!(_, cmd_): &Command) -> bool {
use Command_ as C;
match cmd_ {
C::Assign(_, _, e)
| C::Abort(e)
| C::Return { exp: e, .. }
| C::IgnoreAndPop { exp: e, .. }
| C::JumpIf { cond: e, .. }
| C::VariantSwitch { subject: e, .. } => calls_special_function_exp(special, e),
C::Mutate(el, er) => {
calls_special_function_exp(special, el) || calls_special_function_exp(special, er)
}
C::Jump { .. } => false,
C::Break(_) | C::Continue(_) => panic!("ICE break/continue not translated to jumps"),
}
}

#[growing_stack]
fn calls_special_function_exp(special: &[(&str, &str, &str)], e: &Exp) -> bool {
use UnannotatedExp_ as E;
match &e.exp.value {
E::Unit { .. }
| E::Move { .. }
| E::Copy { .. }
| E::Constant(_)
| E::ErrorConstant { .. }
| E::BorrowLocal(_, _)
| E::Unreachable
| E::UnresolvedError
| E::Value(_) => false,

E::Freeze(e)
| E::Dereference(e)
| E::UnaryExp(_, e)
| E::Borrow(_, e, _, _)
| E::Cast(e, _) => calls_special_function_exp(special, e),

E::BinopExp(el, _, er) => {
calls_special_function_exp(special, el) || calls_special_function_exp(special, er)
}

E::ModuleCall(call) => {
special.iter().any(|(a, m, f)| call.is(*a, *m, *f))
|| call
.arguments
.iter()
.any(|arg| calls_special_function_exp(special, arg))
}
E::Vector(_, _, _, es) | E::Multiple(es) => {
es.iter().any(|e| calls_special_function_exp(special, e))
}

E::Pack(_, _, es) | E::PackVariant(_, _, _, es) => es
.iter()
.any(|(_, _, e)| calls_special_function_exp(special, e)),
}
}

impl<'a> SimpleAbsInt for ShareOwnedVerifierAI<'a> {
type State = State;
type ExecutionContext = ExecutionContext;
Expand Down Expand Up @@ -348,7 +283,7 @@ impl<'a> ShareOwnedVerifierAI<'a> {
};
let Some(tn) = f
.type_arguments
.get(0)
.first()
.and_then(|t| t.value.type_name())
.and_then(|n| n.value.datatype_name())
else {
Expand Down Expand Up @@ -405,9 +340,9 @@ fn phantom_positions(
m: &ModuleIdent,
n: &DatatypeName,
) -> Vec</* is_phantom */ bool> {
let ty_params = match info.datatype_kind(&m, &n) {
DatatypeKind::Struct => &info.struct_definition(&m, &n).type_parameters,
DatatypeKind::Enum => &info.enum_definition(&m, &n).type_parameters,
let ty_params = match info.datatype_kind(m, n) {
DatatypeKind::Struct => &info.struct_definition(m, n).type_parameters,
DatatypeKind::Enum => &info.enum_definition(m, n).type_parameters,
};
ty_params.iter().map(|tp| tp.is_phantom).collect()
}
Expand Down
8 changes: 2 additions & 6 deletions external-crates/move/crates/move-compiler/src/typing/translate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,8 @@ pub fn program(
.into_iter()
.map(|(mident, minfo)| (mident, minfo.use_funs))
.collect();
let module_info = TypingProgramInfo::new(
&compilation_env,
pre_compiled_lib,
&modules,
module_use_funs,
);
let module_info =
TypingProgramInfo::new(compilation_env, pre_compiled_lib, &modules, module_use_funs);
let prog = T::Program {
modules,
info: Arc::new(module_info),
Expand Down