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.

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

Honor #[verify_only] in move-compiler-v2. Add relevant tests from v1. Also add tests of #[deprecated]. #13732

Merged
merged 11 commits into from
Jun 29, 2024
Merged
3 changes: 3 additions & 0 deletions third_party/move/move-compiler-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,10 @@ pub fn run_checker(options: Options) -> anyhow::Result<GlobalEnv> {
&options.known_attributes
},
options.language_version.unwrap_or_default(),
options.warn_deprecated,
options.warn_of_deprecation_use_in_aptos_libs,
options.compile_test_code,
options.compile_verify_code,
)?;
// Store address aliases
let map = addrs
Expand Down
66 changes: 64 additions & 2 deletions third_party/move/move-compiler-v2/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@
use clap::Parser;
use codespan_reporting::diagnostic::Severity;
use itertools::Itertools;
use move_command_line_common::env::read_env_var;
use move_compiler::command_line as cli;
use move_command_line_common::env::{bool_to_str, read_env_var};
use move_compiler::{
command_line as cli,
shared::{
move_compiler_warn_of_deprecation_use_env_var,
warn_of_deprecation_use_in_aptos_libs_env_var,
},
};
use move_model::metadata::LanguageVersion;
use once_cell::sync::Lazy;
use std::{
Expand All @@ -25,29 +31,36 @@
num_args = 0..
)]
pub dependencies: Vec<String>,

Copy link
Contributor

Choose a reason for hiding this comment

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

Has this been done by the auto formatter? Please note that there is no agreement in general what is the better style, it actually pretty much depends on the IDE and font you are using. Its not necessarily better now. I would leave such decisions of formatting at the author and not enforce my private style tastes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just can't read it the other way. Call it an accessibility issue for the partially-blind.

/// Named address mapping.
#[clap(
short,
num_args = 0..
)]
pub named_address_mapping: Vec<String>,

/// Output directory.
#[clap(short, long, default_value = "")]
pub output_dir: String,

/// The language version to use.
#[clap(long, value_parser = clap::value_parser!(LanguageVersion))]
pub language_version: Option<LanguageVersion>,

/// Do not complain about unknown attributes in Move code.
#[clap(long, default_value = "false")]
pub skip_attribute_checks: bool,

/// Known attributes for this dialect of move; if empty, assumes third-party Move.
/// Only used if skip_attribute_checks is false.
#[clap(skip)]
pub known_attributes: BTreeSet<String>,

/// Whether we generate code for tests. This specifically guarantees stable output
/// for baseline testing.
#[clap(long)]
pub testing: bool,

/// Active experiments. Experiments alter default behavior of the compiler.
/// See `Experiment` struct.
#[clap(short)]
Expand All @@ -56,31 +69,52 @@
num_args = 0..
)]
pub experiments: Vec<String>,

/// A transient cache for memoization of experiment checks.
#[clap(skip)]
pub experiment_cache: RefCell<BTreeMap<String, bool>>,

/// Sources to compile (positional arg, therefore last).
/// Each source should be a path to either (1) a Move file or (2) a directory containing Move
/// files, all to be compiled (e.g., not the root directory of a package---which contains
/// Move.toml---but a specific subdirectorysuch as `sources`, `scripts`, and/or `tests`,
/// depending on compilation mode).
pub sources: Vec<String>,

/// Dependencies to compile but not treat as a test/docgen/warning/prover target.
/// Each source_dep should be a path to either (1) a Move file or (2) a directory containing
/// Move files, all to be compiled (e.g., not the root directory of a package---which contains
/// Move.toml---but a specific subdirectorysuch as `sources`).
#[clap(skip)]
pub sources_deps: Vec<String>,

#[clap(long = cli::MOVE_COMPILER_WARN_OF_DEPRECATION_USE_FLAG,
default_value=bool_to_str(move_compiler_warn_of_deprecation_use_env_var()))]
pub warn_deprecated: bool,

Check warning on line 93 in third_party/move/move-compiler-v2/src/options.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/options.rs#L93

Added line #L93 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering how this relates to the title of the PR. Should you update the title?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it. Though it's the final commit message that really matters.


