Skip to content

Commit

Permalink
[move] Added warnings-as-errors flag. Fix warnings in stdlib/sui fram…
Browse files Browse the repository at this point in the history
…ework. (MystenLabs#14616)

## Description 

- Added two new compiler flags for silencing all warnings and for making
warnings errors
- Turned on warnings as errors for all sui-framework related builds
- Fixed the countless errors/warnings in the Sui framework
- Some breaking changes to spelling in unit test flags

## Test Plan 

- Added tests 

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [X] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

The Move compiler now supports silencing all warnings and treating all
warnings as errors. Unused mutable reference warnings have been silenced
for public functions.
Additionally, the spelling for some unit test flags has changed (many
changing from snake to spine case).
  • Loading branch information
tnowacki authored Nov 6, 2023
1 parent c44a5dc commit 1d5cfd7
Show file tree
Hide file tree
Showing 145 changed files with 816 additions and 710 deletions.
24 changes: 15 additions & 9 deletions crates/sui-framework-tests/src/metered_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,27 @@ use prometheus::Registry;
use std::{path::PathBuf, sync::Arc, time::Instant};
use sui_adapter::adapter::{default_verifier_config, run_metered_move_bytecode_verifier};
use sui_framework::BuiltInFramework;
use sui_move_build::{BuildConfig, SuiPackageHooks};
use sui_move_build::{CompiledPackage, SuiPackageHooks};
use sui_protocol_config::ProtocolConfig;
use sui_types::{error::SuiError, metrics::BytecodeVerifierMetrics};
use sui_types::{
error::{SuiError, SuiResult},
metrics::BytecodeVerifierMetrics,
};
use sui_verifier::meter::SuiVerifierMeter;

fn build(path: PathBuf) -> SuiResult<CompiledPackage> {
let mut config = sui_move_build::BuildConfig::new_for_testing();
config.config.warnings_are_errors = true;
config.build(path)
}

#[test]
#[cfg_attr(msim, ignore)]
fn test_metered_move_bytecode_verifier() {
move_package::package_hooks::register_package_hooks(Box::new(SuiPackageHooks));
let path =
PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../sui-framework/packages/sui-framework");
let compiled_package = BuildConfig::new_for_testing().build(path).unwrap();
let compiled_package = build(path).unwrap();
let compiled_modules: Vec<_> = compiled_package.get_modules().cloned().collect();

let mut metered_verifier_config = default_verifier_config(
Expand Down Expand Up @@ -203,13 +212,13 @@ fn test_metered_move_bytecode_verifier() {
let with_unpublished_deps = false;
let path =
PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../sui_programmability/examples/basics");
let package = BuildConfig::new_for_testing().build(path).unwrap();
let package = build(path).unwrap();
packages.push(package.get_dependency_sorted_modules(with_unpublished_deps));
packages.push(package.get_dependency_sorted_modules(with_unpublished_deps));

let path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
.join("../../sui_programmability/examples/fungible_tokens");
let package = BuildConfig::new_for_testing().build(path).unwrap();
let package = build(path).unwrap();
packages.push(package.get_dependency_sorted_modules(with_unpublished_deps));

let is_metered = true;
Expand Down Expand Up @@ -324,10 +333,7 @@ fn test_build_and_verify_programmability_examples() {
continue;
};

let modules = BuildConfig::new_for_testing()
.build(path)
.unwrap()
.into_modules();
let modules = build(path).unwrap().into_modules();

let mut meter = SuiVerifierMeter::new(&metered_verifier_config);
run_metered_move_bytecode_verifier(
Expand Down
4 changes: 4 additions & 0 deletions crates/sui-framework-tests/src/unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ fn check_package_builds(path: PathBuf) {
config.config.dev_mode = true;
config.run_bytecode_verifier = true;
config.print_diags_to_stderr = true;
config.config.warnings_are_errors = true;
config.config.silence_warnings = false;

config
.build(path.clone())
Expand All @@ -113,6 +115,8 @@ fn check_move_unit_tests(path: PathBuf) {
config.config.test_mode = true;
config.run_bytecode_verifier = true;
config.print_diags_to_stderr = true;
config.config.warnings_are_errors = true;
config.config.silence_warnings = false;
let move_config = config.config.clone();
let testing_config = UnitTestingConfig::default_with_bound(Some(3_000_000));

Expand Down
2 changes: 2 additions & 0 deletions crates/sui-framework/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ fn build_packages(
) {
let config = MoveBuildConfig {
generate_docs: true,
warnings_are_errors: true,
install_dir: Some(PathBuf::from(".")),
..Default::default()
};
Expand All @@ -103,6 +104,7 @@ fn build_packages(
let config = MoveBuildConfig {
generate_docs: true,
test_mode: true,
warnings_are_errors: true,
install_dir: Some(PathBuf::from(".")),
..Default::default()
};
Expand Down
18 changes: 9 additions & 9 deletions crates/sui-framework/packages/deepbook/sources/clob_v2.move
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ module deepbook::clob_v2 {

let pool_uid = object::new(ctx);
let pool_id = *object::uid_as_inner(&pool_uid);

event::emit(PoolCreated {
pool_id,
base_asset: base_type_name,
Expand Down Expand Up @@ -1901,7 +1901,7 @@ module deepbook::clob_v2 {
balance::create_for_testing(FEE_AMOUNT_FOR_CREATE_POOL),
test_scenario::ctx(scenario)
);
// let pool =
// let pool =
transfer::share_object(WrappedPool {
id: object::new(test_scenario::ctx(scenario)),
pool
Expand Down Expand Up @@ -2264,7 +2264,7 @@ module deepbook::clob_v2 {
};
test_scenario::next_tx(&mut test, alice);
{
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&mut test);
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&test);
let clock = test_scenario::take_shared<Clock>(&test);
let account_cap = test_scenario::take_from_address<AccountCap>(&test, alice);
let account_cap_user = account_owner(&account_cap);
Expand Down Expand Up @@ -2326,7 +2326,7 @@ module deepbook::clob_v2 {
};
test_scenario::next_tx(&mut test, alice);
{
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&mut test);
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&test);
let clock = test_scenario::take_shared<Clock>(&test);
let account_cap = test_scenario::take_from_address<AccountCap>(&test, alice);
let account_cap_user = account_owner(&account_cap);
Expand Down Expand Up @@ -2409,7 +2409,7 @@ module deepbook::clob_v2 {

test_scenario::next_tx(&mut test, bob);
{
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&mut test);
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&test);
let clock = test_scenario::take_shared<Clock>(&test);
let account_cap = test_scenario::take_from_address<AccountCap>(&test, bob);
let account_cap_user = account_owner(&account_cap);
Expand Down Expand Up @@ -2467,7 +2467,7 @@ module deepbook::clob_v2 {
};
test_scenario::next_tx(&mut test, alice);
{
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&mut test);
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&test);
let clock = test_scenario::take_shared<Clock>(&test);
let account_cap = test_scenario::take_from_address<AccountCap>(&test, alice);
let account_cap_user = account_owner(&account_cap);
Expand Down Expand Up @@ -2550,7 +2550,7 @@ module deepbook::clob_v2 {

test_scenario::next_tx(&mut test, bob);
{
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&mut test);
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&test);
let clock = test_scenario::take_shared<Clock>(&test);
let account_cap = test_scenario::take_from_address<AccountCap>(&test, bob);
let account_cap_user = account_owner(&account_cap);
Expand Down Expand Up @@ -2607,7 +2607,7 @@ module deepbook::clob_v2 {
};
test_scenario::next_tx(&mut test, alice);
{
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&mut test);
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&test);
let clock = test_scenario::take_shared<Clock>(&test);
let account_cap = test_scenario::take_from_address<AccountCap>(&test, alice);
let account_cap_user = account_owner(&account_cap);
Expand Down Expand Up @@ -2696,7 +2696,7 @@ module deepbook::clob_v2 {

test_scenario::next_tx(&mut test, bob);
{
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&mut test);
let pool = test_scenario::take_shared<Pool<SUI, USD>>(&test);
let clock = test_scenario::take_shared<Clock>(&test);
let account_cap = test_scenario::take_from_address<AccountCap>(&test, bob);
let account_cap_user = account_owner(&account_cap);
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-framework/packages/deepbook/sources/custodian.move
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ module deepbook::custodian {
};
test_scenario::next_tx(&mut test, bob);
{
let custodian = take_shared<Custodian<USD>>(&mut test);
let custodian = take_shared<Custodian<USD>>(&test);
let account_cap = take_from_sender<AccountCap>(&test);
let account_cap_user = object::id(&account_cap);
let _ = borrow_account_balance(&custodian, account_cap_user);
Expand All @@ -249,7 +249,7 @@ module deepbook::custodian {
};
test_scenario::next_tx(&mut test, bob);
{
let custodian = take_shared<Custodian<USD>>(&mut test);
let custodian = take_shared<Custodian<USD>>(&test);
let account_cap = take_from_sender<AccountCap>(&test);
let account_cap_user = object::id(&account_cap);
let (asset_available, asset_locked) = account_balance(&custodian, account_cap_user);
Expand All @@ -261,7 +261,7 @@ module deepbook::custodian {
};
test_scenario::next_tx(&mut test, bob);
{
let custodian = take_shared<Custodian<USD>>(&mut test);
let custodian = take_shared<Custodian<USD>>(&test);
let account_cap = take_from_sender<AccountCap>(&test);
let account_cap_user = object::id(&account_cap);
deposit(&mut custodian, mint_for_testing<USD>(10000, ctx(&mut test)), account_cap_user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ module deepbook::custodian_v2 {
};
test_scenario::next_tx(&mut test, bob);
{
let custodian = take_shared<Custodian<USD>>(&mut test);
let custodian = take_shared<Custodian<USD>>(&test);
let account_cap = take_from_sender<AccountCap>(&test);
let _ = borrow_account_balance(&custodian, bob);
test_scenario::return_to_sender<AccountCap>(&test, account_cap);
Expand All @@ -290,7 +290,7 @@ module deepbook::custodian_v2 {
};
test_scenario::next_tx(&mut test, bob);
{
let custodian = take_shared<Custodian<USD>>(&mut test);
let custodian = take_shared<Custodian<USD>>(&test);
let account_cap = take_from_sender<AccountCap>(&test);
let (asset_available, asset_locked) = account_balance(&custodian, bob);
assert_eq(asset_available, 0);
Expand All @@ -301,7 +301,7 @@ module deepbook::custodian_v2 {
};
test_scenario::next_tx(&mut test, bob);
{
let custodian = take_shared<Custodian<USD>>(&mut test);
let custodian = take_shared<Custodian<USD>>(&test);
let account_cap = take_from_sender<AccountCap>(&test);
deposit(&mut custodian, mint_for_testing<USD>(10000, ctx(&mut test)), bob);
let (asset_available, asset_locked) = account_balance(&custodian, bob);
Expand Down
Loading

0 comments on commit 1d5cfd7

Please sign in to comment.