From 7b5ffed288692bd967f73a2af481627538c336da Mon Sep 17 00:00:00 2001 From: James Wilson Date: Fri, 8 Aug 2025 12:40:41 +0100 Subject: [PATCH] Address nits --- Cargo.lock | 1 + crates/compiler/Cargo.toml | 1 + crates/compiler/src/lib.rs | 11 ++++-- crates/compiler/src/revive_resolc.rs | 8 ++-- crates/compiler/src/solc.rs | 4 +- crates/core/src/main.rs | 6 +-- crates/format/src/mode.rs | 55 +++++++++++++++------------- crates/report/src/reporter.rs | 10 ++--- 8 files changed, 53 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7f8e5b3..3daf8c0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4044,6 +4044,7 @@ dependencies = [ "revive-common", "revive-dt-common", "revive-dt-config", + "revive-dt-format", "revive-dt-solc-binaries", "revive-solc-json-interface", "semver 1.0.26", diff --git a/crates/compiler/Cargo.toml b/crates/compiler/Cargo.toml index 9e10a10..ee69f39 100644 --- a/crates/compiler/Cargo.toml +++ b/crates/compiler/Cargo.toml @@ -12,6 +12,7 @@ rust-version.workspace = true revive-solc-json-interface = { workspace = true } revive-dt-common = { workspace = true } revive-dt-config = { workspace = true } +revive-dt-format = { workspace = true } revive-dt-solc-binaries = { workspace = true } revive-common = { workspace = true } diff --git a/crates/compiler/src/lib.rs b/crates/compiler/src/lib.rs index 551831e..4a6835c 100644 --- a/crates/compiler/src/lib.rs +++ b/crates/compiler/src/lib.rs @@ -19,6 +19,9 @@ use revive_common::EVMVersion; use revive_dt_common::types::VersionOrRequirement; use revive_dt_config::Arguments; +// Re-export this as it's a part of the compiler interface. +pub use revive_dt_format::mode::ModeOptimizerSetting; + pub mod revive_js; pub mod revive_resolc; pub mod solc; @@ -48,7 +51,7 @@ pub trait SolidityCompiler { /// The generic compilation input configuration. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CompilerInput { - pub enable_optimization: Option, + pub optimization: Option, pub via_ir: Option, pub evm_version: Option, pub allow_paths: Vec, @@ -84,7 +87,7 @@ where pub fn new() -> Self { Self { input: CompilerInput { - enable_optimization: Default::default(), + optimization: Default::default(), via_ir: Default::default(), evm_version: Default::default(), allow_paths: Default::default(), @@ -96,8 +99,8 @@ where } } - pub fn with_optimization(mut self, value: impl Into>) -> Self { - self.input.enable_optimization = value.into(); + pub fn with_optimization(mut self, value: impl Into>) -> Self { + self.input.optimization = value.into(); self } diff --git a/crates/compiler/src/revive_resolc.rs b/crates/compiler/src/revive_resolc.rs index 0771211..d5abf71 100644 --- a/crates/compiler/src/revive_resolc.rs +++ b/crates/compiler/src/revive_resolc.rs @@ -14,7 +14,7 @@ use revive_solc_json_interface::{ SolcStandardJsonOutput, }; -use crate::{CompilerInput, CompilerOutput, SolidityCompiler}; +use crate::{CompilerInput, CompilerOutput, ModeOptimizerSetting, SolidityCompiler}; use alloy::json_abi::JsonAbi; use anyhow::Context; @@ -39,7 +39,7 @@ impl SolidityCompiler for Resolc { async fn build( &self, CompilerInput { - enable_optimization, + optimization, // Ignored and not honored since this is required for the resolc compilation. via_ir: _via_ir, evm_version, @@ -78,7 +78,9 @@ impl SolidityCompiler for Resolc { output_selection: Some(SolcStandardJsonInputSettingsSelection::new_required()), via_ir: Some(true), optimizer: SolcStandardJsonInputSettingsOptimizer::new( - enable_optimization.unwrap_or(false), + optimization + .unwrap_or(ModeOptimizerSetting::M0) + .optimizations_enabled(), None, &Version::new(0, 0, 0), false, diff --git a/crates/compiler/src/solc.rs b/crates/compiler/src/solc.rs index a07cdb9..4312bee 100644 --- a/crates/compiler/src/solc.rs +++ b/crates/compiler/src/solc.rs @@ -35,7 +35,7 @@ impl SolidityCompiler for Solc { async fn build( &self, CompilerInput { - enable_optimization, + optimization, via_ir, evm_version, allow_paths, @@ -55,7 +55,7 @@ impl SolidityCompiler for Solc { ), settings: Settings { optimizer: Optimizer { - enabled: enable_optimization, + enabled: optimization.map(|o| o.optimizations_enabled()), details: Some(Default::default()), ..Default::default() }, diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index d28e3c8..50daf01 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -34,7 +34,7 @@ use revive_dt_format::{ corpus::Corpus, input::Input, metadata::{ContractInstance, ContractPathAndIdent, Metadata, MetadataFile}, - mode::{Mode, ModeOptimizerSetting, ModePipeline}, + mode::Mode, }; use revive_dt_node::pool::NodePool; use revive_dt_report::reporter::{Report, Span}; @@ -661,8 +661,8 @@ async fn compile_contracts( let compiler = Compiler::::new() .with_allow_path(metadata.directory()?) - .with_optimization(mode.optimize_setting != ModeOptimizerSetting::M0) - .with_via_ir(mode.pipeline == ModePipeline::Y); + .with_optimization(mode.optimize_setting) + .with_via_ir(mode.via_yul_ir()); let mut compiler = metadata .files_to_compile()? .try_fold(compiler, |compiler, path| compiler.with_source(&path))?; diff --git a/crates/format/src/mode.rs b/crates/format/src/mode.rs index 9bf3434..91a8062 100644 --- a/crates/format/src/mode.rs +++ b/crates/format/src/mode.rs @@ -1,7 +1,6 @@ use regex::Regex; use revive_dt_common::types::VersionOrRequirement; use semver::Version; -use serde::de::Deserializer; use serde::{Deserialize, Serialize}; use std::collections::HashSet; use std::fmt::Display; @@ -11,10 +10,10 @@ use std::sync::LazyLock; /// This represents a mode that a given test should be run with, if possible. /// /// We obtain this by taking a [`ParsedMode`], which may be looser or more strict -/// in its requirements, and then expanding it out into a list of [`TestMode`]s. +/// in its requirements, and then expanding it out into a list of [`Mode`]s. /// /// Use [`ParsedMode::to_test_modes()`] to do this. -#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize)] pub struct Mode { pub pipeline: ModePipeline, pub optimize_setting: ModeOptimizerSetting, @@ -62,6 +61,11 @@ impl Mode { None => default.into(), } } + + /// Should we go via Yul IR? + pub fn via_yul_ir(&self) -> bool { + self.pipeline == ModePipeline::Y + } } /// This represents a mode that has been parsed from test metadata. @@ -73,7 +77,8 @@ impl Mode { /// ``` /// /// We can parse valid mode strings into [`ParsedMode`] using [`ParsedMode::from_str`]. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, Serialize)] +#[serde(try_from = "String", into = "String")] pub struct ParsedMode { pub pipeline: Option, pub optimize_flag: Option, @@ -129,26 +134,6 @@ impl FromStr for ParsedMode { } } -impl<'de> Deserialize<'de> for ParsedMode { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let mode_string = String::deserialize(deserializer)?; - ParsedMode::from_str(&mode_string).map_err(serde::de::Error::custom) - } -} - -impl Serialize for ParsedMode { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - let mode_string = self.to_string(); - serializer.serialize_str(&mode_string) - } -} - impl Display for ParsedMode { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fmt_mode_parts( @@ -161,6 +146,19 @@ impl Display for ParsedMode { } } +impl From for String { + fn from(parsed_mode: ParsedMode) -> Self { + parsed_mode.to_string() + } +} + +impl TryFrom for ParsedMode { + type Error = ParseModeError; + fn try_from(value: String) -> Result { + ParsedMode::from_str(&value) + } +} + fn fmt_mode_parts( pipeline: Option<&ModePipeline>, optimize_flag: Option, @@ -204,8 +202,8 @@ impl ParsedMode { |p| EitherIter::B(std::iter::once(*p)), ); - let optimize_flag_setting = self.optimize_flag.map(|b| { - if b { + let optimize_flag_setting = self.optimize_flag.map(|flag| { + if flag { ModeOptimizerSetting::M3 } else { ModeOptimizerSetting::M0 @@ -346,6 +344,11 @@ impl ModeOptimizerSetting { ] .into_iter() } + + /// Are any optimizations enabled? + pub fn optimizations_enabled(&self) -> bool { + !matches!(self, ModeOptimizerSetting::M0) + } } /// An iterator that could be either of two iterators. diff --git a/crates/report/src/reporter.rs b/crates/report/src/reporter.rs index ef5a677..3a49254 100644 --- a/crates/report/src/reporter.rs +++ b/crates/report/src/reporter.rs @@ -13,7 +13,7 @@ use std::{ use anyhow::Context; use revive_dt_compiler::{CompilerInput, CompilerOutput}; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use revive_dt_config::{Arguments, TestingPlatform}; use revive_dt_format::{corpus::Corpus, mode::Mode}; @@ -23,7 +23,7 @@ use crate::analyzer::CompilerStatistics; pub(crate) static REPORTER: OnceLock> = OnceLock::new(); /// The `Report` datastructure stores all relevant inforamtion required for generating reports. -#[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize)] pub struct Report { /// The configuration used during the test. pub config: Arguments, @@ -41,7 +41,7 @@ pub struct Report { } /// Contains a compiled contract. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize)] pub struct CompilationTask { /// The observed compiler input. pub json_input: CompilerInput, @@ -56,7 +56,7 @@ pub struct CompilationTask { } /// Represents a report about a compilation task. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize)] pub struct CompilationResult { /// The observed compilation task. pub compilation_task: CompilationTask, @@ -65,7 +65,7 @@ pub struct CompilationResult { } /// The [Span] struct indicates the context of what is being reported. -#[derive(Clone, Copy, Debug, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Serialize)] pub struct Span { /// The corpus index this belongs to. corpus: usize,