/// Show warnings about use of deprecated usage in the Aptos libraries,
/// which we should generally not bother users with.
/// Note that current value of this constant is "Wdeprecation-aptos"
#[clap(long = cli::WARN_OF_DEPRECATION_USE_IN_APTOS_LIBS_FLAG,
default_value=bool_to_str(warn_of_deprecation_use_in_aptos_libs_env_var()))]
pub warn_of_deprecation_use_in_aptos_libs: bool,

Check warning on line 100 in third_party/move/move-compiler-v2/src/options.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/options.rs#L100

Added line #L100 was not covered by tests

/// Show warnings about unused functions, fields, constants, etc.
/// Note that the current value of this constant is "Wunused"
#[clap(long = cli::WARN_UNUSED_FLAG, default_value="false")]
pub warn_unused: bool,

/// Whether to compile everything, including dependencies.
#[clap(long)]
pub whole_program: bool,

/// Whether to compile #[test] and #[test_only] code
#[clap(skip)]
pub compile_test_code: bool,

/// Whether to compile #[verify_only] code
#[clap(skip)]
pub compile_verify_code: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you point to where this flag is used apart from unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

run_move_prover_v2(), in third_party/move/move-prover/src/lib.rs.

}

impl Default for Options {
Expand Down Expand Up @@ -169,6 +203,34 @@
..self
}
}

pub fn set_compile_verify_code(self, value: bool) -> Self {
Self {
compile_verify_code: value,
..self
}
}

Check warning on line 212 in third_party/move/move-compiler-v2/src/options.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/options.rs#L207-L212

Added lines #L207 - L212 were not covered by tests

pub fn set_warn_deprecated(self, value: bool) -> Self {
Self {
warn_deprecated: value,
..self
}
}

Check warning on line 219 in third_party/move/move-compiler-v2/src/options.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/options.rs#L214-L219

Added lines #L214 - L219 were not covered by tests

pub fn set_warn_of_deprecation_use_in_aptos_libs(self, value: bool) -> Self {
Self {
warn_of_deprecation_use_in_aptos_libs: value,
..self
}
}

Check warning on line 226 in third_party/move/move-compiler-v2/src/options.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/options.rs#L221-L226

Added lines #L221 - L226 were not covered by tests

pub fn set_warn_unused(self, value: bool) -> Self {
Self {
warn_unused: value,
..self
}
}

Check warning on line 233 in third_party/move/move-compiler-v2/src/options.rs

View check run for this annotation

Codecov / codecov/patch

third_party/move/move-compiler-v2/src/options.rs#L228-L233

Added lines #L228 - L233 were not covered by tests
}

/// Finds the experiment in the list of definitions. A definition
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

Diagnostics:
error: duplicate declaration, item, or annotation
┌─ tests/deprecated/deprecated_constant_duplicated_struct.move:5:11
4 │ struct C { }
│ - Alias previously defined here
5 │ const C: u64 = 0;
│ ^ Duplicate module member or alias 'C'. Top level names in a namespace must be unique

warning: unused alias
┌─ tests/deprecated/deprecated_constant_duplicated_struct.move:12:15
12 │ use 0x42::mod1;
│ ^^^^ Unused 'use' of alias 'mod1'. Consider removing it

warning: unused alias
┌─ tests/deprecated/deprecated_constant_duplicated_struct.move:31:15
31 │ use 0x42::mod1;
│ ^^^^ Unused 'use' of alias 'mod1'. Consider removing it

error: invalid name
┌─ tests/deprecated/deprecated_constant_duplicated_struct.move:32:26
32 │ use 0x42::mod1::C as mod1;
│ ^^^^ Invalid constant alias name 'mod1'. Constant alias names must start with 'A'..'Z'
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
address 0x42 {
module mod1 {
#[deprecated]
struct C { }
const C: u64 = 0;
public fun mod1() {}
}
}

address 0x41 {
module N {
use 0x42::mod1;
use 0x42::mod1::C as D;
use 0x42::mod1::C as C;
use 0x42::mod1::mod1;

fun f1(): 0x42::mod1::C {
mod1();
C;
{
use 0x42::mod1::C;
C
};
D
}
}
}


