From 415de9c1361a1272d61f484121174b67de46087a Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Sun, 8 Dec 2024 20:45:31 +0100 Subject: [PATCH 01/15] add --message-format to aptos move compile --- Cargo.lock | 3 + aptos-move/framework/src/built_package.rs | 10 ++- crates/aptos/src/move_tool/coverage.rs | 2 + crates/aptos/src/move_tool/lint.rs | 7 +- crates/aptos/src/move_tool/mod.rs | 7 +- crates/aptos/src/test/mod.rs | 2 + third_party/move/move-compiler-v2/Cargo.toml | 3 + .../move-compiler-v2/src/diagnostics/human.rs | 28 ++++++ .../move-compiler-v2/src/diagnostics/json.rs | 21 +++++ .../src/diagnostics/message_format.rs | 26 ++++++ .../move-compiler-v2/src/diagnostics/mod.rs | 46 ++++++++++ third_party/move/move-compiler-v2/src/lib.rs | 88 +++++++++++++++++++ .../move/move-compiler-v2/src/options.rs | 4 + third_party/move/move-model/src/model.rs | 58 ++++++++++-- third_party/move/move-prover/src/lib.rs | 6 +- .../move-cli/src/base/test_validation.rs | 21 ++++- .../src/compilation/compiled_package.rs | 10 ++- .../move/tools/move-package/src/lib.rs | 7 +- 18 files changed, 331 insertions(+), 18 deletions(-) create mode 100644 third_party/move/move-compiler-v2/src/diagnostics/human.rs create mode 100644 third_party/move/move-compiler-v2/src/diagnostics/json.rs create mode 100644 third_party/move/move-compiler-v2/src/diagnostics/message_format.rs create mode 100644 third_party/move/move-compiler-v2/src/diagnostics/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 80a48ae3ef7c5..82dc4e1d36ae1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11192,6 +11192,7 @@ dependencies = [ "anyhow", "bcs 0.1.4", "clap 4.5.21", + "codespan", "codespan-reporting", "datatest-stable", "ethnum", @@ -11216,6 +11217,8 @@ dependencies = [ "num 0.4.1", "once_cell", "petgraph 0.6.5", + "serde", + "serde_json", "walkdir", ] diff --git a/aptos-move/framework/src/built_package.rs b/aptos-move/framework/src/built_package.rs index 1a7d5edfb3d9f..00652ea438c9f 100644 --- a/aptos-move/framework/src/built_package.rs +++ b/aptos-move/framework/src/built_package.rs @@ -19,7 +19,10 @@ use itertools::Itertools; use move_binary_format::{file_format_common::VERSION_7, CompiledModule}; use move_command_line_common::files::MOVE_COMPILED_EXTENSION; use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule}; -use move_compiler_v2::{external_checks::ExternalChecks, options::Options, Experiment}; +use move_compiler_v2::{ + diagnostics::message_format::MessageFormat, external_checks::ExternalChecks, options::Options, + Experiment, +}; use move_core_types::{language_storage::ModuleId, metadata::Metadata}; use move_model::{ metadata::{ @@ -107,6 +110,8 @@ pub struct BuildOptions { /// Select bytecode, language, compiler for Move 2 #[clap(long)] pub move_2: bool, + #[clap(long, default_value = "human")] + pub message_format: MessageFormat, } // Because named_addresses has no parser, we can't use clap's default impl. This must be aligned @@ -135,6 +140,7 @@ impl Default for BuildOptions { known_attributes: extended_checks::get_all_attribute_names().clone(), experiments: vec![], move_2: false, + message_format: MessageFormat::default(), } } } @@ -207,6 +213,7 @@ pub fn build_model( skip_attribute_checks, known_attributes, experiments, + message_format: MessageFormat::default(), }, }; let compiler_version = compiler_version.unwrap_or_default(); @@ -263,6 +270,7 @@ impl BuiltPackage { skip_attribute_checks, known_attributes: options.known_attributes.clone(), experiments: options.experiments.clone(), + message_format: options.message_format.clone(), }, }; diff --git a/crates/aptos/src/move_tool/coverage.rs b/crates/aptos/src/move_tool/coverage.rs index 366b6bb9472f9..6fb958bf9a91b 100644 --- a/crates/aptos/src/move_tool/coverage.rs +++ b/crates/aptos/src/move_tool/coverage.rs @@ -9,6 +9,7 @@ use aptos_framework::extended_checks; use async_trait::async_trait; use clap::{Parser, Subcommand}; use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule}; +use move_compiler_v2::diagnostics::message_format::MessageFormat; use move_coverage::{ coverage_map::CoverageMap, format_csv_summary, format_human_summary, @@ -178,6 +179,7 @@ fn compile_coverage( compiler_version: move_options.compiler_version, language_version: move_options.language_version, experiments: experiments_from_opt_level(&move_options.optimize), + message_format: MessageFormat::default(), }, ..Default::default() }; diff --git a/crates/aptos/src/move_tool/lint.rs b/crates/aptos/src/move_tool/lint.rs index 0b320098d65bf..9851e5e65d59a 100644 --- a/crates/aptos/src/move_tool/lint.rs +++ b/crates/aptos/src/move_tool/lint.rs @@ -8,7 +8,7 @@ use crate::{ use aptos_framework::{BuildOptions, BuiltPackage}; use async_trait::async_trait; use clap::Parser; -use move_compiler_v2::Experiment; +use move_compiler_v2::{diagnostics::message_format::MessageFormat, Experiment}; use move_linter::MoveLintChecks; use move_model::metadata::{CompilerVersion, LanguageVersion, LATEST_STABLE_LANGUAGE_VERSION}; use move_package::source_package::std_lib::StdVersion; @@ -75,6 +75,9 @@ pub struct LintPackage { /// See #[clap(long, env = "APTOS_CHECK_TEST_CODE")] pub check_test_code: bool, + + #[clap(long, default_value = "human")] + pub message_format: MessageFormat, } impl LintPackage { @@ -89,6 +92,7 @@ impl LintPackage { language_version, skip_attribute_checks, check_test_code, + message_format: _, } = self.clone(); MovePackageDir { dev, @@ -126,6 +130,7 @@ impl CliCommand<&'static str> for LintPackage { let package_path = move_options.get_package_path()?; let included_artifacts = IncludedArtifacts::Sparse; let build_options = BuildOptions { + message_format: self.message_format, ..included_artifacts.build_options_with_experiments( &move_options, more_experiments, diff --git a/crates/aptos/src/move_tool/mod.rs b/crates/aptos/src/move_tool/mod.rs index b2959e013c0b8..298a795251fb0 100644 --- a/crates/aptos/src/move_tool/mod.rs +++ b/crates/aptos/src/move_tool/mod.rs @@ -60,7 +60,7 @@ use colored::Colorize; use itertools::Itertools; use move_cli::{self, base::test::UnitTestResult}; use move_command_line_common::{address::NumericalAddress, env::MOVE_HOME}; -use move_compiler_v2::Experiment; +use move_compiler_v2::{diagnostics::message_format::MessageFormat, Experiment}; use move_core_types::{identifier::Identifier, language_storage::ModuleId, u256::U256}; use move_model::metadata::{CompilerVersion, LanguageVersion}; use move_package::{source_package::layout::SourcePackageLayout, BuildConfig, CompilerConfig}; @@ -390,6 +390,9 @@ pub struct CompilePackage { #[clap(long)] pub save_metadata: bool, + #[clap(long, default_value = "human")] + pub message_format: MessageFormat, + #[clap(flatten)] pub included_artifacts_args: IncludedArtifactsArgs, #[clap(flatten)] @@ -405,6 +408,7 @@ impl CliCommand> for CompilePackage { async fn execute(self) -> CliTypedResult> { let build_options = BuildOptions { install_dir: self.move_options.output_dir.clone(), + message_format: self.message_format.clone(), ..self .included_artifacts_args .included_artifacts @@ -567,6 +571,7 @@ impl CliCommand<&'static str> for TestPackage { compiler_version: self.move_options.compiler_version, language_version: self.move_options.language_version, experiments: experiments_from_opt_level(&self.move_options.optimize), + message_format: MessageFormat::default(), }, ..Default::default() }; diff --git a/crates/aptos/src/test/mod.rs b/crates/aptos/src/test/mod.rs index eee3d6e19a5d9..4a2eaa945384e 100644 --- a/crates/aptos/src/test/mod.rs +++ b/crates/aptos/src/test/mod.rs @@ -64,6 +64,7 @@ use aptos_rest_client::{ use aptos_sdk::move_types::{account_address::AccountAddress, language_storage::ModuleId}; use aptos_temppath::TempPath; use aptos_types::on_chain_config::ValidatorSet; +use move_compiler_v2::diagnostics::message_format::MessageFormat; use move_core_types::ident_str; use reqwest::Url; use serde::{Deserialize, Serialize}; @@ -856,6 +857,7 @@ impl CliTestFramework { included_artifacts_args: IncludedArtifactsArgs { included_artifacts: included_artifacts.unwrap_or(IncludedArtifacts::Sparse), }, + message_format: MessageFormat::default(), } .execute() .await diff --git a/third_party/move/move-compiler-v2/Cargo.toml b/third_party/move/move-compiler-v2/Cargo.toml index f24853b9f9b2b..39e464e534e39 100644 --- a/third_party/move/move-compiler-v2/Cargo.toml +++ b/third_party/move/move-compiler-v2/Cargo.toml @@ -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 } @@ -35,6 +36,8 @@ move-symbol-pool = { workspace = true } num = { workspace = true } once_cell = { workspace = true } petgraph = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } [dev-dependencies] anyhow = { workspace = true } diff --git a/third_party/move/move-compiler-v2/src/diagnostics/human.rs b/third_party/move/move-compiler-v2/src/diagnostics/human.rs new file mode 100644 index 0000000000000..47e44af1b1cd1 --- /dev/null +++ b/third_party/move/move-compiler-v2/src/diagnostics/human.rs @@ -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 { + writer: W, +} + +impl HumanEmitter +where + W: WriteColor, +{ + pub fn new(writer: W) -> Self { + HumanEmitter { writer } + } +} + +impl Emitter for HumanEmitter +where + W: WriteColor, +{ + fn emit(&mut self, source_files: &Files, diag: &Diagnostic) { + emit(&mut self.writer, &Config::default(), source_files, diag).expect("emit must not fail") + } +} diff --git a/third_party/move/move-compiler-v2/src/diagnostics/json.rs b/third_party/move/move-compiler-v2/src/diagnostics/json.rs new file mode 100644 index 0000000000000..d25071bdc8f12 --- /dev/null +++ b/third_party/move/move-compiler-v2/src/diagnostics/json.rs @@ -0,0 +1,21 @@ +use crate::diagnostics::Emitter; +use codespan::{FileId, Files}; +use codespan_reporting::diagnostic::Diagnostic; +use std::io; + +pub struct JsonEmitter { + writer: W, +} + +impl JsonEmitter { + pub fn new(writer: W) -> Self { + JsonEmitter { writer } + } +} + +impl Emitter for JsonEmitter { + fn emit(&mut self, _source_files: &Files, diag: &Diagnostic) { + serde_json::to_writer(&mut self.writer, diag).expect("emit must not fail"); + writeln!(&mut self.writer).unwrap(); + } +} diff --git a/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs b/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs new file mode 100644 index 0000000000000..4e69defaca7d8 --- /dev/null +++ b/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs @@ -0,0 +1,26 @@ +use crate::diagnostics::{human::HumanEmitter, json::JsonEmitter, Emitter}; +use clap::ValueEnum; +use codespan_reporting::term::termcolor::{ColorChoice, StandardStream}; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Default, Serialize, Deserialize, PartialOrd, Eq, PartialEq, Clone, ValueEnum)] +pub enum MessageFormat { + #[default] + Human, + Json, +} + +impl MessageFormat { + pub fn into_emitter(self) -> Box { + match self { + MessageFormat::Human => { + let stderr = StandardStream::stderr(ColorChoice::Auto); + Box::new(HumanEmitter::new(stderr)) + }, + MessageFormat::Json => { + let stderr = StandardStream::stderr(ColorChoice::Auto); + Box::new(JsonEmitter::new(stderr)) + }, + } + } +} diff --git a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs new file mode 100644 index 0000000000000..ba34e841a524c --- /dev/null +++ b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs @@ -0,0 +1,46 @@ +use anyhow::bail; +use codespan::{FileId, Files}; +use codespan_reporting::diagnostic::{Diagnostic, Severity}; +use move_model::model::GlobalEnv; + +pub mod human; +pub mod json; +pub mod message_format; + +pub struct DiagnosticReporter { + emitter: Box, +} + +impl DiagnosticReporter { + pub fn new(emitter: Box) -> Self { + DiagnosticReporter { emitter } + } + + /// Writes accumulated diagnostics of given or higher severity. + pub fn report_diag(&mut self, global_env: &GlobalEnv, severity: Severity) { + global_env.report_diag_with_filter( + |files, diag| self.emitter.as_mut().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. + pub 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(()) + } + } +} + +pub trait Emitter { + fn emit(&mut self, source_files: &Files, diag: &Diagnostic); +} diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index 84eda127b9f03..cd04430e37f99 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -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; @@ -14,6 +15,7 @@ pub mod pipeline; pub mod plan_builder; use crate::{ + diagnostics::DiagnosticReporter, env_pipeline::{ acquires_checker, ast_simplifier, cyclic_instantiation_checker, flow_insensitive_checkers, function_checker, inliner, lambda_lifter, lambda_lifter::LambdaLiftingOptions, @@ -153,6 +155,79 @@ where Ok((env, annotated_units)) } +/// Run move compiler and print errors to given writer. Returns the set of compiled units. +pub fn run_move_compiler_with_reporter( + reporter: &mut DiagnosticReporter, + options: Options, +) -> anyhow::Result<(GlobalEnv, Vec)> { + logging::setup_logging(); + info!("Move Compiler v2"); + + // Run context check. + let mut env = run_checker_and_rewriters(options.clone())?; + check_errors_with_reporter(&env, reporter, "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_with_reporter(&env, reporter, "code generation errors")?; + // check_errors(&env, error_writer, "code generation errors")?; + debug!("After bytecode_gen, GlobalEnv={}", env.dump_env()); + + // Run transformation pipeline + let pipeline = bytecode_pipeline(&env); + if log_enabled!(Level::Debug) { + // Dump bytecode, providing a name for the target derived from the first input file. + let dump_base_name = options + .sources + .first() + .and_then(|f| { + Path::new(f) + .file_name() + .map(|f| f.to_string_lossy().as_ref().to_owned()) + }) + .unwrap_or_else(|| "dump".to_owned()); + pipeline.run_with_dump( + &env, + &mut targets, + &dump_base_name, + false, + &pipeline::register_formatters, + || !env.has_errors(), + ) + } else { + pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors()) + } + check_errors_with_reporter(&env, reporter, "stackless-bytecode analysis errors")?; + // check_errors(&env, error_writer, "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_with_reporter(&env, reporter, "assembling errors")?; + // check_errors(&env, error_writer, "assembling errors")?; + + debug!( + "File format bytecode:\n{}", + disassemble_compiled_units(&modules_and_scripts)? + ); + + let annotated_units = annotate_units(modules_and_scripts); + run_bytecode_verifier(&annotated_units, &mut env); + check_errors_with_reporter(&env, reporter, "bytecode verification errors")?; + // check_errors(&env, error_writer, "bytecode verification errors")?; + + // Finally mark this model to be generated by v2 + env.set_compiler_v2(true); + + Ok((env, annotated_units)) +} + /// Run move compiler and print errors to given writer for the purpose of analysis, like /// e.g. the Move prover. After successful compilation attaches the generated bytecode /// to the model. @@ -614,6 +689,19 @@ where env.check_diag(error_writer, options.report_severity(), msg) } +/// Report any diags in the env to the writer and fail if there are errors. +pub fn check_errors_with_reporter( + env: &GlobalEnv, + reporter: &mut DiagnosticReporter, + msg: &str, +) -> anyhow::Result<()> { + let options = env.get_extension::().unwrap_or_default(); + reporter.report_diag(env, options.report_severity()); + // env.report_diag(error_writer, options.report_severity()); + reporter.check_diag(env, options.report_severity(), msg) + // env.check_diag(error_writer, options.report_severity(), msg) +} + /// Annotate the given compiled units. pub fn annotate_units(units: Vec) -> Vec { units diff --git a/third_party/move/move-compiler-v2/src/options.rs b/third_party/move/move-compiler-v2/src/options.rs index 0df18f732cd59..88fdd6d1f164d 100644 --- a/third_party/move/move-compiler-v2/src/options.rs +++ b/third_party/move/move-compiler-v2/src/options.rs @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ + diagnostics::message_format::MessageFormat, experiments::{DefaultValue, EXPERIMENTS}, external_checks::ExternalChecks, }; @@ -129,6 +130,9 @@ pub struct Options { /// External checks to be performed. #[clap(skip)] pub external_checks: Vec>, + + #[clap(long, default_value = "human")] + pub message_format: MessageFormat, } impl Default for Options { diff --git a/third_party/move/move-model/src/model.rs b/third_party/move/move-model/src/model.rs index e7a87dd266c74..3fe632bc65007 100644 --- a/third_party/move/move-model/src/model.rs +++ b/third_party/move/move-model/src/model.rs @@ -1202,7 +1202,13 @@ impl GlobalEnv { /// Writes accumulated diagnostics of given or higher severity. pub fn report_diag(&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, + ); + // self.report_diag_with_filter(writer, |d| d.severity >= severity) } /// Helper function to report diagnostics, check for errors, and fail with a message on @@ -1301,11 +1307,11 @@ impl GlobalEnv { } /// Writes accumulated diagnostics that pass through `filter` - pub fn report_diag_with_filter) -> bool>( - &self, - writer: &mut W, - mut filter: F, - ) { + pub fn report_diag_with_filter(&self, mut emitter: E, mut filter: F) + where + E: FnMut(&Files, &Diagnostic), + F: FnMut(&Diagnostic) -> bool, + { let mut shown = BTreeSet::new(); self.diags.borrow_mut().sort_by(|a, b| { let reported_ordering = a.1.cmp(&b.1); @@ -1327,14 +1333,50 @@ 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); + // emit(writer, &Config::default(), &self.source_files, diag) + // .expect("emit must not fail"); } *reported = true; } } } + // /// Writes accumulated diagnostics that pass through `filter` + // pub fn report_diag_with_filter) -> bool>( + // &self, + // writer: &mut W, + // mut filter: F, + // ) { + // let mut shown = BTreeSet::new(); + // self.diags.borrow_mut().sort_by(|a, b| { + // let reported_ordering = a.1.cmp(&b.1); + // if Ordering::Equal == reported_ordering { + // GlobalEnv::cmp_diagnostic(&a.0, &b.0) + // } else { + // reported_ordering + // } + // }); + // for (diag, reported) in self.diags.borrow_mut().iter_mut().filter(|(d, reported)| { + // !reported + // && filter(d) + // && (d.severity >= Severity::Error + // || d.labels + // .iter() + // .any(|label| self.file_id_is_primary_target.contains(&label.file_id))) + // }) { + // if !*reported { + // // 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"); + // } + // *reported = true; + // } + // } + // } + /// Adds a global invariant to this environment. pub fn add_global_invariant(&mut self, inv: GlobalInvariant) { let id = inv.id; diff --git a/third_party/move/move-prover/src/lib.rs b/third_party/move/move-prover/src/lib.rs index 46810fe825d9d..7a09645c602ce 100644 --- a/third_party/move/move-prover/src/lib.rs +++ b/third_party/move/move-prover/src/lib.rs @@ -11,7 +11,10 @@ use codespan_reporting::term::termcolor::{ColorChoice, StandardStream, WriteColo use log::{debug, info, warn}; use move_abigen::Abigen; use move_compiler::shared::{known_attributes::KnownAttribute, PackagePaths}; -use move_compiler_v2::{env_pipeline::rewrite_target::RewritingScope, Experiment}; +use move_compiler_v2::{ + diagnostics::message_format::MessageFormat, env_pipeline::rewrite_target::RewritingScope, + Experiment, +}; use move_docgen::Docgen; use move_errmapgen::ErrmapGen; use move_model::{ @@ -92,6 +95,7 @@ pub fn run_move_prover_v2( compile_test_code: false, compile_verify_code: true, external_checks: vec![], + message_format: MessageFormat::default(), }; let mut env = move_compiler_v2::run_move_compiler_for_analysis(error_writer, compiler_options)?; diff --git a/third_party/move/tools/move-cli/src/base/test_validation.rs b/third_party/move/tools/move-cli/src/base/test_validation.rs index e721eba9fcad4..9b632500e2baa 100644 --- a/third_party/move/tools/move-cli/src/base/test_validation.rs +++ b/third_party/move/tools/move-cli/src/base/test_validation.rs @@ -5,7 +5,7 @@ //! This module manages validation of the unit tests, in addition to standard compiler //! checking. -use codespan_reporting::term::{termcolor, termcolor::StandardStream}; +use codespan_reporting::term::{emit, termcolor, termcolor::StandardStream, Config}; use move_model::model::GlobalEnv; use once_cell::sync::Lazy; use std::sync::Mutex; @@ -35,8 +35,11 @@ pub(crate) fn validate(env: &GlobalEnv) { pub fn has_errors_then_report(model: &GlobalEnv) -> bool { let mut has_errors = false; + let mut writer = StandardStream::stderr(termcolor::ColorChoice::Auto); model.report_diag_with_filter( - &mut StandardStream::stderr(termcolor::ColorChoice::Auto), + |files, diag| { + emit(&mut writer, &Config::default(), files, diag).expect("emit must not fail") + }, |d| { let include = d.labels.iter().all(|l| { let fname = model.get_file(l.file_id).to_string_lossy(); @@ -49,5 +52,19 @@ pub fn has_errors_then_report(model: &GlobalEnv) -> bool { include }, ); + // model.report_diag_with_filter( + // &mut StandardStream::stderr(termcolor::ColorChoice::Auto), + // |d| { + // let include = d.labels.iter().all(|l| { + // let fname = model.get_file(l.file_id).to_string_lossy(); + // !fname.contains("aptos-framework/sources") + // && !fname.contains("aptos-stdlib/sources") + // }); + // if include && d.severity == codespan_reporting::diagnostic::Severity::Error { + // has_errors = true; + // } + // include + // }, + // ); has_errors } diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index 31fb59bd73bef..6d25dcce9601a 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -27,7 +27,9 @@ use move_compiler::{ shared::{Flags, NamedAddressMap, NumericalAddress, PackagePaths}, Compiler, }; -use move_compiler_v2::{external_checks::ExternalChecks, Experiment}; +use move_compiler_v2::{ + diagnostics::DiagnosticReporter, external_checks::ExternalChecks, Experiment, +}; use move_docgen::{Docgen, DocgenOptions}; use move_model::{ model::GlobalEnv, options::ModelBuilderOptions, @@ -699,6 +701,7 @@ impl CompiledPackage { compile_test_code: flags.keep_testing_functions(), experiments: config.experiments.clone(), external_checks, + message_format: config.message_format.clone(), ..Default::default() }; options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true); @@ -1155,8 +1158,9 @@ pub fn build_and_report_v2_driver(options: move_compiler_v2::Options) -> Compile pub fn build_and_report_no_exit_v2_driver( options: move_compiler_v2::Options, ) -> CompilerDriverResult { - let mut writer = StandardStream::stderr(ColorChoice::Auto); - let (env, units) = move_compiler_v2::run_move_compiler(&mut writer, options)?; + let emitter = options.message_format.clone().into_emitter(); + let mut reporter = DiagnosticReporter::new(emitter); + let (env, units) = move_compiler_v2::run_move_compiler_with_reporter(&mut reporter, options)?; Ok(( move_compiler_v2::make_files_source_text(&env), units, diff --git a/third_party/move/tools/move-package/src/lib.rs b/third_party/move/tools/move-package/src/lib.rs index 75f7d112f1eae..467600a70c3cd 100644 --- a/third_party/move/tools/move-package/src/lib.rs +++ b/third_party/move/tools/move-package/src/lib.rs @@ -22,7 +22,9 @@ use clap::*; use move_compiler::{ command_line::SKIP_ATTRIBUTE_CHECKS, shared::known_attributes::KnownAttribute, }; -use move_compiler_v2::external_checks::ExternalChecks; +use move_compiler_v2::{ + diagnostics::message_format::MessageFormat, external_checks::ExternalChecks, +}; use move_core_types::account_address::AccountAddress; use move_model::{ metadata::{CompilerVersion, LanguageVersion}, @@ -172,6 +174,9 @@ pub struct CompilerConfig { /// Experiments for v2 compiler to set to true #[clap(long, global = true)] pub experiments: Vec, + + #[clap(long, default_value = "human")] + pub message_format: MessageFormat, } #[derive(Debug, Clone, Eq, PartialEq, PartialOrd)] From 51040e4243b8929741811fdabee64812e4a1b1b7 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Fri, 13 Dec 2024 22:23:59 +0100 Subject: [PATCH 02/15] get rid of diagnostic reporter --- .../move-compiler-v2/src/diagnostics/mod.rs | 20 ++++---------- third_party/move/move-compiler-v2/src/lib.rs | 26 ++++++++++--------- .../src/compilation/compiled_package.rs | 7 ++--- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs index ba34e841a524c..c8ad4afe9c717 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs @@ -7,26 +7,20 @@ pub mod human; pub mod json; pub mod message_format; -pub struct DiagnosticReporter { - emitter: Box, -} - -impl DiagnosticReporter { - pub fn new(emitter: Box) -> Self { - DiagnosticReporter { emitter } - } +pub trait Emitter { + fn emit(&mut self, source_files: &Files, diag: &Diagnostic); /// Writes accumulated diagnostics of given or higher severity. - pub fn report_diag(&mut self, global_env: &GlobalEnv, severity: Severity) { + fn report_diag(&mut self, global_env: &GlobalEnv, severity: Severity) { global_env.report_diag_with_filter( - |files, diag| self.emitter.as_mut().emit(files, diag), + |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. - pub fn check_diag( + fn check_diag( &mut self, global_env: &GlobalEnv, report_severity: Severity, @@ -40,7 +34,3 @@ impl DiagnosticReporter { } } } - -pub trait Emitter { - fn emit(&mut self, source_files: &Files, diag: &Diagnostic); -} diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index cd04430e37f99..df7e380446389 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -15,7 +15,7 @@ pub mod pipeline; pub mod plan_builder; use crate::{ - diagnostics::DiagnosticReporter, + diagnostics::Emitter, env_pipeline::{ acquires_checker, ast_simplifier, cyclic_instantiation_checker, flow_insensitive_checkers, function_checker, inliner, lambda_lifter, lambda_lifter::LambdaLiftingOptions, @@ -156,8 +156,8 @@ where } /// Run move compiler and print errors to given writer. Returns the set of compiled units. -pub fn run_move_compiler_with_reporter( - reporter: &mut DiagnosticReporter, +pub fn run_move_compiler_with_emitter( + mut emitter: Box, options: Options, ) -> anyhow::Result<(GlobalEnv, Vec)> { logging::setup_logging(); @@ -165,7 +165,7 @@ pub fn run_move_compiler_with_reporter( // Run context check. let mut env = run_checker_and_rewriters(options.clone())?; - check_errors_with_reporter(&env, reporter, "checking errors")?; + check_errors_with_emitter(&env, emitter.as_mut(), "checking errors")?; if options.experiment_on(Experiment::STOP_BEFORE_STACKLESS_BYTECODE) { std::process::exit(0) @@ -173,7 +173,7 @@ pub fn run_move_compiler_with_reporter( // Run code generator let mut targets = run_bytecode_gen(&env); - check_errors_with_reporter(&env, reporter, "code generation errors")?; + check_errors_with_emitter(&env, emitter.as_mut(), "code generation errors")?; // check_errors(&env, error_writer, "code generation errors")?; debug!("After bytecode_gen, GlobalEnv={}", env.dump_env()); @@ -201,7 +201,7 @@ pub fn run_move_compiler_with_reporter( } else { pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors()) } - check_errors_with_reporter(&env, reporter, "stackless-bytecode analysis errors")?; + check_errors_with_emitter(&env, emitter.as_mut(), "stackless-bytecode analysis errors")?; // check_errors(&env, error_writer, "stackless-bytecode analysis errors")?; if options.experiment_on(Experiment::STOP_BEFORE_FILE_FORMAT) { @@ -209,7 +209,7 @@ pub fn run_move_compiler_with_reporter( } let modules_and_scripts = run_file_format_gen(&mut env, &targets); - check_errors_with_reporter(&env, reporter, "assembling errors")?; + check_errors_with_emitter(&env, emitter.as_mut(), "assembling errors")?; // check_errors(&env, error_writer, "assembling errors")?; debug!( @@ -219,7 +219,7 @@ pub fn run_move_compiler_with_reporter( let annotated_units = annotate_units(modules_and_scripts); run_bytecode_verifier(&annotated_units, &mut env); - check_errors_with_reporter(&env, reporter, "bytecode verification errors")?; + check_errors_with_emitter(&env, emitter.as_mut(), "bytecode verification errors")?; // check_errors(&env, error_writer, "bytecode verification errors")?; // Finally mark this model to be generated by v2 @@ -690,15 +690,17 @@ where } /// Report any diags in the env to the writer and fail if there are errors. -pub fn check_errors_with_reporter( +pub fn check_errors_with_emitter( env: &GlobalEnv, - reporter: &mut DiagnosticReporter, + emitter: &mut dyn Emitter, msg: &str, ) -> anyhow::Result<()> { let options = env.get_extension::().unwrap_or_default(); - reporter.report_diag(env, options.report_severity()); + + emitter.report_diag(env, options.report_severity()); + emitter.check_diag(env, options.report_severity(), msg) + // env.report_diag(error_writer, options.report_severity()); - reporter.check_diag(env, options.report_severity(), msg) // env.check_diag(error_writer, options.report_severity(), msg) } diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index 6d25dcce9601a..d9722b69cabcf 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -27,9 +27,7 @@ use move_compiler::{ shared::{Flags, NamedAddressMap, NumericalAddress, PackagePaths}, Compiler, }; -use move_compiler_v2::{ - diagnostics::DiagnosticReporter, external_checks::ExternalChecks, Experiment, -}; +use move_compiler_v2::{external_checks::ExternalChecks, Experiment}; use move_docgen::{Docgen, DocgenOptions}; use move_model::{ model::GlobalEnv, options::ModelBuilderOptions, @@ -1159,8 +1157,7 @@ pub fn build_and_report_no_exit_v2_driver( options: move_compiler_v2::Options, ) -> CompilerDriverResult { let emitter = options.message_format.clone().into_emitter(); - let mut reporter = DiagnosticReporter::new(emitter); - let (env, units) = move_compiler_v2::run_move_compiler_with_reporter(&mut reporter, options)?; + let (env, units) = move_compiler_v2::run_move_compiler_with_emitter(emitter, options)?; Ok(( move_compiler_v2::make_files_source_text(&env), units, From 7c92606ead5bb947306a6eb69973dc638e4b1bbf Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Wed, 18 Dec 2024 14:50:33 +0100 Subject: [PATCH 03/15] get rid of explicit message_format flag in favor of experiments --- aptos-move/framework/src/built_package.rs | 7 +------ crates/aptos/src/move_tool/coverage.rs | 2 -- crates/aptos/src/move_tool/lint.rs | 11 +++++----- crates/aptos/src/move_tool/mod.rs | 7 +------ crates/aptos/src/test/mod.rs | 1 - .../move-compiler-v2/src/diagnostics/human.rs | 5 +++-- .../move-compiler-v2/src/diagnostics/json.rs | 5 +++-- .../src/diagnostics/message_format.rs | 17 ---------------- .../move-compiler-v2/src/diagnostics/mod.rs | 20 ++++++++++++++++++- .../move/move-compiler-v2/src/experiments.rs | 6 ++++++ .../move/move-compiler-v2/src/options.rs | 4 ---- .../ast-generator-tests/tests/testsuite.rs | 1 + third_party/move/move-prover/src/lib.rs | 3 +-- .../src/framework.rs | 1 + .../move/tools/move-linter/tests/testsuite.rs | 1 + .../src/compilation/compiled_package.rs | 3 +-- .../move/tools/move-package/src/lib.rs | 5 +---- 17 files changed, 45 insertions(+), 54 deletions(-) diff --git a/aptos-move/framework/src/built_package.rs b/aptos-move/framework/src/built_package.rs index 00652ea438c9f..c3faf12a76cc0 100644 --- a/aptos-move/framework/src/built_package.rs +++ b/aptos-move/framework/src/built_package.rs @@ -20,7 +20,7 @@ use move_binary_format::{file_format_common::VERSION_7, CompiledModule}; use move_command_line_common::files::MOVE_COMPILED_EXTENSION; use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule}; use move_compiler_v2::{ - diagnostics::message_format::MessageFormat, external_checks::ExternalChecks, options::Options, + external_checks::ExternalChecks, options::Options, Experiment, }; use move_core_types::{language_storage::ModuleId, metadata::Metadata}; @@ -110,8 +110,6 @@ pub struct BuildOptions { /// Select bytecode, language, compiler for Move 2 #[clap(long)] pub move_2: bool, - #[clap(long, default_value = "human")] - pub message_format: MessageFormat, } // Because named_addresses has no parser, we can't use clap's default impl. This must be aligned @@ -140,7 +138,6 @@ impl Default for BuildOptions { known_attributes: extended_checks::get_all_attribute_names().clone(), experiments: vec![], move_2: false, - message_format: MessageFormat::default(), } } } @@ -213,7 +210,6 @@ pub fn build_model( skip_attribute_checks, known_attributes, experiments, - message_format: MessageFormat::default(), }, }; let compiler_version = compiler_version.unwrap_or_default(); @@ -270,7 +266,6 @@ impl BuiltPackage { skip_attribute_checks, known_attributes: options.known_attributes.clone(), experiments: options.experiments.clone(), - message_format: options.message_format.clone(), }, }; diff --git a/crates/aptos/src/move_tool/coverage.rs b/crates/aptos/src/move_tool/coverage.rs index 6fb958bf9a91b..366b6bb9472f9 100644 --- a/crates/aptos/src/move_tool/coverage.rs +++ b/crates/aptos/src/move_tool/coverage.rs @@ -9,7 +9,6 @@ use aptos_framework::extended_checks; use async_trait::async_trait; use clap::{Parser, Subcommand}; use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule}; -use move_compiler_v2::diagnostics::message_format::MessageFormat; use move_coverage::{ coverage_map::CoverageMap, format_csv_summary, format_human_summary, @@ -179,7 +178,6 @@ fn compile_coverage( compiler_version: move_options.compiler_version, language_version: move_options.language_version, experiments: experiments_from_opt_level(&move_options.optimize), - message_format: MessageFormat::default(), }, ..Default::default() }; diff --git a/crates/aptos/src/move_tool/lint.rs b/crates/aptos/src/move_tool/lint.rs index 9851e5e65d59a..954f83b9b9269 100644 --- a/crates/aptos/src/move_tool/lint.rs +++ b/crates/aptos/src/move_tool/lint.rs @@ -8,7 +8,7 @@ use crate::{ use aptos_framework::{BuildOptions, BuiltPackage}; use async_trait::async_trait; use clap::Parser; -use move_compiler_v2::{diagnostics::message_format::MessageFormat, Experiment}; +use move_compiler_v2::Experiment; use move_linter::MoveLintChecks; use move_model::metadata::{CompilerVersion, LanguageVersion, LATEST_STABLE_LANGUAGE_VERSION}; use move_package::source_package::std_lib::StdVersion; @@ -76,8 +76,9 @@ pub struct LintPackage { #[clap(long, env = "APTOS_CHECK_TEST_CODE")] pub check_test_code: bool, - #[clap(long, default_value = "human")] - pub message_format: MessageFormat, + /// Experiments + #[clap(long, hide(true))] + pub experiments: Vec, } impl LintPackage { @@ -92,7 +93,7 @@ impl LintPackage { language_version, skip_attribute_checks, check_test_code, - message_format: _, + experiments, } = self.clone(); MovePackageDir { dev, @@ -104,6 +105,7 @@ impl LintPackage { language_version, skip_attribute_checks, check_test_code, + experiments, ..MovePackageDir::new() } } @@ -130,7 +132,6 @@ impl CliCommand<&'static str> for LintPackage { let package_path = move_options.get_package_path()?; let included_artifacts = IncludedArtifacts::Sparse; let build_options = BuildOptions { - message_format: self.message_format, ..included_artifacts.build_options_with_experiments( &move_options, more_experiments, diff --git a/crates/aptos/src/move_tool/mod.rs b/crates/aptos/src/move_tool/mod.rs index 298a795251fb0..b2959e013c0b8 100644 --- a/crates/aptos/src/move_tool/mod.rs +++ b/crates/aptos/src/move_tool/mod.rs @@ -60,7 +60,7 @@ use colored::Colorize; use itertools::Itertools; use move_cli::{self, base::test::UnitTestResult}; use move_command_line_common::{address::NumericalAddress, env::MOVE_HOME}; -use move_compiler_v2::{diagnostics::message_format::MessageFormat, Experiment}; +use move_compiler_v2::Experiment; use move_core_types::{identifier::Identifier, language_storage::ModuleId, u256::U256}; use move_model::metadata::{CompilerVersion, LanguageVersion}; use move_package::{source_package::layout::SourcePackageLayout, BuildConfig, CompilerConfig}; @@ -390,9 +390,6 @@ pub struct CompilePackage { #[clap(long)] pub save_metadata: bool, - #[clap(long, default_value = "human")] - pub message_format: MessageFormat, - #[clap(flatten)] pub included_artifacts_args: IncludedArtifactsArgs, #[clap(flatten)] @@ -408,7 +405,6 @@ impl CliCommand> for CompilePackage { async fn execute(self) -> CliTypedResult> { let build_options = BuildOptions { install_dir: self.move_options.output_dir.clone(), - message_format: self.message_format.clone(), ..self .included_artifacts_args .included_artifacts @@ -571,7 +567,6 @@ impl CliCommand<&'static str> for TestPackage { compiler_version: self.move_options.compiler_version, language_version: self.move_options.language_version, experiments: experiments_from_opt_level(&self.move_options.optimize), - message_format: MessageFormat::default(), }, ..Default::default() }; diff --git a/crates/aptos/src/test/mod.rs b/crates/aptos/src/test/mod.rs index 4a2eaa945384e..7dc5a9f27abf9 100644 --- a/crates/aptos/src/test/mod.rs +++ b/crates/aptos/src/test/mod.rs @@ -857,7 +857,6 @@ impl CliTestFramework { included_artifacts_args: IncludedArtifactsArgs { included_artifacts: included_artifacts.unwrap_or(IncludedArtifacts::Sparse), }, - message_format: MessageFormat::default(), } .execute() .await diff --git a/third_party/move/move-compiler-v2/src/diagnostics/human.rs b/third_party/move/move-compiler-v2/src/diagnostics/human.rs index 47e44af1b1cd1..0015538fe9d6d 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/human.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/human.rs @@ -13,8 +13,9 @@ impl HumanEmitter where W: WriteColor, { - pub fn new(writer: W) -> Self { - HumanEmitter { writer } + pub fn new(writer: W) -> Box { + let emitter = HumanEmitter { writer }; + Box::new(emitter) } } diff --git a/third_party/move/move-compiler-v2/src/diagnostics/json.rs b/third_party/move/move-compiler-v2/src/diagnostics/json.rs index d25071bdc8f12..631db00543b97 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/json.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/json.rs @@ -8,8 +8,9 @@ pub struct JsonEmitter { } impl JsonEmitter { - pub fn new(writer: W) -> Self { - JsonEmitter { writer } + pub fn new(writer: W) -> Box { + let emitter = JsonEmitter { writer }; + Box::new(emitter) } } diff --git a/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs b/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs index 4e69defaca7d8..39f54753fcea7 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs @@ -1,6 +1,4 @@ -use crate::diagnostics::{human::HumanEmitter, json::JsonEmitter, Emitter}; use clap::ValueEnum; -use codespan_reporting::term::termcolor::{ColorChoice, StandardStream}; use serde::{Deserialize, Serialize}; #[derive(Debug, Default, Serialize, Deserialize, PartialOrd, Eq, PartialEq, Clone, ValueEnum)] @@ -9,18 +7,3 @@ pub enum MessageFormat { Human, Json, } - -impl MessageFormat { - pub fn into_emitter(self) -> Box { - match self { - MessageFormat::Human => { - let stderr = StandardStream::stderr(ColorChoice::Auto); - Box::new(HumanEmitter::new(stderr)) - }, - MessageFormat::Json => { - let stderr = StandardStream::stderr(ColorChoice::Auto); - Box::new(JsonEmitter::new(stderr)) - }, - } - } -} diff --git a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs index c8ad4afe9c717..c4e0f84552924 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs @@ -1,12 +1,30 @@ +use crate::{ + diagnostics::{human::HumanEmitter, json::JsonEmitter}, + options, Experiment, +}; use anyhow::bail; use codespan::{FileId, Files}; -use codespan_reporting::diagnostic::{Diagnostic, Severity}; +use codespan_reporting::{ + diagnostic::{Diagnostic, Severity}, + term::termcolor::{ColorChoice, StandardStream}, +}; use move_model::model::GlobalEnv; pub mod human; pub mod json; pub mod message_format; +impl options::Options { + pub fn to_emitter(&self) -> Box { + let stderr = StandardStream::stderr(ColorChoice::Auto); + if self.experiment_on(Experiment::MESSAGE_FORMAT_JSON) { + JsonEmitter::new(stderr) + } else { + HumanEmitter::new(stderr) + } + } +} + pub trait Emitter { fn emit(&mut self, source_files: &Files, diag: &Diagnostic); diff --git a/third_party/move/move-compiler-v2/src/experiments.rs b/third_party/move/move-compiler-v2/src/experiments.rs index fd6174e8231bf..b952dadea9930 100644 --- a/third_party/move/move-compiler-v2/src/experiments.rs +++ b/third_party/move/move-compiler-v2/src/experiments.rs @@ -276,6 +276,11 @@ pub static EXPERIMENTS: Lazy> = Lazy::new(|| { description: "Avoid storing to a local during assigns".to_string(), default: Inherited(Experiment::OPTIMIZE_WAITING_FOR_COMPARE_TESTS.to_string()), }, + Experiment { + name: Experiment::MESSAGE_FORMAT_JSON.to_string(), + description: "Enable json format for compiler messages".to_string(), + default: Given(false), + } ]; experiments .into_iter() @@ -330,4 +335,5 @@ impl Experiment { pub const USAGE_CHECK: &'static str = "usage-check"; pub const VARIABLE_COALESCING: &'static str = "variable-coalescing"; pub const VARIABLE_COALESCING_ANNOTATE: &'static str = "variable-coalescing-annotate"; + pub const MESSAGE_FORMAT_JSON: &'static str = "message-format-json"; } diff --git a/third_party/move/move-compiler-v2/src/options.rs b/third_party/move/move-compiler-v2/src/options.rs index 88fdd6d1f164d..0df18f732cd59 100644 --- a/third_party/move/move-compiler-v2/src/options.rs +++ b/third_party/move/move-compiler-v2/src/options.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - diagnostics::message_format::MessageFormat, experiments::{DefaultValue, EXPERIMENTS}, external_checks::ExternalChecks, }; @@ -130,9 +129,6 @@ pub struct Options { /// External checks to be performed. #[clap(skip)] pub external_checks: Vec>, - - #[clap(long, default_value = "human")] - pub message_format: MessageFormat, } impl Default for Options { diff --git a/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs b/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs index dcd03c9ca3eb9..fd3a3d03806b2 100644 --- a/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs +++ b/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs @@ -15,6 +15,7 @@ use std::{ collections::BTreeSet, path::{Path, PathBuf}, }; +use move_compiler_v2::diagnostics::buffer::BufferEmitter; /// Extension for expected output files pub const EXP_EXT: &str = "exp"; diff --git a/third_party/move/move-prover/src/lib.rs b/third_party/move/move-prover/src/lib.rs index 7a09645c602ce..61c115343e07f 100644 --- a/third_party/move/move-prover/src/lib.rs +++ b/third_party/move/move-prover/src/lib.rs @@ -12,7 +12,7 @@ use log::{debug, info, warn}; use move_abigen::Abigen; use move_compiler::shared::{known_attributes::KnownAttribute, PackagePaths}; use move_compiler_v2::{ - diagnostics::message_format::MessageFormat, env_pipeline::rewrite_target::RewritingScope, + env_pipeline::rewrite_target::RewritingScope, Experiment, }; use move_docgen::Docgen; @@ -95,7 +95,6 @@ pub fn run_move_prover_v2( compile_test_code: false, compile_verify_code: true, external_checks: vec![], - message_format: MessageFormat::default(), }; let mut env = move_compiler_v2::run_move_compiler_for_analysis(error_writer, compiler_options)?; diff --git a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs index be05bd2c0e333..ddd26c0345cab 100644 --- a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs +++ b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs @@ -59,6 +59,7 @@ use std::{ path::Path, }; use tempfile::NamedTempFile; +use move_compiler_v2::diagnostics::buffer::BufferEmitter; pub struct ProcessedModule { module: CompiledModule, diff --git a/third_party/move/tools/move-linter/tests/testsuite.rs b/third_party/move/tools/move-linter/tests/testsuite.rs index 454957cdd161b..6955bf87473ab 100644 --- a/third_party/move/tools/move-linter/tests/testsuite.rs +++ b/third_party/move/tools/move-linter/tests/testsuite.rs @@ -7,6 +7,7 @@ use move_linter::MoveLintChecks; use move_model::metadata::{CompilerVersion, LanguageVersion}; use move_prover_test_utils::baseline_test; use std::path::{Path, PathBuf}; +use move_compiler_v2::diagnostics::Emitter; /// Extension for expected output files. pub const EXP_EXT: &str = "exp"; diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index d9722b69cabcf..4f1908fb4152d 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -699,7 +699,6 @@ impl CompiledPackage { compile_test_code: flags.keep_testing_functions(), experiments: config.experiments.clone(), external_checks, - message_format: config.message_format.clone(), ..Default::default() }; options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true); @@ -1156,7 +1155,7 @@ pub fn build_and_report_v2_driver(options: move_compiler_v2::Options) -> Compile pub fn build_and_report_no_exit_v2_driver( options: move_compiler_v2::Options, ) -> CompilerDriverResult { - let emitter = options.message_format.clone().into_emitter(); + let emitter = options.to_emitter(); let (env, units) = move_compiler_v2::run_move_compiler_with_emitter(emitter, options)?; Ok(( move_compiler_v2::make_files_source_text(&env), diff --git a/third_party/move/tools/move-package/src/lib.rs b/third_party/move/tools/move-package/src/lib.rs index 467600a70c3cd..9b62ab48f55fb 100644 --- a/third_party/move/tools/move-package/src/lib.rs +++ b/third_party/move/tools/move-package/src/lib.rs @@ -23,7 +23,7 @@ use move_compiler::{ command_line::SKIP_ATTRIBUTE_CHECKS, shared::known_attributes::KnownAttribute, }; use move_compiler_v2::{ - diagnostics::message_format::MessageFormat, external_checks::ExternalChecks, + external_checks::ExternalChecks, }; use move_core_types::account_address::AccountAddress; use move_model::{ @@ -174,9 +174,6 @@ pub struct CompilerConfig { /// Experiments for v2 compiler to set to true #[clap(long, global = true)] pub experiments: Vec, - - #[clap(long, default_value = "human")] - pub message_format: MessageFormat, } #[derive(Debug, Clone, Eq, PartialEq, PartialOrd)] From dc7c5631982f0d0ed57d6d0e2018a298e5ad3532 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Wed, 18 Dec 2024 16:11:26 +0100 Subject: [PATCH 04/15] receive mut ref writer instead of value to enable code reuse --- aptos-move/framework/src/built_package.rs | 5 +- crates/aptos/src/test/mod.rs | 1 - .../move-compiler-v2/src/diagnostics/human.rs | 13 ++- .../move-compiler-v2/src/diagnostics/json.rs | 15 ++- .../src/diagnostics/message_format.rs | 9 -- .../move-compiler-v2/src/diagnostics/mod.rs | 10 +- .../move/move-compiler-v2/src/experiments.rs | 4 +- third_party/move/move-compiler-v2/src/lib.rs | 102 +++--------------- .../ast-generator-tests/tests/testsuite.rs | 1 - third_party/move/move-prover/src/lib.rs | 5 +- .../src/framework.rs | 1 - .../move/tools/move-linter/tests/testsuite.rs | 1 - .../src/compilation/compiled_package.rs | 5 +- .../move/tools/move-package/src/lib.rs | 4 +- 14 files changed, 38 insertions(+), 138 deletions(-) delete mode 100644 third_party/move/move-compiler-v2/src/diagnostics/message_format.rs diff --git a/aptos-move/framework/src/built_package.rs b/aptos-move/framework/src/built_package.rs index c3faf12a76cc0..1a7d5edfb3d9f 100644 --- a/aptos-move/framework/src/built_package.rs +++ b/aptos-move/framework/src/built_package.rs @@ -19,10 +19,7 @@ use itertools::Itertools; use move_binary_format::{file_format_common::VERSION_7, CompiledModule}; use move_command_line_common::files::MOVE_COMPILED_EXTENSION; use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule}; -use move_compiler_v2::{ - external_checks::ExternalChecks, options::Options, - Experiment, -}; +use move_compiler_v2::{external_checks::ExternalChecks, options::Options, Experiment}; use move_core_types::{language_storage::ModuleId, metadata::Metadata}; use move_model::{ metadata::{ diff --git a/crates/aptos/src/test/mod.rs b/crates/aptos/src/test/mod.rs index 7dc5a9f27abf9..eee3d6e19a5d9 100644 --- a/crates/aptos/src/test/mod.rs +++ b/crates/aptos/src/test/mod.rs @@ -64,7 +64,6 @@ use aptos_rest_client::{ use aptos_sdk::move_types::{account_address::AccountAddress, language_storage::ModuleId}; use aptos_temppath::TempPath; use aptos_types::on_chain_config::ValidatorSet; -use move_compiler_v2::diagnostics::message_format::MessageFormat; use move_core_types::ident_str; use reqwest::Url; use serde::{Deserialize, Serialize}; diff --git a/third_party/move/move-compiler-v2/src/diagnostics/human.rs b/third_party/move/move-compiler-v2/src/diagnostics/human.rs index 0015538fe9d6d..aa4770e68746a 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/human.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/human.rs @@ -5,21 +5,20 @@ use codespan_reporting::{ term::{emit, termcolor::WriteColor, Config}, }; -pub struct HumanEmitter { - writer: W, +pub struct HumanEmitter<'w, W: WriteColor> { + writer: &'w mut W, } -impl HumanEmitter +impl<'w, W> HumanEmitter<'w, W> where W: WriteColor, { - pub fn new(writer: W) -> Box { - let emitter = HumanEmitter { writer }; - Box::new(emitter) + pub fn new(writer: &'w mut W) -> Self { + HumanEmitter { writer } } } -impl Emitter for HumanEmitter +impl<'w, W> Emitter for HumanEmitter<'w, W> where W: WriteColor, { diff --git a/third_party/move/move-compiler-v2/src/diagnostics/json.rs b/third_party/move/move-compiler-v2/src/diagnostics/json.rs index 631db00543b97..1816fcab7e5bf 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/json.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/json.rs @@ -1,20 +1,19 @@ use crate::diagnostics::Emitter; use codespan::{FileId, Files}; use codespan_reporting::diagnostic::Diagnostic; -use std::io; +use std::io::Write; -pub struct JsonEmitter { - writer: W, +pub struct JsonEmitter<'w, W: Write> { + writer: &'w mut W, } -impl JsonEmitter { - pub fn new(writer: W) -> Box { - let emitter = JsonEmitter { writer }; - Box::new(emitter) +impl<'w, W: Write> JsonEmitter<'w, W> { + pub fn new(writer: &'w mut W) -> Self { + JsonEmitter { writer } } } -impl Emitter for JsonEmitter { +impl<'w, W: Write> Emitter for JsonEmitter<'w, W> { fn emit(&mut self, _source_files: &Files, diag: &Diagnostic) { serde_json::to_writer(&mut self.writer, diag).expect("emit must not fail"); writeln!(&mut self.writer).unwrap(); diff --git a/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs b/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs deleted file mode 100644 index 39f54753fcea7..0000000000000 --- a/third_party/move/move-compiler-v2/src/diagnostics/message_format.rs +++ /dev/null @@ -1,9 +0,0 @@ -use clap::ValueEnum; -use serde::{Deserialize, Serialize}; - -#[derive(Debug, Default, Serialize, Deserialize, PartialOrd, Eq, PartialEq, Clone, ValueEnum)] -pub enum MessageFormat { - #[default] - Human, - Json, -} diff --git a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs index c4e0f84552924..8114dbdabf345 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs @@ -6,21 +6,19 @@ use anyhow::bail; use codespan::{FileId, Files}; use codespan_reporting::{ diagnostic::{Diagnostic, Severity}, - term::termcolor::{ColorChoice, StandardStream}, + term::termcolor::StandardStream, }; use move_model::model::GlobalEnv; pub mod human; pub mod json; -pub mod message_format; impl options::Options { - pub fn to_emitter(&self) -> Box { - let stderr = StandardStream::stderr(ColorChoice::Auto); + pub fn to_emitter<'w>(&self, stderr: &'w mut StandardStream) -> Box { if self.experiment_on(Experiment::MESSAGE_FORMAT_JSON) { - JsonEmitter::new(stderr) + Box::new(JsonEmitter::new(stderr)) } else { - HumanEmitter::new(stderr) + Box::new(HumanEmitter::new(stderr)) } } } diff --git a/third_party/move/move-compiler-v2/src/experiments.rs b/third_party/move/move-compiler-v2/src/experiments.rs index b952dadea9930..7862684265e69 100644 --- a/third_party/move/move-compiler-v2/src/experiments.rs +++ b/third_party/move/move-compiler-v2/src/experiments.rs @@ -280,7 +280,7 @@ pub static EXPERIMENTS: Lazy> = Lazy::new(|| { name: Experiment::MESSAGE_FORMAT_JSON.to_string(), description: "Enable json format for compiler messages".to_string(), default: Given(false), - } + }, ]; experiments .into_iter() @@ -313,6 +313,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 = "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 = @@ -335,5 +336,4 @@ impl Experiment { pub const USAGE_CHECK: &'static str = "usage-check"; pub const VARIABLE_COALESCING: &'static str = "variable-coalescing"; pub const VARIABLE_COALESCING_ANNOTATE: &'static str = "variable-coalescing-annotate"; - pub const MESSAGE_FORMAT_JSON: &'static str = "message-format-json"; } diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index df7e380446389..75ce5b5117d30 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -15,7 +15,7 @@ pub mod pipeline; pub mod plan_builder; use crate::{ - diagnostics::Emitter, + diagnostics::{human::HumanEmitter, Emitter}, env_pipeline::{ acquires_checker, ast_simplifier, cyclic_instantiation_checker, flow_insensitive_checkers, function_checker, inliner, lambda_lifter, lambda_lifter::LambdaLiftingOptions, @@ -79,8 +79,9 @@ use std::{collections::BTreeSet, io::Write, path::Path}; pub fn run_move_compiler_to_stderr( options: Options, ) -> anyhow::Result<(GlobalEnv, Vec)> { - 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); + run_move_compiler_with_emitter(&mut emitter, options) } /// Run move compiler and print errors to given writer. Returns the set of compiled units. @@ -91,73 +92,13 @@ pub fn run_move_compiler( where W: WriteColor + Write, { - 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")?; - - 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")?; - debug!("After bytecode_gen, GlobalEnv={}", env.dump_env()); - - // Run transformation pipeline - let pipeline = bytecode_pipeline(&env); - if log_enabled!(Level::Debug) { - // Dump bytecode, providing a name for the target derived from the first input file. - let dump_base_name = options - .sources - .first() - .and_then(|f| { - Path::new(f) - .file_name() - .map(|f| f.to_string_lossy().as_ref().to_owned()) - }) - .unwrap_or_else(|| "dump".to_owned()); - pipeline.run_with_dump( - &env, - &mut targets, - &dump_base_name, - false, - &pipeline::register_formatters, - || !env.has_errors(), - ) - } else { - pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors()) - } - check_errors(&env, error_writer, "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")?; - - debug!( - "File format bytecode:\n{}", - disassemble_compiled_units(&modules_and_scripts)? - ); - - let annotated_units = annotate_units(modules_and_scripts); - run_bytecode_verifier(&annotated_units, &mut env); - check_errors(&env, error_writer, "bytecode verification errors")?; - - // Finally mark this model to be generated by v2 - env.set_compiler_v2(true); - - Ok((env, annotated_units)) + let mut emitter = HumanEmitter::new(error_writer); + run_move_compiler_with_emitter(&mut emitter, options) } /// Run move compiler and print errors to given writer. Returns the set of compiled units. pub fn run_move_compiler_with_emitter( - mut emitter: Box, + emitter: &mut dyn Emitter, options: Options, ) -> anyhow::Result<(GlobalEnv, Vec)> { logging::setup_logging(); @@ -165,7 +106,7 @@ pub fn run_move_compiler_with_emitter( // Run context check. let mut env = run_checker_and_rewriters(options.clone())?; - check_errors_with_emitter(&env, emitter.as_mut(), "checking errors")?; + check_errors(&env, emitter, "checking errors")?; if options.experiment_on(Experiment::STOP_BEFORE_STACKLESS_BYTECODE) { std::process::exit(0) @@ -173,7 +114,7 @@ pub fn run_move_compiler_with_emitter( // Run code generator let mut targets = run_bytecode_gen(&env); - check_errors_with_emitter(&env, emitter.as_mut(), "code generation errors")?; + check_errors(&env, emitter, "code generation errors")?; // check_errors(&env, error_writer, "code generation errors")?; debug!("After bytecode_gen, GlobalEnv={}", env.dump_env()); @@ -201,7 +142,7 @@ pub fn run_move_compiler_with_emitter( } else { pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors()) } - check_errors_with_emitter(&env, emitter.as_mut(), "stackless-bytecode analysis errors")?; + check_errors(&env, emitter, "stackless-bytecode analysis errors")?; // check_errors(&env, error_writer, "stackless-bytecode analysis errors")?; if options.experiment_on(Experiment::STOP_BEFORE_FILE_FORMAT) { @@ -209,7 +150,7 @@ pub fn run_move_compiler_with_emitter( } let modules_and_scripts = run_file_format_gen(&mut env, &targets); - check_errors_with_emitter(&env, emitter.as_mut(), "assembling errors")?; + check_errors(&env, emitter, "assembling errors")?; // check_errors(&env, error_writer, "assembling errors")?; debug!( @@ -219,7 +160,7 @@ pub fn run_move_compiler_with_emitter( let annotated_units = annotate_units(modules_and_scripts); run_bytecode_verifier(&annotated_units, &mut env); - check_errors_with_emitter(&env, emitter.as_mut(), "bytecode verification errors")?; + check_errors(&env, emitter, "bytecode verification errors")?; // check_errors(&env, error_writer, "bytecode verification errors")?; // Finally mark this model to be generated by v2 @@ -680,28 +621,11 @@ 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(env: &GlobalEnv, error_writer: &mut W, msg: &str) -> anyhow::Result<()> -where - W: WriteColor + Write, -{ - let options = env.get_extension::().unwrap_or_default(); - env.report_diag(error_writer, options.report_severity()); - env.check_diag(error_writer, options.report_severity(), msg) -} - -/// Report any diags in the env to the writer and fail if there are errors. -pub fn check_errors_with_emitter( - env: &GlobalEnv, - emitter: &mut dyn Emitter, - msg: &str, -) -> anyhow::Result<()> { +pub fn check_errors(env: &GlobalEnv, emitter: &mut dyn Emitter, msg: &str) -> anyhow::Result<()> { let options = env.get_extension::().unwrap_or_default(); emitter.report_diag(env, options.report_severity()); emitter.check_diag(env, options.report_severity(), msg) - - // env.report_diag(error_writer, options.report_severity()); - // env.check_diag(error_writer, options.report_severity(), msg) } /// Annotate the given compiled units. diff --git a/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs b/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs index fd3a3d03806b2..dcd03c9ca3eb9 100644 --- a/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs +++ b/third_party/move/move-model/bytecode/ast-generator-tests/tests/testsuite.rs @@ -15,7 +15,6 @@ use std::{ collections::BTreeSet, path::{Path, PathBuf}, }; -use move_compiler_v2::diagnostics::buffer::BufferEmitter; /// Extension for expected output files pub const EXP_EXT: &str = "exp"; diff --git a/third_party/move/move-prover/src/lib.rs b/third_party/move/move-prover/src/lib.rs index 61c115343e07f..46810fe825d9d 100644 --- a/third_party/move/move-prover/src/lib.rs +++ b/third_party/move/move-prover/src/lib.rs @@ -11,10 +11,7 @@ use codespan_reporting::term::termcolor::{ColorChoice, StandardStream, WriteColo use log::{debug, info, warn}; use move_abigen::Abigen; use move_compiler::shared::{known_attributes::KnownAttribute, PackagePaths}; -use move_compiler_v2::{ - env_pipeline::rewrite_target::RewritingScope, - Experiment, -}; +use move_compiler_v2::{env_pipeline::rewrite_target::RewritingScope, Experiment}; use move_docgen::Docgen; use move_errmapgen::ErrmapGen; use move_model::{ diff --git a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs index ddd26c0345cab..be05bd2c0e333 100644 --- a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs +++ b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs @@ -59,7 +59,6 @@ use std::{ path::Path, }; use tempfile::NamedTempFile; -use move_compiler_v2::diagnostics::buffer::BufferEmitter; pub struct ProcessedModule { module: CompiledModule, diff --git a/third_party/move/tools/move-linter/tests/testsuite.rs b/third_party/move/tools/move-linter/tests/testsuite.rs index 6955bf87473ab..454957cdd161b 100644 --- a/third_party/move/tools/move-linter/tests/testsuite.rs +++ b/third_party/move/tools/move-linter/tests/testsuite.rs @@ -7,7 +7,6 @@ use move_linter::MoveLintChecks; use move_model::metadata::{CompilerVersion, LanguageVersion}; use move_prover_test_utils::baseline_test; use std::path::{Path, PathBuf}; -use move_compiler_v2::diagnostics::Emitter; /// Extension for expected output files. pub const EXP_EXT: &str = "exp"; diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index 4f1908fb4152d..7ecbdc2987d38 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -1155,8 +1155,9 @@ pub fn build_and_report_v2_driver(options: move_compiler_v2::Options) -> Compile pub fn build_and_report_no_exit_v2_driver( options: move_compiler_v2::Options, ) -> CompilerDriverResult { - let emitter = options.to_emitter(); - let (env, units) = move_compiler_v2::run_move_compiler_with_emitter(emitter, options)?; + let mut stderr = StandardStream::stderr(ColorChoice::Auto); + let mut emitter = options.to_emitter(&mut stderr); + let (env, units) = move_compiler_v2::run_move_compiler_with_emitter(emitter.as_mut(), options)?; Ok(( move_compiler_v2::make_files_source_text(&env), units, diff --git a/third_party/move/tools/move-package/src/lib.rs b/third_party/move/tools/move-package/src/lib.rs index 9b62ab48f55fb..75f7d112f1eae 100644 --- a/third_party/move/tools/move-package/src/lib.rs +++ b/third_party/move/tools/move-package/src/lib.rs @@ -22,9 +22,7 @@ use clap::*; use move_compiler::{ command_line::SKIP_ATTRIBUTE_CHECKS, shared::known_attributes::KnownAttribute, }; -use move_compiler_v2::{ - external_checks::ExternalChecks, -}; +use move_compiler_v2::external_checks::ExternalChecks; use move_core_types::account_address::AccountAddress; use move_model::{ metadata::{CompilerVersion, LanguageVersion}, From 8b27689bb4e982d8b4f51439ebdb256c22d006e2 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Wed, 18 Dec 2024 16:13:47 +0100 Subject: [PATCH 05/15] remove commented code --- third_party/move/move-compiler-v2/src/lib.rs | 5 +-- third_party/move/move-model/src/model.rs | 38 ------------------- .../move-cli/src/base/test_validation.rs | 14 ------- 3 files changed, 1 insertion(+), 56 deletions(-) diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index 75ce5b5117d30..a809bc55615bc 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -115,7 +115,7 @@ pub fn run_move_compiler_with_emitter( // Run code generator let mut targets = run_bytecode_gen(&env); check_errors(&env, emitter, "code generation errors")?; - // check_errors(&env, error_writer, "code generation errors")?; + debug!("After bytecode_gen, GlobalEnv={}", env.dump_env()); // Run transformation pipeline @@ -143,7 +143,6 @@ pub fn run_move_compiler_with_emitter( pipeline.run_with_hook(&env, &mut targets, |_| {}, |_, _, _| !env.has_errors()) } check_errors(&env, emitter, "stackless-bytecode analysis errors")?; - // check_errors(&env, error_writer, "stackless-bytecode analysis errors")?; if options.experiment_on(Experiment::STOP_BEFORE_FILE_FORMAT) { std::process::exit(0) @@ -151,7 +150,6 @@ pub fn run_move_compiler_with_emitter( let modules_and_scripts = run_file_format_gen(&mut env, &targets); check_errors(&env, emitter, "assembling errors")?; - // check_errors(&env, error_writer, "assembling errors")?; debug!( "File format bytecode:\n{}", @@ -161,7 +159,6 @@ pub fn run_move_compiler_with_emitter( let annotated_units = annotate_units(modules_and_scripts); run_bytecode_verifier(&annotated_units, &mut env); check_errors(&env, emitter, "bytecode verification errors")?; - // check_errors(&env, error_writer, "bytecode verification errors")?; // Finally mark this model to be generated by v2 env.set_compiler_v2(true); diff --git a/third_party/move/move-model/src/model.rs b/third_party/move/move-model/src/model.rs index 3fe632bc65007..07cd129f61bea 100644 --- a/third_party/move/move-model/src/model.rs +++ b/third_party/move/move-model/src/model.rs @@ -1208,7 +1208,6 @@ impl GlobalEnv { }, |d| d.severity >= severity, ); - // self.report_diag_with_filter(writer, |d| d.severity >= severity) } /// Helper function to report diagnostics, check for errors, and fail with a message on @@ -1334,49 +1333,12 @@ impl GlobalEnv { // duplication of expressions via schema inclusion. if shown.insert(format!("{:?}", diag)) { emitter(&self.source_files, diag); - // emit(writer, &Config::default(), &self.source_files, diag) - // .expect("emit must not fail"); } *reported = true; } } } - // /// Writes accumulated diagnostics that pass through `filter` - // pub fn report_diag_with_filter) -> bool>( - // &self, - // writer: &mut W, - // mut filter: F, - // ) { - // let mut shown = BTreeSet::new(); - // self.diags.borrow_mut().sort_by(|a, b| { - // let reported_ordering = a.1.cmp(&b.1); - // if Ordering::Equal == reported_ordering { - // GlobalEnv::cmp_diagnostic(&a.0, &b.0) - // } else { - // reported_ordering - // } - // }); - // for (diag, reported) in self.diags.borrow_mut().iter_mut().filter(|(d, reported)| { - // !reported - // && filter(d) - // && (d.severity >= Severity::Error - // || d.labels - // .iter() - // .any(|label| self.file_id_is_primary_target.contains(&label.file_id))) - // }) { - // if !*reported { - // // 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"); - // } - // *reported = true; - // } - // } - // } - /// Adds a global invariant to this environment. pub fn add_global_invariant(&mut self, inv: GlobalInvariant) { let id = inv.id; diff --git a/third_party/move/tools/move-cli/src/base/test_validation.rs b/third_party/move/tools/move-cli/src/base/test_validation.rs index 9b632500e2baa..1203cd581c221 100644 --- a/third_party/move/tools/move-cli/src/base/test_validation.rs +++ b/third_party/move/tools/move-cli/src/base/test_validation.rs @@ -52,19 +52,5 @@ pub fn has_errors_then_report(model: &GlobalEnv) -> bool { include }, ); - // model.report_diag_with_filter( - // &mut StandardStream::stderr(termcolor::ColorChoice::Auto), - // |d| { - // let include = d.labels.iter().all(|l| { - // let fname = model.get_file(l.file_id).to_string_lossy(); - // !fname.contains("aptos-framework/sources") - // && !fname.contains("aptos-stdlib/sources") - // }); - // if include && d.severity == codespan_reporting::diagnostic::Severity::Error { - // has_errors = true; - // } - // include - // }, - // ); has_errors } From a6065ec11b3a1c7e2151732956d15695e9f20d7b Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Wed, 18 Dec 2024 23:25:47 +0100 Subject: [PATCH 06/15] move from dyn trait in argument position --- third_party/move/move-compiler-v2/src/lib.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index d1ca96f1065cb..1417c21f04319 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -97,10 +97,13 @@ where } /// Run move compiler and print errors to given writer. Returns the set of compiled units. -pub fn run_move_compiler_with_emitter( - emitter: &mut dyn Emitter, +pub fn run_move_compiler_with_emitter( + emitter: &mut E, options: Options, -) -> anyhow::Result<(GlobalEnv, Vec)> { +) -> anyhow::Result<(GlobalEnv, Vec)> +where + E: Emitter + ?Sized, +{ logging::setup_logging(); info!("Move Compiler v2"); @@ -618,7 +621,10 @@ 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(env: &GlobalEnv, emitter: &mut dyn Emitter, msg: &str) -> anyhow::Result<()> { +pub fn check_errors(env: &GlobalEnv, emitter: &mut E, msg: &str) -> anyhow::Result<()> +where + E: Emitter + ?Sized, +{ let options = env.get_extension::().unwrap_or_default(); emitter.report_diag(env, options.report_severity()); From 50070360b1929f48f0662f642cad7c8ed840307d Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Thu, 19 Dec 2024 12:09:46 +0100 Subject: [PATCH 07/15] replace one more run_move_compiler() --- .../tools/move-package/src/compilation/compiled_package.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index 7ecbdc2987d38..15f5103b9feed 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -42,6 +42,7 @@ use std::{ sync::Arc, }; use termcolor::{ColorChoice, StandardStream}; +use move_compiler_v2::diagnostics::human::HumanEmitter; #[derive(Debug, Clone)] pub enum CompilationCachingStatus { @@ -1137,8 +1138,9 @@ pub fn unimplemented_v2_driver(_options: move_compiler_v2::Options) -> CompilerD /// Runs the v2 compiler, exiting the process if any errors occurred. pub fn build_and_report_v2_driver(options: move_compiler_v2::Options) -> CompilerDriverResult { - let mut writer = StandardStream::stderr(ColorChoice::Auto); - match move_compiler_v2::run_move_compiler(&mut writer, options) { + let mut stderr = StandardStream::stderr(ColorChoice::Auto); + let mut emitter = options.to_emitter(&mut stderr); + match move_compiler_v2::run_move_compiler_with_emitter(&mut emitter, options) { Ok((env, units)) => Ok(( move_compiler_v2::make_files_source_text(&env), units, From 2e9c2dee7fc9d5062e260d8d4fbe2a66b51087c4 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Thu, 19 Dec 2024 12:10:54 +0100 Subject: [PATCH 08/15] message-format-json -> compiler-message-format-json --- third_party/move/move-compiler-v2/src/experiments.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/move/move-compiler-v2/src/experiments.rs b/third_party/move/move-compiler-v2/src/experiments.rs index 191dfca472aad..e7a5176aa78a8 100644 --- a/third_party/move/move-compiler-v2/src/experiments.rs +++ b/third_party/move/move-compiler-v2/src/experiments.rs @@ -307,7 +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 = "message-format-json"; + 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 = From 76590da5e497bd6966e17fc1b00ce94b7ad8f14c Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Thu, 19 Dec 2024 12:26:35 +0100 Subject: [PATCH 09/15] remove run_move_compiler_with_emitter --- Cargo.lock | 1 - third_party/move/move-compiler-v2/Cargo.toml | 1 - third_party/move/move-compiler-v2/src/lib.rs | 21 +++++-------------- .../src/framework.rs | 4 +++- .../move/tools/move-linter/tests/testsuite.rs | 5 +++-- .../src/compilation/compiled_package.rs | 5 ++--- 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8c4db4d0faa4b..feb02cd1e0d53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11276,7 +11276,6 @@ dependencies = [ "num 0.4.1", "once_cell", "petgraph 0.6.5", - "serde", "serde_json", "walkdir", ] diff --git a/third_party/move/move-compiler-v2/Cargo.toml b/third_party/move/move-compiler-v2/Cargo.toml index 39e464e534e39..74654702066d0 100644 --- a/third_party/move/move-compiler-v2/Cargo.toml +++ b/third_party/move/move-compiler-v2/Cargo.toml @@ -36,7 +36,6 @@ move-symbol-pool = { workspace = true } num = { workspace = true } once_cell = { workspace = true } petgraph = { workspace = true } -serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } [dev-dependencies] diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index 1417c21f04319..97c6610a10595 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -73,7 +73,7 @@ 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( @@ -81,23 +81,11 @@ pub fn run_move_compiler_to_stderr( ) -> anyhow::Result<(GlobalEnv, Vec)> { let mut stderr = StandardStream::stderr(ColorChoice::Auto); let mut emitter = HumanEmitter::new(&mut stderr); - run_move_compiler_with_emitter(&mut emitter, options) + 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( - error_writer: &mut W, - options: Options, -) -> anyhow::Result<(GlobalEnv, Vec)> -where - W: WriteColor + Write, -{ - let mut emitter = HumanEmitter::new(error_writer); - run_move_compiler_with_emitter(&mut emitter, options) -} - -/// Run move compiler and print errors to given writer. Returns the set of compiled units. -pub fn run_move_compiler_with_emitter( +pub fn run_move_compiler( emitter: &mut E, options: Options, ) -> anyhow::Result<(GlobalEnv, Vec)> @@ -179,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) diff --git a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs index be05bd2c0e333..a13770c87eca8 100644 --- a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs +++ b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs @@ -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}, @@ -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); + 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))?; diff --git a/third_party/move/tools/move-linter/tests/testsuite.rs b/third_party/move/tools/move-linter/tests/testsuite.rs index 454957cdd161b..02ef906c1bdc9 100644 --- a/third_party/move/tools/move-linter/tests/testsuite.rs +++ b/third_party/move/tools/move-linter/tests/testsuite.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use codespan_reporting::{diagnostic::Severity, term::termcolor::Buffer}; -use move_compiler_v2::{run_move_compiler, Experiment}; +use move_compiler_v2::{diagnostics::human::HumanEmitter, run_move_compiler, Experiment}; use move_linter::MoveLintChecks; use move_model::metadata::{CompilerVersion, LanguageVersion}; use move_prover_test_utils::baseline_test; @@ -30,7 +30,8 @@ fn test_runner(path: &Path) -> datatest_stable::Result<()> { }; let mut output = String::new(); let mut error_writer = Buffer::no_color(); - match run_move_compiler(&mut error_writer, compiler_options) { + let mut emitter = HumanEmitter::new(&mut error_writer); + match run_move_compiler(&mut emitter, compiler_options) { Err(e) => { output.push_str(&format!( "Aborting with compilation errors:\n{:#}\n{}\n", diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index 15f5103b9feed..dd04c8da87e3b 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -42,7 +42,6 @@ use std::{ sync::Arc, }; use termcolor::{ColorChoice, StandardStream}; -use move_compiler_v2::diagnostics::human::HumanEmitter; #[derive(Debug, Clone)] pub enum CompilationCachingStatus { @@ -1140,7 +1139,7 @@ pub fn unimplemented_v2_driver(_options: move_compiler_v2::Options) -> CompilerD pub fn build_and_report_v2_driver(options: move_compiler_v2::Options) -> CompilerDriverResult { let mut stderr = StandardStream::stderr(ColorChoice::Auto); let mut emitter = options.to_emitter(&mut stderr); - match move_compiler_v2::run_move_compiler_with_emitter(&mut emitter, options) { + match move_compiler_v2::run_move_compiler(emitter.as_mut(), options) { Ok((env, units)) => Ok(( move_compiler_v2::make_files_source_text(&env), units, @@ -1159,7 +1158,7 @@ pub fn build_and_report_no_exit_v2_driver( ) -> CompilerDriverResult { let mut stderr = StandardStream::stderr(ColorChoice::Auto); let mut emitter = options.to_emitter(&mut stderr); - let (env, units) = move_compiler_v2::run_move_compiler_with_emitter(emitter.as_mut(), options)?; + let (env, units) = move_compiler_v2::run_move_compiler(emitter.as_mut(), options)?; Ok(( move_compiler_v2::make_files_source_text(&env), units, From d871047d51cbc02e3e3b7cbf8e05cdff8b3ddcde Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Thu, 19 Dec 2024 21:24:27 +0100 Subject: [PATCH 10/15] convert file_id into fpath in the json diagnostic --- .../move-compiler-v2/src/diagnostics/json.rs | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/third_party/move/move-compiler-v2/src/diagnostics/json.rs b/third_party/move/move-compiler-v2/src/diagnostics/json.rs index 1816fcab7e5bf..fdb6e0bb028bf 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/json.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/json.rs @@ -1,6 +1,6 @@ use crate::diagnostics::Emitter; use codespan::{FileId, Files}; -use codespan_reporting::diagnostic::Diagnostic; +use codespan_reporting::diagnostic::{Diagnostic, Label}; use std::io::Write; pub struct JsonEmitter<'w, W: Write> { @@ -14,8 +14,25 @@ impl<'w, W: Write> JsonEmitter<'w, W> { } impl<'w, W: Write> Emitter for JsonEmitter<'w, W> { - fn emit(&mut self, _source_files: &Files, diag: &Diagnostic) { - serde_json::to_writer(&mut self.writer, diag).expect("emit must not fail"); + fn emit(&mut self, source_files: &Files, diag: &Diagnostic) { + 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(); } } From 13d863abab0ebceb326790e2e6c91a9e52c59d88 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Sat, 21 Dec 2024 11:57:29 +0100 Subject: [PATCH 11/15] replace unwrap() with expect() --- third_party/move/move-compiler-v2/src/diagnostics/json.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/third_party/move/move-compiler-v2/src/diagnostics/json.rs b/third_party/move/move-compiler-v2/src/diagnostics/json.rs index fdb6e0bb028bf..b4af7135ed714 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/json.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/json.rs @@ -20,7 +20,7 @@ impl<'w, W: Write> Emitter for JsonEmitter<'w, W> { .iter() .map(|label| { let fpath = codespan_reporting::files::Files::name(source_files, label.file_id) - .expect("emit must not fail") + .expect("always Ok() in the impl") .to_string(); Label::new(label.style, fpath, label.range.clone()) }) @@ -32,7 +32,8 @@ impl<'w, W: Write> Emitter for JsonEmitter<'w, W> { 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(); + serde_json::to_writer(&mut self.writer, &json_diag).expect("it should be serializable"); + writeln!(&mut self.writer) + .expect("dest is stderr / in-memory buffer, it should always be available"); } } From 0881eefb5e5d2a2c9e4f0bc9d89049bd2fbb39b2 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Sat, 21 Dec 2024 12:10:53 +0100 Subject: [PATCH 12/15] get emitter from options in more places --- .../move/move-compiler-v2/src/diagnostics/mod.rs | 11 +++++++---- third_party/move/move-compiler-v2/src/lib.rs | 10 +++++----- .../transactional-test-runner/src/framework.rs | 7 ++++--- .../move-package/src/compilation/compiled_package.rs | 4 ++-- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs index 8114dbdabf345..3899e57e774e2 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs @@ -6,7 +6,7 @@ use anyhow::bail; use codespan::{FileId, Files}; use codespan_reporting::{ diagnostic::{Diagnostic, Severity}, - term::termcolor::StandardStream, + term::termcolor::WriteColor, }; use move_model::model::GlobalEnv; @@ -14,11 +14,14 @@ pub mod human; pub mod json; impl options::Options { - pub fn to_emitter<'w>(&self, stderr: &'w mut StandardStream) -> Box { + pub fn message_emitter<'w, W>(&self, dest: &'w mut W) -> Box + where + W: WriteColor, + { if self.experiment_on(Experiment::MESSAGE_FORMAT_JSON) { - Box::new(JsonEmitter::new(stderr)) + Box::new(JsonEmitter::new(dest)) } else { - Box::new(HumanEmitter::new(stderr)) + Box::new(HumanEmitter::new(dest)) } } } diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index 97c6610a10595..133e5785890e9 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -15,7 +15,7 @@ pub mod pipeline; pub mod plan_builder; use crate::{ - diagnostics::{human::HumanEmitter, Emitter}, + diagnostics::Emitter, env_pipeline::{ acquires_checker, ast_simplifier, cyclic_instantiation_checker, flow_insensitive_checkers, function_checker, inliner, lambda_lifter, lambda_lifter::LambdaLiftingOptions, @@ -80,8 +80,8 @@ pub fn run_move_compiler_to_stderr( options: Options, ) -> anyhow::Result<(GlobalEnv, Vec)> { let mut stderr = StandardStream::stderr(ColorChoice::Auto); - let mut emitter = HumanEmitter::new(&mut stderr); - run_move_compiler(&mut emitter, options) + let mut emitter = options.message_emitter(&mut stderr); + run_move_compiler(emitter.as_mut(), options) } /// Run move compiler and print errors to given writer. Returns the set of compiled units. @@ -167,8 +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 mut emitter = HumanEmitter::new(error_writer); - let (env, _units) = run_move_compiler(&mut emitter, options)?; + let mut emitter = options.message_emitter(error_writer); + let (env, _units) = run_move_compiler(emitter.as_mut(), options)?; // Reset for subsequent analysis env.treat_everything_as_target(false); Ok(env) diff --git a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs index a13770c87eca8..36ec758254a7f 100644 --- a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs +++ b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs @@ -37,7 +37,6 @@ use move_compiler::{ }, FullyCompiledProgram, }; -use move_compiler_v2::diagnostics::human::HumanEmitter; use move_core_types::{ account_address::AccountAddress, identifier::{IdentStr, Identifier}, @@ -825,8 +824,10 @@ fn compile_source_unit_v2( options = options.set_experiment(exp, value) } let mut error_writer = termcolor::Buffer::no_color(); - let mut emitter = HumanEmitter::new(&mut error_writer); - let result = move_compiler_v2::run_move_compiler(&mut emitter, options); + let result = { + let mut emitter = options.message_emitter(&mut error_writer); + move_compiler_v2::run_move_compiler(emitter.as_mut(), 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))?; diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index dd04c8da87e3b..3ef1c66c92f17 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -1138,7 +1138,7 @@ pub fn unimplemented_v2_driver(_options: move_compiler_v2::Options) -> CompilerD /// Runs the v2 compiler, exiting the process if any errors occurred. pub fn build_and_report_v2_driver(options: move_compiler_v2::Options) -> CompilerDriverResult { let mut stderr = StandardStream::stderr(ColorChoice::Auto); - let mut emitter = options.to_emitter(&mut stderr); + let mut emitter = options.message_emitter(&mut stderr); match move_compiler_v2::run_move_compiler(emitter.as_mut(), options) { Ok((env, units)) => Ok(( move_compiler_v2::make_files_source_text(&env), @@ -1157,7 +1157,7 @@ pub fn build_and_report_no_exit_v2_driver( options: move_compiler_v2::Options, ) -> CompilerDriverResult { let mut stderr = StandardStream::stderr(ColorChoice::Auto); - let mut emitter = options.to_emitter(&mut stderr); + let mut emitter = options.message_emitter(&mut stderr); let (env, units) = move_compiler_v2::run_move_compiler(emitter.as_mut(), options)?; Ok(( move_compiler_v2::make_files_source_text(&env), From 34c49bcb62eb8e9c46fe43c0f817774b37934989 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Sat, 21 Dec 2024 14:42:05 +0100 Subject: [PATCH 13/15] generate tests for json emitter --- .../move-compiler-v2/src/diagnostics/mod.rs | 2 +- third_party/move/move-compiler-v2/src/lib.rs | 4 +- .../compiler-message-format-json/errors.exp | 4 ++ .../compiler-message-format-json/errors.move | 6 +++ .../compiler-message-format-json/warnings.exp | 4 ++ .../warnings.move | 6 +++ .../move/move-compiler-v2/tests/testsuite.rs | 43 +++++++++++++------ .../src/framework.rs | 2 +- .../src/compilation/compiled_package.rs | 4 +- 9 files changed, 56 insertions(+), 19 deletions(-) create mode 100644 third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp create mode 100644 third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.move create mode 100644 third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp create mode 100644 third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.move diff --git a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs index 3899e57e774e2..1c03b2552345a 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/mod.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/mod.rs @@ -14,7 +14,7 @@ pub mod human; pub mod json; impl options::Options { - pub fn message_emitter<'w, W>(&self, dest: &'w mut W) -> Box + pub fn error_emitter<'w, W>(&self, dest: &'w mut W) -> Box where W: WriteColor, { diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index 133e5785890e9..7f5c6c4139c43 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -80,7 +80,7 @@ pub fn run_move_compiler_to_stderr( options: Options, ) -> anyhow::Result<(GlobalEnv, Vec)> { let mut stderr = StandardStream::stderr(ColorChoice::Auto); - let mut emitter = options.message_emitter(&mut stderr); + let mut emitter = options.error_emitter(&mut stderr); run_move_compiler(emitter.as_mut(), options) } @@ -167,7 +167,7 @@ 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 mut emitter = options.message_emitter(error_writer); + let mut emitter = options.error_emitter(error_writer); let (env, _units) = run_move_compiler(emitter.as_mut(), options)?; // Reset for subsequent analysis env.treat_everything_as_target(false); diff --git a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp new file mode 100644 index 0000000000000..1e56f0f54c734 --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp @@ -0,0 +1,4 @@ + +Diagnostics: +{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/errors.move","range":{"start":51,"end":55},"message":""}],"notes":[]} +{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/errors.move","range":{"start":69,"end":73},"message":""}],"notes":[]} diff --git a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.move b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.move new file mode 100644 index 0000000000000..4038807912f3a --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.move @@ -0,0 +1,6 @@ +module 0x42::errors { + fun main() { + 1 + true; + 2 + true; + } +} diff --git a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp new file mode 100644 index 0000000000000..89b8ff91cf49d --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp @@ -0,0 +1,4 @@ + +Diagnostics: +{"severity":"Warning","code":null,"message":"Unused local variable `a`. Consider removing or prefixing with an underscore: `_a`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/warnings.move","range":{"start":53,"end":54},"message":""}],"notes":[]} +{"severity":"Warning","code":null,"message":"Unused local variable `b`. Consider removing or prefixing with an underscore: `_b`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/warnings.move","range":{"start":72,"end":73},"message":""}],"notes":[]} diff --git a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.move b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.move new file mode 100644 index 0000000000000..f9e857c702fe7 --- /dev/null +++ b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.move @@ -0,0 +1,6 @@ +module 0x42::warnings { + fun main() { + let a = 1; + let b = 2; + } +} diff --git a/third_party/move/move-compiler-v2/tests/testsuite.rs b/third_party/move/move-compiler-v2/tests/testsuite.rs index a5e6ef2322d8d..9233703a0986b 100644 --- a/third_party/move/move-compiler-v2/tests/testsuite.rs +++ b/third_party/move/move-compiler-v2/tests/testsuite.rs @@ -737,6 +737,20 @@ const TEST_CONFIGS: Lazy> = Lazy::new(|| { dump_bytecode: DumpLevel::EndStage, dump_bytecode_filter: None, }, + TestConfig { + name: "compiler-message-format-json", + runner: |p| run_test(p, get_config_by_name("compiler-message-format-json")), + include: vec!["/compiler-message-format-json/"], + exclude: vec![], + exp_suffix: None, + options: opts + .clone() + .set_experiment(Experiment::MESSAGE_FORMAT_JSON, true), + stop_after: StopAfter::AstPipeline, + dump_ast: DumpLevel::None, + dump_bytecode: DumpLevel::None, + dump_bytecode_filter: None, + }, ]; configs.into_iter().map(|c| (c.name, c)).collect() }); @@ -780,7 +794,7 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> { // Run context checker let mut env = move_compiler_v2::run_checker(options.clone())?; - let mut ok = check_diags(&mut test_output.borrow_mut(), &env); + let mut ok = check_diags(&mut test_output.borrow_mut(), &env, &options); if ok { // Run env processor pipeline. @@ -796,10 +810,10 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> { test_output .borrow_mut() .push_str(&String::from_utf8_lossy(&out.into_inner())); - ok = check_diags(&mut test_output.borrow_mut(), &env); + ok = check_diags(&mut test_output.borrow_mut(), &env, &options); } else { env_pipeline.run(&mut env); - ok = check_diags(&mut test_output.borrow_mut(), &env); + ok = check_diags(&mut test_output.borrow_mut(), &env, &options); if ok && config.dump_ast == DumpLevel::EndStage { test_output.borrow_mut().push_str(&format!( "// -- Model dump before bytecode pipeline\n{}\n", @@ -824,13 +838,13 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> { // In real use, this is run outside of the compilation process, but the needed info is // available in `env` once we finish the AST. plan_builder::construct_test_plan(&env, None); - ok = check_diags(&mut test_output.borrow_mut(), &env); + ok = check_diags(&mut test_output.borrow_mut(), &env, &options); } if ok && config.stop_after > StopAfter::AstPipeline { // Run stackless bytecode generator let mut targets = move_compiler_v2::run_bytecode_gen(&env); - ok = check_diags(&mut test_output.borrow_mut(), &env); + ok = check_diags(&mut test_output.borrow_mut(), &env, &options); if ok { // Run the target pipeline. let bytecode_pipeline = if config.stop_after == StopAfter::BytecodeGen { @@ -852,7 +866,7 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> { // bytecode from the generator, if requested. |targets_before| { let out = &mut test_output.borrow_mut(); - update_diags(ok.borrow_mut(), out, &env); + update_diags(ok.borrow_mut(), out, &env, &options); if bytecode_dump_enabled(&config, true, INITIAL_BYTECODE_STAGE) { let dump = &move_stackless_bytecode::print_targets_with_annotations_for_test( @@ -870,7 +884,7 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> { // bytecode after the processor, if requested. |i, processor, targets_after| { let out = &mut test_output.borrow_mut(); - update_diags(ok.borrow_mut(), out, &env); + update_diags(ok.borrow_mut(), out, &env, &options); if bytecode_dump_enabled(&config, i + 1 == count, processor.name().as_str()) { let title = format!("after {}:", processor.name()); let dump = @@ -890,7 +904,7 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> { if *ok.borrow() && config.stop_after == StopAfter::FileFormat { let units = run_file_format_gen(&mut env, &targets); let out = &mut test_output.borrow_mut(); - update_diags(ok.borrow_mut(), out, &env); + update_diags(ok.borrow_mut(), out, &env, &options); if *ok.borrow() { if bytecode_dump_enabled(&config, true, FILE_FORMAT_STAGE) { out.push_str( @@ -904,7 +918,7 @@ fn run_test(path: &Path, config: TestConfig) -> datatest_stable::Result<()> { } else { out.push_str("\n============ bytecode verification failed ========\n"); } - check_diags(out, &env); + check_diags(out, &env, &options); } } } @@ -929,9 +943,12 @@ fn bytecode_dump_enabled(config: &TestConfig, is_last: bool, name: &str) -> bool } /// Checks for diagnostics and adds them to the baseline. -fn check_diags(baseline: &mut String, env: &GlobalEnv) -> bool { +fn check_diags(baseline: &mut String, env: &GlobalEnv, options: &Options) -> bool { let mut error_writer = Buffer::no_color(); - env.report_diag(&mut error_writer, Severity::Note); + { + let mut emitter = options.error_emitter(&mut error_writer); + emitter.report_diag(env, Severity::Note); + } let diag = String::from_utf8_lossy(&error_writer.into_inner()).to_string(); if !diag.is_empty() { *baseline += &format!("\nDiagnostics:\n{}", diag); @@ -941,8 +958,8 @@ fn check_diags(baseline: &mut String, env: &GlobalEnv) -> bool { ok } -fn update_diags(mut ok: RefMut, baseline: &mut String, env: &GlobalEnv) { - if !check_diags(baseline, env) { +fn update_diags(mut ok: RefMut, baseline: &mut String, env: &GlobalEnv, options: &Options) { + if !check_diags(baseline, env, options) { *ok = false; } } diff --git a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs index 36ec758254a7f..5ca8a4b0763e8 100644 --- a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs +++ b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs @@ -825,7 +825,7 @@ fn compile_source_unit_v2( } let mut error_writer = termcolor::Buffer::no_color(); let result = { - let mut emitter = options.message_emitter(&mut error_writer); + let mut emitter = options.error_emitter(&mut error_writer); move_compiler_v2::run_move_compiler(emitter.as_mut(), options) }; let error_str = String::from_utf8_lossy(&error_writer.into_inner()).to_string(); diff --git a/third_party/move/tools/move-package/src/compilation/compiled_package.rs b/third_party/move/tools/move-package/src/compilation/compiled_package.rs index 3ef1c66c92f17..3ed670dc3ddb1 100644 --- a/third_party/move/tools/move-package/src/compilation/compiled_package.rs +++ b/third_party/move/tools/move-package/src/compilation/compiled_package.rs @@ -1138,7 +1138,7 @@ pub fn unimplemented_v2_driver(_options: move_compiler_v2::Options) -> CompilerD /// Runs the v2 compiler, exiting the process if any errors occurred. pub fn build_and_report_v2_driver(options: move_compiler_v2::Options) -> CompilerDriverResult { let mut stderr = StandardStream::stderr(ColorChoice::Auto); - let mut emitter = options.message_emitter(&mut stderr); + let mut emitter = options.error_emitter(&mut stderr); match move_compiler_v2::run_move_compiler(emitter.as_mut(), options) { Ok((env, units)) => Ok(( move_compiler_v2::make_files_source_text(&env), @@ -1157,7 +1157,7 @@ pub fn build_and_report_no_exit_v2_driver( options: move_compiler_v2::Options, ) -> CompilerDriverResult { let mut stderr = StandardStream::stderr(ColorChoice::Auto); - let mut emitter = options.message_emitter(&mut stderr); + let mut emitter = options.error_emitter(&mut stderr); let (env, units) = move_compiler_v2::run_move_compiler(emitter.as_mut(), options)?; Ok(( move_compiler_v2::make_files_source_text(&env), From 2f2a0c7833e784b4044d3e8121776b13bdb11660 Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Sat, 21 Dec 2024 14:50:15 +0100 Subject: [PATCH 14/15] add small docstring to the emitters --- third_party/move/move-compiler-v2/src/diagnostics/human.rs | 2 ++ third_party/move/move-compiler-v2/src/diagnostics/json.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/third_party/move/move-compiler-v2/src/diagnostics/human.rs b/third_party/move/move-compiler-v2/src/diagnostics/human.rs index aa4770e68746a..b5e09cc722499 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/human.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/human.rs @@ -5,6 +5,8 @@ use codespan_reporting::{ term::{emit, termcolor::WriteColor, Config}, }; +/// It's used in the native aptos-cli output to show error messages. +/// Wraps the `codespan_reporting::term::emit()` method. pub struct HumanEmitter<'w, W: WriteColor> { writer: &'w mut W, } diff --git a/third_party/move/move-compiler-v2/src/diagnostics/json.rs b/third_party/move/move-compiler-v2/src/diagnostics/json.rs index b4af7135ed714..9848ff63a1974 100644 --- a/third_party/move/move-compiler-v2/src/diagnostics/json.rs +++ b/third_party/move/move-compiler-v2/src/diagnostics/json.rs @@ -3,6 +3,8 @@ use codespan::{FileId, Files}; use codespan_reporting::diagnostic::{Diagnostic, Label}; use std::io::Write; +/// Shows compiler errors as a structured JSON output. +/// Exists to support various tools external to the aptos-cli, i.e. IDEs. pub struct JsonEmitter<'w, W: Write> { writer: &'w mut W, } From 8311f0073a00cc450487ba2117c169f2e84b993b Mon Sep 17 00:00:00 2001 From: Maksim Kurnikov Date: Sat, 21 Dec 2024 15:06:18 +0100 Subject: [PATCH 15/15] fix json output paths --- .../tests/compiler-message-format-json/errors.exp | 4 ++-- .../tests/compiler-message-format-json/warnings.exp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp index 1e56f0f54c734..f64b61007a78d 100644 --- a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp +++ b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/errors.exp @@ -1,4 +1,4 @@ Diagnostics: -{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/errors.move","range":{"start":51,"end":55},"message":""}],"notes":[]} -{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/errors.move","range":{"start":69,"end":73},"message":""}],"notes":[]} +{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/errors.move","range":{"start":51,"end":55},"message":""}],"notes":[]} +{"severity":"Error","code":null,"message":"cannot use `bool` with an operator which expects a value of type `integer`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/errors.move","range":{"start":69,"end":73},"message":""}],"notes":[]} diff --git a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp index 89b8ff91cf49d..7950ff370d030 100644 --- a/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp +++ b/third_party/move/move-compiler-v2/tests/compiler-message-format-json/warnings.exp @@ -1,4 +1,4 @@ Diagnostics: -{"severity":"Warning","code":null,"message":"Unused local variable `a`. Consider removing or prefixing with an underscore: `_a`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/warnings.move","range":{"start":53,"end":54},"message":""}],"notes":[]} -{"severity":"Warning","code":null,"message":"Unused local variable `b`. Consider removing or prefixing with an underscore: `_b`","labels":[{"style":"Primary","file_id":"tests/error-message-format/json/warnings.move","range":{"start":72,"end":73},"message":""}],"notes":[]} +{"severity":"Warning","code":null,"message":"Unused local variable `a`. Consider removing or prefixing with an underscore: `_a`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/warnings.move","range":{"start":53,"end":54},"message":""}],"notes":[]} +{"severity":"Warning","code":null,"message":"Unused local variable `b`. Consider removing or prefixing with an underscore: `_b`","labels":[{"style":"Primary","file_id":"tests/compiler-message-format-json/warnings.move","range":{"start":72,"end":73},"message":""}],"notes":[]}