Skip to content

Commit

Permalink
[move-compiler-v2] reorganize/simplify options in aptos-move CLI, add…
Browse files Browse the repository at this point in the history
…ing lint and --optimization-level={none,default,extra} flags (#14427)
  • Loading branch information
brmataptos authored Aug 28, 2024
1 parent 189e810 commit de33d9c
Show file tree
Hide file tree
Showing 18 changed files with 444 additions and 196 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions aptos-move/framework/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ move-binary-format = { workspace = true }
move-cli = { workspace = true }
move-command-line-common = { workspace = true }
move-compiler = { workspace = true }
move-compiler-v2 = { workspace = true }
move-core-types = { workspace = true }
move-docgen = { workspace = true }
move-model = { workspace = true }
Expand Down
20 changes: 3 additions & 17 deletions aptos-move/framework/src/aptos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@
#![forbid(unsafe_code)]

use crate::{
docgen::DocgenOptions, extended_checks, path_in_crate,
release_builder::RELEASE_BUNDLE_EXTENSION, release_bundle::ReleaseBundle, BuildOptions,
ReleaseOptions,
docgen::DocgenOptions, path_in_crate, release_builder::RELEASE_BUNDLE_EXTENSION,
release_bundle::ReleaseBundle, BuildOptions, ReleaseOptions,
};
use clap::ValueEnum;
use move_command_line_common::address::NumericalAddress;
Expand Down Expand Up @@ -100,14 +99,8 @@ impl ReleaseTarget {
.collect::<Vec<_>>();
ReleaseOptions {
build_options: BuildOptions {
dev: false,
with_srcs,
with_abis: true,
with_source_maps: false,
with_error_map: true,
named_addresses: Default::default(),
override_std: None,
install_dir: None,
with_docs: true,
docgen_options: Some(DocgenOptions {
include_impl: true,
Expand All @@ -120,14 +113,7 @@ impl ReleaseTarget {
output_format: None,
}),
skip_fetch_latest_git_deps: true,
bytecode_version: None,
compiler_version: None,
language_version: None,
skip_attribute_checks: false,
check_test_code: false,
known_attributes: extended_checks::get_all_attribute_names().clone(),
experiments: vec![],
move_2: false,
..BuildOptions::default()
},
packages: packages.iter().map(|(path, _)| path.to_owned()).collect(),
rust_bindings: packages
Expand Down
14 changes: 14 additions & 0 deletions aptos-move/framework/src/built_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use itertools::Itertools;
use move_binary_format::CompiledModule;
use move_command_line_common::files::MOVE_COMPILED_EXTENSION;
use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule};
use move_compiler_v2::{options::Options, Experiment};
use move_core_types::{language_storage::ModuleId, metadata::Metadata};
use move_model::{
metadata::{CompilerVersion, LanguageVersion},
Expand Down Expand Up @@ -256,6 +257,13 @@ impl BuiltPackage {

// Run extended checks as well derive runtime metadata
let model = &model_opt.expect("move model");

if let Some(model_options) = model.get_extension::<Options>() {
if model_options.experiment_on(Experiment::STOP_BEFORE_EXTENDED_CHECKS) {
std::process::exit(0)
}
}

let runtime_metadata = extended_checks::run_extended_checks(model);
if model.diag_count(Severity::Warning) > 0 {
let mut error_writer = StandardStream::stderr(ColorChoice::Auto);
Expand All @@ -265,6 +273,12 @@ impl BuiltPackage {
}
}

if let Some(model_options) = model.get_extension::<Options>() {
if model_options.experiment_on(Experiment::STOP_AFTER_EXTENDED_CHECKS) {
std::process::exit(0)
}
}

let compiled_pkg_path = package
.compiled_package_info
.build_flags
Expand Down
1 change: 0 additions & 1 deletion consensus/src/epoch_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1809,5 +1809,4 @@ pub enum NoRandomnessReason {
KeyPairDeserializationError(bcs::Error),
KeyPairSerializationError(bcs::Error),
KeyPairPersistError(anyhow::Error),
MyPkNotFoundInValidatorSet,
}
1 change: 1 addition & 0 deletions crates/aptos/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ move-bytecode-source-map = { workspace = true }
move-cli = { workspace = true }
move-command-line-common = { workspace = true }
move-compiler = { workspace = true }
move-compiler-v2 = { workspace = true }
move-core-types = { workspace = true }
move-coverage = { workspace = true }
move-disassembler = { workspace = true }
Expand Down
129 changes: 100 additions & 29 deletions crates/aptos/src/common/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
genesis::git::from_yaml,
move_tool::{ArgWithType, FunctionArgType, MemberId},
};
use anyhow::Context;
use anyhow::{bail, Context};
use aptos_api_types::ViewFunction;
use aptos_crypto::{
ed25519::{Ed25519PrivateKey, Ed25519PublicKey, Ed25519Signature},
Expand Down Expand Up @@ -1022,25 +1022,53 @@ impl RestOptions {
}
}

/// Options for optimization level
#[derive(Debug, Clone, Parser)]
pub enum OptimizationLevel {
/// No optimizations
None,
/// Default optimization level
Default,
/// Extra optimizations, that may take more time
Extra,
}

impl Default for OptimizationLevel {
fn default() -> Self {
Self::Default
}
}

impl FromStr for OptimizationLevel {
type Err = anyhow::Error;

/// Parses an optimization level, or default.
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s.to_lowercase().as_str() {
"none" => Ok(Self::None),
"" | "default" => Ok(Self::Default),
"extra" => Ok(Self::Extra),
_ => bail!(
"unrecognized optimization level `{}` (supported versions: `none`, `default`, `aggressive`)",
s
),
}
}
}

/// Options for compiling a move package dir
#[derive(Debug, Clone, Parser)]
pub struct MovePackageDir {
/// Enables dev mode, which uses all dev-addresses and dev-dependencies
///
/// Dev mode allows for changing dependencies and addresses to the preset [dev-addresses] and
/// [dev-dependencies] fields. This works both inside and out of tests for using preset values.
///
/// Currently, it also additionally pulls in all test compilation artifacts
#[clap(long)]
pub dev: bool,
/// Path to a move package (the folder with a Move.toml file)
/// Path to a move package (the folder with a Move.toml file). Defaults to current directory.
#[clap(long, value_parser)]
pub package_dir: Option<PathBuf>,

/// Path to save the compiled move package
///
/// Defaults to `<package_dir>/build`
#[clap(long, value_parser)]
pub output_dir: Option<PathBuf>,

/// Named addresses for the move binary
///
/// Example: alice=0x1234, bob=0x5678
Expand All @@ -1061,42 +1089,83 @@ pub struct MovePackageDir {
#[clap(long)]
pub(crate) skip_fetch_latest_git_deps: bool,

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

/// Enables dev mode, which uses all dev-addresses and dev-dependencies
///
/// Dev mode allows for changing dependencies and addresses to the preset [dev-addresses] and
/// [dev-dependencies] fields. This works both inside and out of tests for using preset values.
///
/// Currently, it also additionally pulls in all test compilation artifacts
#[clap(long)]
pub dev: bool,

/// Do apply extended checks for Aptos (e.g. `#[view]` attribute) also on test code.
/// NOTE: this behavior will become the default in the future.
/// See <https://github.com/aptos-labs/aptos-core/issues/10335>
#[clap(long, env = "APTOS_CHECK_TEST_CODE")]
pub check_test_code: bool,

/// Select optimization level. Choices are "none", "default", or "extra".
/// Level "extra" may spend more time on expensive optimizations in the future.
/// Level "none" does no optimizations, possibly leading to use of too many runtime resources.
/// Level "default" is the recommended level, and the default if not provided.
#[clap(long, alias = "optimization_level", value_parser = clap::value_parser!(OptimizationLevel))]
pub optimize: Option<OptimizationLevel>,

/// Experiments
#[clap(long, hide(true))]
pub experiments: Vec<String>,

/// ...or --bytecode `BYTECODE_VERSION`
/// Specify the version of the bytecode the compiler is going to emit.
#[clap(long, default_value_if("move_2", "true", "7"))]
/// Defaults to `6`, or `7` if language version 2 is selected
/// (through `--move-2` or `--language_version=2`), .
#[clap(
long,
default_value_if("move_2", "true", "7"),
alias = "bytecode",
verbatim_doc_comment
)]
pub bytecode_version: Option<u32>,

/// ...or `--compiler COMPILER_VERSION`
/// Specify the version of the compiler.
/// Currently, default to `v1`
/// Defaults to `1`, or `2` if `--move-2` is selected.
#[clap(long, value_parser = clap::value_parser!(CompilerVersion),
default_value_if("move_2", "true", "2.0"))]
alias = "compiler",
default_value_if("move_2", "true", "2.0"),
verbatim_doc_comment)]
pub compiler_version: Option<CompilerVersion>,

