Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: don't request Solc JSON AST unless absolutely necessary #7197

Merged
merged 6 commits into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/cli/src/opts/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ pub use self::paths::ProjectPathsArgs;
#[derive(Clone, Debug, Default, Serialize, Parser)]
#[clap(next_help_heading = "Compiler options")]
pub struct CompilerArgs {
/// Includes the AST as JSON in the compiler output.
#[clap(long, help_heading = "Compiler options")]
pub ast: bool,

/// The target EVM version.
#[clap(long, value_name = "VERSION")]
#[serde(skip_serializing_if = "Option::is_none")]
Expand Down
1 change: 1 addition & 0 deletions crates/config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ extra_output_files = []
names = false
sizes = false
via_ir = false
ast = false
# caches storage retrieved locally for certain chains and endpoints
# can also be restricted to `chains = ["optimism", "mainnet"]`
# by default all endpoints will be cached, alternative options are "remote" for only caching non localhost endpoints and "<regex>"
Expand Down
13 changes: 11 additions & 2 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ pub struct Config {
/// If set to true, changes compilation pipeline to go through the Yul intermediate
/// representation.
pub via_ir: bool,
/// Whether to include the AST as JSON in the compiler output.
pub ast: bool,
/// RPC storage caching settings determines what chains and endpoints to cache
pub rpc_storage_caching: StorageCachingConfig,
/// Disables storage caching entirely. This overrides any settings made in
Expand Down Expand Up @@ -1064,7 +1066,7 @@ impl Config {
}
}

Ok(Settings {
let mut settings = Settings {
libraries: self.libraries_with_remappings()?,
optimizer: self.optimizer(),
evm_version: Some(self.evm_version),
Expand All @@ -1087,7 +1089,13 @@ impl Config {
// Set with `with_extra_output` below.
output_selection: Default::default(),
}
.with_extra_output(self.configured_artifacts_handler().output_selection()))
.with_extra_output(self.configured_artifacts_handler().output_selection());

if self.ast {
settings = settings.with_ast();
}

Ok(settings)
}

/// Returns the default figment
Expand Down Expand Up @@ -1877,6 +1885,7 @@ impl Default for Config {
ignored_file_paths: vec![],
deny_warnings: false,
via_ir: false,
ast: false,
rpc_storage_caching: Default::default(),
rpc_endpoints: Default::default(),
etherscan: Default::default(),
Expand Down
73 changes: 34 additions & 39 deletions crates/forge/bin/cmd/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ impl CoverageArgs {
// Set fuzz seed so coverage reports are deterministic
config.fuzz.seed = Some(U256::from_be_bytes(STATIC_FUZZ_SEED));

// Coverage analysis requires the Solc AST output.
config.ast = true;

let (project, output) = self.build(&config)?;
p_println!(!self.opts.silent => "Analysing contracts...");
let report = self.prepare(&config, output.clone())?;
Expand All @@ -99,50 +102,42 @@ impl CoverageArgs {
/// Builds the project.
fn build(&self, config: &Config) -> Result<(Project, ProjectCompileOutput)> {
// Set up the project
let project = {
let mut project = config.ephemeral_no_artifacts_project()?;

if self.ir_minimum {
// TODO: How to detect solc version if the user does not specify a solc version in
// config case1: specify local installed solc ?
// case2: multiple solc versions used and auto_detect_solc == true
if let Some(SolcReq::Version(version)) = &config.solc {
if *version < Version::new(0, 8, 13) {
return Err(eyre::eyre!(
let mut project = config.ephemeral_no_artifacts_project()?;
if self.ir_minimum {
// TODO: How to detect solc version if the user does not specify a solc version in
// config case1: specify local installed solc ?
// case2: multiple solc versions used and auto_detect_solc == true
if let Some(SolcReq::Version(version)) = &config.solc {
if *version < Version::new(0, 8, 13) {
return Err(eyre::eyre!(
"viaIR with minimum optimization is only available in Solidity 0.8.13 and above."
));
}
}

// print warning message
p_println!(!self.opts.silent => "{}",
Paint::yellow(
concat!(
"Warning! \"--ir-minimum\" flag enables viaIR with minimum optimization, which can result in inaccurate source mappings.\n",
"Only use this flag as a workaround if you are experiencing \"stack too deep\" errors.\n",
"Note that \"viaIR\" is only available in Solidity 0.8.13 and above.\n",
"See more:\n",
"https://github.com/foundry-rs/foundry/issues/3357\n"
)));

// Enable viaIR with minimum optimization
// https://github.com/ethereum/solidity/issues/12533#issuecomment-1013073350
// And also in new releases of solidity:
// https://github.com/ethereum/solidity/issues/13972#issuecomment-1628632202
project.solc_config.settings =
project.solc_config.settings.with_via_ir_minimum_optimization()
} else {
project.solc_config.settings.optimizer.disable();
project.solc_config.settings.optimizer.runs = None;
project.solc_config.settings.optimizer.details = None;
project.solc_config.settings.via_ir = None;
}

// Coverage analysis requires the Solc AST output.
project.solc_config.settings = project.solc_config.settings.with_ast();

project
};
// print warning message
p_println!(!self.opts.silent => "{}",
Paint::yellow(
concat!(
"Warning! \"--ir-minimum\" flag enables viaIR with minimum optimization, which can result in inaccurate source mappings.\n",
"Only use this flag as a workaround if you are experiencing \"stack too deep\" errors.\n",
"Note that \"viaIR\" is only available in Solidity 0.8.13 and above.\n",
"See more:\n",
"https://github.com/foundry-rs/foundry/issues/3357\n"
DaniPopes marked this conversation as resolved.
Show resolved Hide resolved
)));

// Enable viaIR with minimum optimization
// https://github.com/ethereum/solidity/issues/12533#issuecomment-1013073350
// And also in new releases of solidity:
// https://github.com/ethereum/solidity/issues/13972#issuecomment-1628632202
project.solc_config.settings =
project.solc_config.settings.with_via_ir_minimum_optimization()
} else {
project.solc_config.settings.optimizer.disable();
project.solc_config.settings.optimizer.runs = None;
project.solc_config.settings.optimizer.details = None;
project.solc_config.settings.via_ir = None;
}

let output = ProjectCompiler::default()
.compile(&project)?
Expand Down
7 changes: 3 additions & 4 deletions crates/forge/bin/cmd/flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,10 @@ impl FlattenArgs {

// flatten is a subset of `BuildArgs` so we can reuse that to get the config
let build_args = CoreBuildArgs { project_paths, ..Default::default() };
let config = build_args.try_load_config_emit_warnings()?;
let mut project = config.ephemeral_no_artifacts_project()?;

let mut config = build_args.try_load_config_emit_warnings()?;
// `Flattener` uses the typed AST for better flattening results.
project.solc_config.settings = project.solc_config.settings.with_ast();
config.ast = true;
let project = config.ephemeral_no_artifacts_project()?;

let target_path = dunce::canonicalize(target_path)?;
let compiler_output = ProjectCompiler::new().files([target_path.clone()]).compile(&project);
Expand Down
1 change: 1 addition & 0 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ forgetest!(can_extract_config_values, |prj, cmd| {
ignored_file_paths: vec![],
deny_warnings: false,
via_ir: true,
ast: false,
rpc_storage_caching: StorageCachingConfig {
chains: CachedChains::None,
endpoints: CachedEndpoints::Remote,
Expand Down
Loading