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

[CLI] Add compiler-message-format-json experiment value to aptos move compile and aptos move lint #15540

Merged
merged 18 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
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.

6 changes: 6 additions & 0 deletions crates/aptos/src/move_tool/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ pub struct LintPackage {
/// See <https://github.com/aptos-labs/aptos-core/issues/10335>
#[clap(long, env = "APTOS_CHECK_TEST_CODE")]
pub check_test_code: bool,

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

impl LintPackage {
Expand All @@ -89,6 +93,7 @@ impl LintPackage {
language_version,
skip_attribute_checks,
check_test_code,
experiments,
} = self.clone();
MovePackageDir {
dev,
Expand All @@ -100,6 +105,7 @@ impl LintPackage {
language_version,
skip_attribute_checks,
check_test_code,
experiments,
..MovePackageDir::new()
}
}
Expand Down
2 changes: 2 additions & 0 deletions third_party/move/move-compiler-v2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ abstract-domain-derive = { path = "../move-model/bytecode/abstract_domain_derive
anyhow = { workspace = true }
bcs = { workspace = true }
clap = { workspace = true, features = ["derive", "env"] }
codespan = { workspace = true }
codespan-reporting = { workspace = true, features = ["serde", "serialization"] }
ethnum = { workspace = true }
flexi_logger = { workspace = true }
Expand All @@ -35,6 +36,7 @@ move-symbol-pool = { workspace = true }
num = { workspace = true }
once_cell = { workspace = true }
petgraph = { workspace = true }
serde_json = { workspace = true }

[dev-dependencies]
anyhow = { workspace = true }
Expand Down
28 changes: 28 additions & 0 deletions third_party/move/move-compiler-v2/src/diagnostics/human.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
use crate::diagnostics::Emitter;
use codespan::{FileId, Files};
use codespan_reporting::{
diagnostic::Diagnostic,
term::{emit, termcolor::WriteColor, Config},
};

pub struct HumanEmitter<'w, W: WriteColor> {
mkurnikov marked this conversation as resolved.
Show resolved Hide resolved
writer: &'w mut W,
}

impl<'w, W> HumanEmitter<'w, W>
where
W: WriteColor,
{
pub fn new(writer: &'w mut W) -> Self {
HumanEmitter { writer }
}
}

impl<'w, W> Emitter for HumanEmitter<'w, W>
where
W: WriteColor,
{
fn emit(&mut self, source_files: &Files<String>, diag: &Diagnostic<FileId>) {
emit(&mut self.writer, &Config::default(), source_files, diag).expect("emit must not fail")
}
}
38 changes: 38 additions & 0 deletions third_party/move/move-compiler-v2/src/diagnostics/json.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use crate::diagnostics::Emitter;
use codespan::{FileId, Files};
use codespan_reporting::diagnostic::{Diagnostic, Label};
use std::io::Write;

pub struct JsonEmitter<'w, W: Write> {
mkurnikov marked this conversation as resolved.
Show resolved Hide resolved
writer: &'w mut W,
}

impl<'w, W: Write> JsonEmitter<'w, W> {
pub fn new(writer: &'w mut W) -> Self {
JsonEmitter { writer }
}
}

impl<'w, W: Write> Emitter for JsonEmitter<'w, W> {
fn emit(&mut self, source_files: &Files<String>, diag: &Diagnostic<FileId>) {
let fpath_labels = diag
.labels
.iter()
.map(|label| {
let fpath = codespan_reporting::files::Files::name(source_files, label.file_id)
.expect("emit must not fail")
.to_string();
Label::new(label.style, fpath, label.range.clone())
})
.collect();
let mut json_diag = Diagnostic::new(diag.severity)
.with_message(diag.message.clone())
.with_labels(fpath_labels)
.with_notes(diag.notes.clone());
if let Some(code) = &diag.code {
json_diag = json_diag.with_code(code)
}
serde_json::to_writer(&mut self.writer, &json_diag).expect("emit must not fail");
writeln!(&mut self.writer).unwrap();
mkurnikov marked this conversation as resolved.
Show resolved Hide resolved
}
}
52 changes: 52 additions & 0 deletions third_party/move/move-compiler-v2/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use crate::{
diagnostics::{human::HumanEmitter, json::JsonEmitter},
options, Experiment,
};
use anyhow::bail;
use codespan::{FileId, Files};
use codespan_reporting::{
diagnostic::{Diagnostic, Severity},
term::termcolor::StandardStream,
};
use move_model::model::GlobalEnv;

pub mod human;
pub mod json;

impl options::Options {
pub fn to_emitter<'w>(&self, stderr: &'w mut StandardStream) -> Box<dyn Emitter + 'w> {
if self.experiment_on(Experiment::MESSAGE_FORMAT_JSON) {
Box::new(JsonEmitter::new(stderr))
} else {
Box::new(HumanEmitter::new(stderr))
}
}
}