/// ...or `--language LANGUAGE_VERSION`
/// Specify the language version to be supported.
/// Currently, default to `v1`
/// Currently, defaults to `1`, unless `--move-2` is selected.
#[clap(long, value_parser = clap::value_parser!(LanguageVersion),
default_value_if("move_2", "true", "2.0"))]
alias = "language",
default_value_if("move_2", "true", "2.0"),
verbatim_doc_comment)]
pub language_version: Option<LanguageVersion>,

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

/// Do apply extended checks for Aptos (e.g. `#[view]` attribute) also on test code.
/// NOTE: this behavior will become the default in the future.
/// See <https://github.com/aptos-labs/aptos-core/issues/10335>
#[clap(long, env = "APTOS_CHECK_TEST_CODE")]
pub check_test_code: bool,

/// Select bytecode, language, compiler for Move 2
#[clap(long)]
/// Select bytecode, language version, and compiler to support Move 2:
/// Same as `--bytecode_version=7 --language_version=2.0 --compiler_version=2.0`
#[clap(long, verbatim_doc_comment)]
pub move_2: bool,
}

impl Default for MovePackageDir {
fn default() -> Self {
Self::new()
}
}

impl MovePackageDir {
pub fn new(package_dir: PathBuf) -> Self {
pub fn new() -> Self {
Self {
dev: false,
package_dir: Some(package_dir),
package_dir: None,
output_dir: None,
named_addresses: Default::default(),
override_std: None,
Expand All @@ -1107,6 +1176,8 @@ impl MovePackageDir {
skip_attribute_checks: false,
check_test_code: false,
move_2: false,
optimize: None,
experiments: vec![],
}
}

Expand Down
12 changes: 1 addition & 11 deletions crates/aptos/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1020,17 +1020,7 @@ impl CliCommand<()> for GenerateUpgradeProposal {
next_execution_hash,
} = self;
let package_path = move_options.get_package_path()?;
let options = included_artifacts.build_options(
move_options.dev,
move_options.skip_fetch_latest_git_deps,
move_options.named_addresses(),
move_options.override_std,
move_options.bytecode_version,
move_options.compiler_version,
move_options.language_version,
move_options.skip_attribute_checks,
move_options.check_test_code,
);
let options = included_artifacts.build_options(&move_options)?;
let package = BuiltPackage::build(package_path, options)?;
let release = ReleasePackage::new(package)?;

Expand Down
Loading

0 comments on commit de33d9c

Please sign in to comment.