Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
awelc committed Jun 27, 2023
1 parent 8c9bbd2 commit 79013e9
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 17 deletions.
3 changes: 3 additions & 0 deletions crates/sui-framework/build.rs
Expand Up @@ -134,20 +134,23 @@ fn build_packages_with_move_config(
config: config.clone(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
lint: false,
}
.build(sui_framework_path)
.unwrap();
let system_pkg = BuildConfig {
config: config.clone(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
lint: false,
}
.build(sui_system_path)
.unwrap();
let deepbook_pkg = BuildConfig {
config,
run_bytecode_verifier: true,
print_diags_to_stderr: false,
lint: false,
}
.build(deepbook_path)
.unwrap();
Expand Down
8 changes: 4 additions & 4 deletions crates/sui-move-build/src/lib.rs
Expand Up @@ -81,6 +81,8 @@ pub struct BuildConfig {
pub run_bytecode_verifier: bool,
/// If true, print build diagnostics to stderr--no printing if false
pub print_diags_to_stderr: bool,
/// If true, run linters
pub lint: bool,
}

impl BuildConfig {
Expand Down Expand Up @@ -157,10 +159,7 @@ impl BuildConfig {
/// Given a `path` and a `build_config`, build the package in that path, including its dependencies.
/// If we are building the Sui framework, we skip the check that the addresses should be 0
pub fn build(self, path: PathBuf) -> SuiResult<CompiledPackage> {
self.build_and_lint(path, false)
}

pub fn build_and_lint(self, path: PathBuf, lint: bool) -> SuiResult<CompiledPackage> {
let lint = self.lint;
let print_diags_to_stderr = self.print_diags_to_stderr;
let run_bytecode_verifier = self.run_bytecode_verifier;
let resolution_graph = self.resolution_graph(&path)?;
Expand Down Expand Up @@ -560,6 +559,7 @@ impl Default for BuildConfig {
config: MoveBuildConfig::default(),
run_bytecode_verifier: true,
print_diags_to_stderr: false,
lint: false,
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions crates/sui-move-build/src/linters/share_owned.rs
Expand Up @@ -84,7 +84,7 @@ impl SimpleAbsIntConstructor for ShareOwnedVerifier {
let Some(_) = &context.module else {
return None
};
Some(ShareOwnedVerifierAI {})
Some(ShareOwnedVerifierAI)
}
}

Expand Down Expand Up @@ -139,15 +139,15 @@ impl SimpleAbsInt for ShareOwnedVerifierAI {
.any(|(addr, module, fun)| f.is(addr, module, fun))
&& args[0] != Value::FreshObj
{
let msg = "You may be trying to share an owned object.";
let uid_msg = "This argument should in most cases come from an object created within the same function.";
let msg = "Potential abort from a (potentially) owned object created by a different transaction.";
let uid_msg = "Creating a fresh object and sharing it within the same function will ensure this does not abort.";
let mut d = diag!(
SHARE_OWNED_DIAG,
(*loc, msg),
(f.arguments.exp.loc, uid_msg)
);
if let Value::NotFreshObj(l) = args[0] {
d.add_secondary_label((l, "A potentially shared object coming from here"))
d.add_secondary_label((l, "A potentially owned object coming from here"))
}
context.add_diag(d)
}
Expand Down Expand Up @@ -191,9 +191,9 @@ impl SimpleAbsInt for ShareOwnedVerifierAI {
if let L::Unpack(_, _, fields) = l_ {
for (f, l) in fields {
let v = if is_obj(l) {
<Self::State as SimpleDomain>::Value::NotFreshObj(f.loc())
Value::NotFreshObj(f.loc())
} else {
<Self::State as SimpleDomain>::Value::default()
Value::default()
};
self.lvalue(context, state, l, v)
}
Expand Down
12 changes: 6 additions & 6 deletions crates/sui-move-build/tests/linter/share_owned.exp
Expand Up @@ -2,22 +2,22 @@ warning[Lint W01001]: possible owned object share
┌─ tests/linter/share_owned.move:14:9
12 │ public entry fun arg_object(o: Obj) {
│ - A potentially shared object coming from here
│ - A potentially owned object coming from here
13 │ let arg = o;
14 │ transfer::public_share_object(arg);
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│ │ │
│ │ This argument should in most cases come from an object created within the same function.
You may be trying to share an owned object.
│ │ Creating a fresh object and sharing it within the same function will ensure this does not abort.
Potential abort from a (potentially) owned object created by a different transaction.

warning[Lint W01001]: possible owned object share
┌─ tests/linter/share_owned.move:35:9
34 │ let Wrapper { id, i: _, o } = w;
│ - A potentially shared object coming from here
│ - A potentially owned object coming from here
35 │ transfer::public_share_object(o);
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│ │ │
│ │ This argument should in most cases come from an object created within the same function.
You may be trying to share an owned object.
│ │ Creating a fresh object and sharing it within the same function will ensure this does not abort.
Potential abort from a (potentially) owned object created by a different transaction.

3 changes: 2 additions & 1 deletion crates/sui-move/src/build.rs
Expand Up @@ -69,8 +69,9 @@ impl Build {
config,
run_bytecode_verifier: true,
print_diags_to_stderr: true,
lint,
}
.build_and_lint(rerooted_path, lint)?;
.build(rerooted_path)?;
if dump_bytecode_as_base64 {
check_invalid_dependencies(&pkg.dependency_ids.invalid)?;
if !with_unpublished_deps {
Expand Down
3 changes: 3 additions & 0 deletions crates/sui/src/client_commands.rs
Expand Up @@ -1245,6 +1245,7 @@ impl SuiClientCommands {
config: build_config,
run_bytecode_verifier: true,
print_diags_to_stderr: true,
lint: false,
}
.build(package_path)?;

Expand Down Expand Up @@ -1284,6 +1285,7 @@ fn compile_package_simple(
config: resolve_lock_file_path(build_config, Some(package_path.clone()))?,
run_bytecode_verifier: false,
print_diags_to_stderr: false,
lint: false,
};
let resolution_graph = config.resolution_graph(&package_path)?;

Expand Down Expand Up @@ -1319,6 +1321,7 @@ async fn compile_package(
config,
run_bytecode_verifier,
print_diags_to_stderr,
lint,
};
let resolution_graph = config.resolution_graph(&package_path)?;
let (package_id, dependencies) = gather_published_ids(&resolution_graph);
Expand Down

0 comments on commit 79013e9

Please sign in to comment.