pub trait Emitter {
fn emit(&mut self, source_files: &Files<String>, diag: &Diagnostic<FileId>);

/// Writes accumulated diagnostics of given or higher severity.
fn report_diag(&mut self, global_env: &GlobalEnv, severity: Severity) {
global_env.report_diag_with_filter(
|files, diag| self.emit(files, diag),
|d| d.severity >= severity,
);
}

/// Helper function to report diagnostics, check for errors, and fail with a message on
/// errors. This function is idempotent and will not report the same diagnostics again.
fn check_diag(
&mut self,
global_env: &GlobalEnv,
report_severity: Severity,
msg: &str,
) -> anyhow::Result<()> {
self.report_diag(global_env, report_severity);
if global_env.has_errors() {
bail!("exiting with {}", msg);
} else {
Ok(())
}
}
}
6 changes: 6 additions & 0 deletions third_party/move/move-compiler-v2/src/experiments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ pub static EXPERIMENTS: Lazy<BTreeMap<String, Experiment>> = Lazy::new(|| {
.to_string(),
default: Given(false),
},
Experiment {
mkurnikov marked this conversation as resolved.
Show resolved Hide resolved
name: Experiment::MESSAGE_FORMAT_JSON.to_string(),
description: "Enable json format for compiler messages".to_string(),
default: Given(false),
},
];
experiments
.into_iter()
Expand Down Expand Up @@ -302,6 +307,7 @@ impl Experiment {
pub const LAMBDA_LIFTING: &'static str = "lambda-lifting";
pub const LAMBDA_VALUES: &'static str = "lambda-values";
pub const LINT_CHECKS: &'static str = "lint-checks";
pub const MESSAGE_FORMAT_JSON: &'static str = "compiler-message-format-json";
pub const OPTIMIZE: &'static str = "optimize";
pub const OPTIMIZE_EXTRA: &'static str = "optimize-extra";
pub const OPTIMIZE_WAITING_FOR_COMPARE_TESTS: &'static str =
Expand Down
38 changes: 22 additions & 16 deletions third_party/move/move-compiler-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0

mod bytecode_generator;
pub mod diagnostics;
pub mod env_pipeline;
mod experiments;
pub mod external_checks;
Expand All @@ -14,6 +15,7 @@ pub mod pipeline;
pub mod plan_builder;

use crate::{
diagnostics::{human::HumanEmitter, Emitter},
env_pipeline::{
acquires_checker, ast_simplifier, cyclic_instantiation_checker, flow_insensitive_checkers,
function_checker, inliner, lambda_lifter, lambda_lifter::LambdaLiftingOptions,
Expand Down Expand Up @@ -71,38 +73,40 @@ use move_stackless_bytecode::function_target_pipeline::{
};
use move_symbol_pool::Symbol;
pub use options::Options;
use std::{collections::BTreeSet, io::Write, path::Path};
use std::{collections::BTreeSet, path::Path};

/// Run Move compiler and print errors to stderr.
pub fn run_move_compiler_to_stderr(
options: Options,
) -> anyhow::Result<(GlobalEnv, Vec<AnnotatedCompiledUnit>)> {
let mut error_writer = StandardStream::stderr(ColorChoice::Auto);
run_move_compiler(&mut error_writer, options)
let mut stderr = StandardStream::stderr(ColorChoice::Auto);
let mut emitter = HumanEmitter::new(&mut stderr);
mkurnikov marked this conversation as resolved.
Show resolved Hide resolved
run_move_compiler(&mut emitter, options)
}

/// Run move compiler and print errors to given writer. Returns the set of compiled units.
pub fn run_move_compiler<W>(
error_writer: &mut W,
pub fn run_move_compiler<E>(
emitter: &mut E,
options: Options,
) -> anyhow::Result<(GlobalEnv, Vec<AnnotatedCompiledUnit>)>
where
W: WriteColor + Write,
E: Emitter + ?Sized,
{
logging::setup_logging();
info!("Move Compiler v2");

// Run context check.
let mut env = run_checker_and_rewriters(options.clone())?;
check_errors(&env, error_writer, "checking errors")?;
check_errors(&env, emitter, "checking errors")?;

if options.experiment_on(Experiment::STOP_BEFORE_STACKLESS_BYTECODE) {
std::process::exit(0)
}

// Run code generator
let mut targets = run_bytecode_gen(&env);
check_errors(&env, error_writer, "code generation errors")?;
check_errors(&env, emitter, "code generation errors")?;

debug!("After bytecode_gen, GlobalEnv={}", env.dump_env());

// Run transformation pipeline
Expand All @@ -129,14 +133,14 @@ where
} else {
pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors())
}
check_errors(&env, error_writer, "stackless-bytecode analysis errors")?;
check_errors(&env, emitter, "stackless-bytecode analysis errors")?;

if options.experiment_on(Experiment::STOP_BEFORE_FILE_FORMAT) {
std::process::exit(0)
}

let modules_and_scripts = run_file_format_gen(&mut env, &targets);
check_errors(&env, error_writer, "assembling errors")?;
check_errors(&env, emitter, "assembling errors")?;

debug!(
"File format bytecode:\n{}",
Expand All @@ -145,7 +149,7 @@ where

let annotated_units = annotate_units(modules_and_scripts);
run_bytecode_verifier(&annotated_units, &mut env);
check_errors(&env, error_writer, "bytecode verification errors")?;
check_errors(&env, emitter, "bytecode verification errors")?;

// Finally mark this model to be generated by v2
env.set_compiler_v2(true);
Expand All @@ -163,7 +167,8 @@ pub fn run_move_compiler_for_analysis(
options.whole_program = true; // will set `treat_everything_as_target`
options = options.set_experiment(Experiment::SPEC_REWRITE, true);
options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true);
let (env, _units) = run_move_compiler(error_writer, options)?;
let mut emitter = HumanEmitter::new(error_writer);
let (env, _units) = run_move_compiler(&mut emitter, options)?;
// Reset for subsequent analysis
env.treat_everything_as_target(false);
Ok(env)
Expand Down Expand Up @@ -605,13 +610,14 @@ fn get_vm_error_loc(env: &GlobalEnv, source_map: &SourceMap, e: &VMError) -> Opt
}

/// Report any diags in the env to the writer and fail if there are errors.
pub fn check_errors<W>(env: &GlobalEnv, error_writer: &mut W, msg: &str) -> anyhow::Result<()>
pub fn check_errors<E>(env: &GlobalEnv, emitter: &mut E, msg: &str) -> anyhow::Result<()>
where
W: WriteColor + Write,
E: Emitter + ?Sized,
{
let options = env.get_extension::<Options>().unwrap_or_default();
env.report_diag(error_writer, options.report_severity());
env.check_diag(error_writer, options.report_severity(), msg)

emitter.report_diag(env, options.report_severity());
emitter.check_diag(env, options.report_severity(), msg)
}

/// Annotate the given compiled units.
Expand Down
20 changes: 12 additions & 8 deletions third_party/move/move-model/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,12 @@ impl GlobalEnv {

/// Writes accumulated diagnostics of given or higher severity.
pub fn report_diag<W: WriteColor>(&self, writer: &mut W, severity: Severity) {
self.report_diag_with_filter(writer, |d| d.severity >= severity)
self.report_diag_with_filter(
|files, diag| {
emit(writer, &Config::default(), files, diag).expect("emit must not fail")
},
|d| d.severity >= severity,
);
}

/// Helper function to report diagnostics, check for errors, and fail with a message on
Expand Down Expand Up @@ -1312,11 +1317,11 @@ impl GlobalEnv {
}

/// Writes accumulated diagnostics that pass through `filter`
pub fn report_diag_with_filter<W: WriteColor, F: FnMut(&Diagnostic<FileId>) -> bool>(
&self,
writer: &mut W,
mut filter: F,
) {
pub fn report_diag_with_filter<E, F>(&self, mut emitter: E, mut filter: F)
where
E: FnMut(&Files<String>, &Diagnostic<FileId>),
F: FnMut(&Diagnostic<FileId>) -> bool,
{
let mut shown = BTreeSet::new();
self.diags.borrow_mut().sort_by(|a, b| {
let reported_ordering = a.1.cmp(&b.1);
Expand All @@ -1338,8 +1343,7 @@ impl GlobalEnv {
// Avoid showing the same message twice. This can happen e.g. because of
// duplication of expressions via schema inclusion.
if shown.insert(format!("{:?}", diag)) {
emit(writer, &Config::default(), &self.source_files, diag)
.expect("emit must not fail");
emitter(&self.source_files, diag);
}
*reported = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use move_compiler::{
},
FullyCompiledProgram,
};
use move_compiler_v2::diagnostics::human::HumanEmitter;
use move_core_types::{
account_address::AccountAddress,
identifier::{IdentStr, Identifier},
Expand Down Expand Up @@ -824,7 +825,8 @@ fn compile_source_unit_v2(
options = options.set_experiment(exp, value)
}
let mut error_writer = termcolor::Buffer::no_color();
let result = move_compiler_v2::run_move_compiler(&mut error_writer, options);
let mut emitter = HumanEmitter::new(&mut error_writer);
mkurnikov marked this conversation as resolved.
Show resolved Hide resolved
let result = move_compiler_v2::run_move_compiler(&mut emitter, options);
let error_str = String::from_utf8_lossy(&error_writer.into_inner()).to_string();
let (model, mut units) =
result.map_err(|_| anyhow::anyhow!("compilation errors:\n {}", error_str))?;
Expand Down
Loading
Loading