script {
use 0x42::mod1;
use 0x42::mod1::C as mod1;
use 0x42::mod1::C as C;
use 0x42::mod1::mod1;

fun f1(): 0x42::mod1::C {
mod1();
C;
{
use 0x42::mod1::C;
C
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@

Diagnostics:
error: duplicate declaration, item, or annotation
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:5:11
3 │ struct C { }
│ - Alias previously defined here
4 │ #[deprecated]
5 │ const C: u64 = 0;
│ ^ Duplicate module member or alias 'C'. Top level names in a namespace must be unique

warning: unused alias
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:12:15
12 │ use 0x42::mod1;
│ ^^^^ Unused 'use' of alias 'mod1'. Consider removing it

warning: Use of deprecated constant
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:13:21
4 │ #[deprecated]
│ ---------- Constant 'C' in module '0x42::mod1' deprecated here
·
13 │ use 0x42::mod1::C as D;
│ ^ Use of deprecated constant 'C' from module '0x42::mod1'

warning: Use of deprecated constant
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:14:21
4 │ #[deprecated]
│ ---------- Constant 'C' in module '0x42::mod1' deprecated here
·
14 │ use 0x42::mod1::C as C;
│ ^ Use of deprecated constant 'C' from module '0x42::mod1'

warning: Use of deprecated struct
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:17:27
4 │ #[deprecated]
│ ---------- Struct 'C' in module '0x42::mod1' deprecated here
·
17 │ fun f1(): 0x42::mod1::C {
│ ^ Use of deprecated struct 'C' from module '0x42::mod1'

warning: Use of deprecated member
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:19:2
4 │ #[deprecated]
│ ---------- Member 'C' in module '0x42::mod1' deprecated here
·
19 │ C;
│ ^ Use of deprecated member 'C' from module '0x42::mod1'

warning: Use of deprecated constant
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:21:22
4 │ #[deprecated]
│ ---------- Constant 'C' in module '0x42::mod1' deprecated here
·
21 │ use 0x42::mod1::C;
│ ^ Use of deprecated constant 'C' from module '0x42::mod1'

warning: Use of deprecated member
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:22:6
4 │ #[deprecated]
│ ---------- Member 'C' in module '0x42::mod1' deprecated here
·
22 │ C
│ ^ Use of deprecated member 'C' from module '0x42::mod1'

warning: Use of deprecated member
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:24:2
4 │ #[deprecated]
│ ---------- Member 'C' in module '0x42::mod1' deprecated here
·
24 │ D
│ ^ Use of deprecated member 'C' from module '0x42::mod1'

warning: unused alias
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:31:15
31 │ use 0x42::mod1;
│ ^^^^ Unused 'use' of alias 'mod1'. Consider removing it

warning: Use of deprecated constant
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:32:21
4 │ #[deprecated]
│ ---------- Constant 'C' in module '0x42::mod1' deprecated here
·
32 │ use 0x42::mod1::C as mod1;
│ ^ Use of deprecated constant 'C' from module '0x42::mod1'

error: invalid name
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:32:26
32 │ use 0x42::mod1::C as mod1;
│ ^^^^ Invalid constant alias name 'mod1'. Constant alias names must start with 'A'..'Z'

warning: Use of deprecated constant
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:33:21
4 │ #[deprecated]
│ ---------- Constant 'C' in module '0x42::mod1' deprecated here
·
33 │ use 0x42::mod1::C as C;
│ ^ Use of deprecated constant 'C' from module '0x42::mod1'

warning: Use of deprecated struct
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:36:27
4 │ #[deprecated]
│ ---------- Struct 'C' in module '0x42::mod1' deprecated here
·
36 │ fun f1(): 0x42::mod1::C {
│ ^ Use of deprecated struct 'C' from module '0x42::mod1'

warning: Use of deprecated member
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:38:2
4 │ #[deprecated]
│ ---------- Member 'C' in module '0x42::mod1' deprecated here
·
38 │ C;
│ ^ Use of deprecated member 'C' from module '0x42::mod1'

warning: Use of deprecated constant
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:40:22
4 │ #[deprecated]
│ ---------- Constant 'C' in module '0x42::mod1' deprecated here
·
40 │ use 0x42::mod1::C;
│ ^ Use of deprecated constant 'C' from module '0x42::mod1'

warning: Use of deprecated member
┌─ tests/deprecated/deprecated_constant_duplicated_struct2.move:41:6
4 │ #[deprecated]
│ ---------- Member 'C' in module '0x42::mod1' deprecated here
·
41 │ C
│ ^ Use of deprecated member 'C' from module '0x42::mod1'
Loading
Loading