Skip to content

Commit 017c59d

Browse files
yash-atreyaDaniPopes
yash-atreya
and
DaniPopes
authored
fix(config): enable optimizer when optimizer_runs set in config (#9673)
* fix(`config`): enable optimizer if `optimizer_runs` has been set * test * fix(`config`): change optimizer properties to Option * fix * nit Co-authored-by: DaniPopes <[email protected]> * fix * nit --------- Co-authored-by: DaniPopes <[email protected]>
1 parent 92fefaf commit 017c59d

File tree

14 files changed

+107
-71
lines changed

14 files changed

+107
-71
lines changed

‎crates/config/src/lib.rs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ pub struct Config {
237237
/// install it
238238
pub offline: bool,
239239
/// Whether to activate optimizer
240-
pub optimizer: bool,
240+
pub optimizer: Option<bool>,
241241
/// The number of runs specifies roughly how often each opcode of the deployed code will be
242242
/// executed across the life-time of the contract. This means it is a trade-off parameter
243243
/// between code size (deploy cost) and code execution cost (cost after deployment).
@@ -248,7 +248,7 @@ pub struct Config {
248248
/// A common misconception is that this parameter specifies the number of iterations of the
249249
/// optimizer. This is not true: The optimizer will always run as many times as it can
250250
/// still improve the code.
251-
pub optimizer_runs: usize,
251+
pub optimizer_runs: Option<usize>,
252252
/// Switch optimizer components on or off in detail.
253253
/// The "enabled" switch above provides two defaults which can be
254254
/// tweaked here. If "details" is given, "enabled" can be omitted.
@@ -669,6 +669,15 @@ impl Config {
669669
add_profile(&Self::DEFAULT_PROFILE);
670670
add_profile(&config.profile);
671671

672+
// Ref: https://github.com/foundry-rs/foundry/issues/9665
673+
// Enables the optimizer if the `optimizer_runs` has been set.
674+
let optimizer = config.optimizer();
675+
if optimizer.runs.is_some_and(|runs| runs > 0) && optimizer.enabled.is_none() {
676+
config.optimizer = Some(true);
677+
} else if optimizer.runs.is_none() && optimizer.enabled.is_some_and(|enabled| enabled) {
678+
// Default optimizer runs set to 200 if `optimizer = true`.
679+
config.optimizer_runs = Some(200);
680+
};
672681
Ok(config)
673682
}
674683

@@ -1466,8 +1475,8 @@ impl Config {
14661475
/// and <https://github.com/ethereum/solidity/blob/bbb7f58be026fdc51b0b4694a6f25c22a1425586/docs/using-the-compiler.rst?plain=1#L293-L294>
14671476
pub fn optimizer(&self) -> Optimizer {
14681477
Optimizer {
1469-
enabled: Some(self.optimizer),
1470-
runs: Some(self.optimizer_runs),
1478+
enabled: self.optimizer,
1479+
runs: self.optimizer_runs,
14711480
// we always set the details because `enabled` is effectively a specific details profile
14721481
// that can still be modified
14731482
details: self.optimizer_details.clone(),
@@ -2298,8 +2307,8 @@ impl Default for Config {
22982307
vyper: Default::default(),
22992308
auto_detect_solc: true,
23002309
offline: false,
2301-
optimizer: false,
2302-
optimizer_runs: 200,
2310+
optimizer: None,
2311+
optimizer_runs: None,
23032312
optimizer_details: None,
23042313
model_checker: None,
23052314
extra_output: Default::default(),
@@ -4066,8 +4075,8 @@ mod tests {
40664075
assert_eq!(config.fuzz.runs, 420);
40674076
assert_eq!(config.invariant.depth, 20);
40684077
assert_eq!(config.fork_block_number, Some(100));
4069-
assert_eq!(config.optimizer_runs, 999);
4070-
assert!(!config.optimizer);
4078+
assert_eq!(config.optimizer_runs, Some(999));
4079+
assert!(!config.optimizer.unwrap());
40714080

40724081
Ok(())
40734082
});

‎crates/forge/tests/cli/build.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ contract Dummy {
7272
});
7373

7474
forgetest!(initcode_size_exceeds_limit, |prj, cmd| {
75-
prj.write_config(Config { optimizer: true, ..Default::default() });
75+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
7676
prj.add_source("LargeContract", generate_large_contract(5450).as_str()).unwrap();
7777
cmd.args(["build", "--sizes"]).assert_failure().stdout_eq(str![[r#"
7878
[COMPILING_FILES] with [SOLC_VERSION]
@@ -104,7 +104,7 @@ Compiler run successful!
104104
});
105105

106106
forgetest!(initcode_size_limit_can_be_ignored, |prj, cmd| {
107-
prj.write_config(Config { optimizer: true, ..Default::default() });
107+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
108108
prj.add_source("LargeContract", generate_large_contract(5450).as_str()).unwrap();
109109
cmd.args(["build", "--sizes", "--ignore-eip-3860"]).assert_success().stdout_eq(str![[r#"
110110
[COMPILING_FILES] with [SOLC_VERSION]
@@ -151,7 +151,7 @@ Compiler run successful!
151151
// tests build output is as expected
152152
forgetest_init!(build_sizes_no_forge_std, |prj, cmd| {
153153
prj.write_config(Config {
154-
optimizer: true,
154+
optimizer: Some(true),
155155
solc: Some(foundry_config::SolcReq::Version(semver::Version::new(0, 8, 27))),
156156
..Default::default()
157157
});

‎crates/forge/tests/cli/cmd.rs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1573,7 +1573,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| {
15731573

15741574
// report for all
15751575
prj.write_config(Config {
1576-
optimizer: true,
1576+
optimizer: Some(true),
15771577
gas_reports: (vec!["*".to_string()]),
15781578
gas_reports_ignore: (vec![]),
15791579
..Default::default()
@@ -1683,7 +1683,7 @@ Ran 3 test suites [ELAPSED]: 3 tests passed, 0 failed, 0 skipped (3 total tests)
16831683
.is_json(),
16841684
);
16851685

1686-
prj.write_config(Config { optimizer: true, gas_reports: (vec![]), ..Default::default() });
1686+
prj.write_config(Config { optimizer: Some(true), gas_reports: (vec![]), ..Default::default() });
16871687
cmd.forge_fuse().arg("test").arg("--gas-report").assert_success().stdout_eq(str![[r#"
16881688
...
16891689
╭----------------------------------------+-----------------+-------+--------+-------+---------╮
@@ -1789,7 +1789,7 @@ Ran 3 test suites [ELAPSED]: 3 tests passed, 0 failed, 0 skipped (3 total tests)
17891789
);
17901790

17911791
prj.write_config(Config {
1792-
optimizer: true,
1792+
optimizer: Some(true),
17931793
gas_reports: (vec!["*".to_string()]),
17941794
..Default::default()
17951795
});
@@ -1898,7 +1898,7 @@ Ran 3 test suites [ELAPSED]: 3 tests passed, 0 failed, 0 skipped (3 total tests)
18981898
);
18991899

19001900
prj.write_config(Config {
1901-
optimizer: true,
1901+
optimizer: Some(true),
19021902
gas_reports: (vec![
19031903
"ContractOne".to_string(),
19041904
"ContractTwo".to_string(),
@@ -2017,7 +2017,7 @@ forgetest!(gas_report_some_contracts, |prj, cmd| {
20172017

20182018
// report for One
20192019
prj.write_config(Config {
2020-
optimizer: true,
2020+
optimizer: Some(true),
20212021
gas_reports: vec!["ContractOne".to_string()],
20222022
..Default::default()
20232023
});
@@ -2068,7 +2068,7 @@ Ran 3 test suites [ELAPSED]: 3 tests passed, 0 failed, 0 skipped (3 total tests)
20682068

20692069
// report for Two
20702070
prj.write_config(Config {
2071-
optimizer: true,
2071+
optimizer: Some(true),
20722072
gas_reports: vec!["ContractTwo".to_string()],
20732073
..Default::default()
20742074
});
@@ -2119,7 +2119,7 @@ Ran 3 test suites [ELAPSED]: 3 tests passed, 0 failed, 0 skipped (3 total tests)
21192119

21202120
// report for Three
21212121
prj.write_config(Config {
2122-
optimizer: true,
2122+
optimizer: Some(true),
21232123
gas_reports: vec!["ContractThree".to_string()],
21242124
..Default::default()
21252125
});
@@ -2175,7 +2175,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| {
21752175

21762176
// ignore ContractOne
21772177
prj.write_config(Config {
2178-
optimizer: true,
2178+
optimizer: Some(true),
21792179
gas_reports: (vec!["*".to_string()]),
21802180
gas_reports_ignore: (vec!["ContractOne".to_string()]),
21812181
..Default::default()
@@ -2258,7 +2258,7 @@ Ran 3 test suites [ELAPSED]: 3 tests passed, 0 failed, 0 skipped (3 total tests)
22582258
// ignore ContractTwo
22592259
cmd.forge_fuse();
22602260
prj.write_config(Config {
2261-
optimizer: true,
2261+
optimizer: Some(true),
22622262
gas_reports: (vec![]),
22632263
gas_reports_ignore: (vec!["ContractTwo".to_string()]),
22642264
..Default::default()
@@ -2345,7 +2345,7 @@ Ran 3 test suites [ELAPSED]: 3 tests passed, 0 failed, 0 skipped (3 total tests)
23452345
// indicating the "double listing".
23462346
cmd.forge_fuse();
23472347
prj.write_config(Config {
2348-
optimizer: true,
2348+
optimizer: Some(true),
23492349
gas_reports: (vec![
23502350
"ContractOne".to_string(),
23512351
"ContractTwo".to_string(),
@@ -2479,7 +2479,7 @@ Warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'.
24792479
});
24802480

24812481
forgetest!(gas_report_flatten_multiple_selectors, |prj, cmd| {
2482-
prj.write_config(Config { optimizer: true, ..Default::default() });
2482+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
24832483
prj.insert_ds_test();
24842484
prj.add_source(
24852485
"Counter.sol",
@@ -2598,7 +2598,7 @@ Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests)
25982598

25992599
// <https://github.com/foundry-rs/foundry/issues/9115>
26002600
forgetest_init!(gas_report_with_fallback, |prj, cmd| {
2601-
prj.write_config(Config { optimizer: true, ..Default::default() });
2601+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
26022602
prj.add_test(
26032603
"DelegateProxyTest.sol",
26042604
r#"
@@ -2742,7 +2742,7 @@ Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests)
27422742

27432743
// <https://github.com/foundry-rs/foundry/issues/9300>
27442744
forgetest_init!(gas_report_size_for_nested_create, |prj, cmd| {
2745-
prj.write_config(Config { optimizer: true, ..Default::default() });
2745+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
27462746
prj.add_test(
27472747
"NestedDeployTest.sol",
27482748
r#"
@@ -3181,7 +3181,7 @@ Error: No source files found in specified build paths.
31813181

31823182
// checks that build --sizes includes all contracts even if unchanged
31833183
forgetest_init!(can_build_sizes_repeatedly, |prj, cmd| {
3184-
prj.write_config(Config { optimizer: true, ..Default::default() });
3184+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
31853185
prj.clear_cache();
31863186

31873187
cmd.args(["build", "--sizes"]).assert_success().stdout_eq(str![[r#"
@@ -3248,7 +3248,7 @@ interface Counter {
32483248
// checks that `clean` also works with the "out" value set in Config
32493249
forgetest_init!(gas_report_include_tests, |prj, cmd| {
32503250
prj.write_config(Config {
3251-
optimizer: true,
3251+
optimizer: Some(true),
32523252
gas_reports_include_tests: true,
32533253
fuzz: FuzzConfig { runs: 1, ..Default::default() },
32543254
..Default::default()

‎crates/forge/tests/cli/config.rs‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ forgetest!(can_extract_config_values, |prj, cmd| {
5050
auto_detect_solc: false,
5151
auto_detect_remappings: true,
5252
offline: true,
53-
optimizer: false,
54-
optimizer_runs: 1000,
53+
optimizer: Some(false),
54+
optimizer_runs: Some(1000),
5555
optimizer_details: Some(OptimizerDetails {
5656
yul: Some(false),
5757
yul_details: Some(YulDetails { stack_allocation: Some(true), ..Default::default() }),
@@ -426,7 +426,7 @@ Compiler run successful!
426426

427427
// test to ensure yul optimizer can be set as intended
428428
forgetest!(can_set_yul_optimizer, |prj, cmd| {
429-
prj.write_config(Config { optimizer: true, ..Default::default() });
429+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
430430
prj.add_source(
431431
"foo.sol",
432432
r"
@@ -476,14 +476,35 @@ forgetest_init!(can_parse_dapp_libraries, |_prj, cmd| {
476476
// test that optimizer runs works
477477
forgetest!(can_set_optimizer_runs, |prj, cmd| {
478478
// explicitly set optimizer runs
479-
let config = Config { optimizer_runs: 1337, ..Default::default() };
479+
let config = Config { optimizer_runs: Some(1337), ..Default::default() };
480480
prj.write_config(config);
481481

482482
let config = cmd.config();
483-
assert_eq!(config.optimizer_runs, 1337);
483+
assert_eq!(config.optimizer_runs, Some(1337));
484484

485485
let config = prj.config_from_output(["--optimizer-runs", "300"]);
486-
assert_eq!(config.optimizer_runs, 300);
486+
assert_eq!(config.optimizer_runs, Some(300));
487+
});
488+
489+
// <https://github.com/foundry-rs/foundry/issues/9665>
490+
forgetest!(enable_optimizer_when_runs_set, |prj, cmd| {
491+
// explicitly set optimizer runs
492+
let config = Config { optimizer_runs: Some(1337), ..Default::default() };
493+
assert!(config.optimizer.is_none());
494+
prj.write_config(config);
495+
496+
let config = cmd.config();
497+
assert!(config.optimizer.unwrap());
498+
});
499+
500+
// test `optimizer_runs` set to 200 by default if optimizer enabled
501+
forgetest!(optimizer_runs_default, |prj, cmd| {
502+
// explicitly set optimizer runs
503+
let config = Config { optimizer: Some(true), ..Default::default() };
504+
prj.write_config(config);
505+
506+
let config = cmd.config();
507+
assert_eq!(config.optimizer_runs, Some(200));
487508
});
488509

489510
// test that gas_price can be set

‎crates/forge/tests/cli/script.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1926,7 +1926,7 @@ forgetest_async!(adheres_to_json_flag, |prj, cmd| {
19261926
}
19271927

19281928
foundry_test_utils::util::initialize(prj.root());
1929-
prj.write_config(Config { optimizer: true, ..Default::default() });
1929+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
19301930
prj.add_script(
19311931
"Foo",
19321932
r#"

‎crates/forge/tests/cli/test_cmd.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests)
591591

592592
// https://github.com/foundry-rs/foundry/issues/6579
593593
forgetest_init!(include_custom_types_in_traces, |prj, cmd| {
594-
prj.write_config(Config { optimizer: true, ..Default::default() });
594+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
595595
prj.wipe_contracts();
596596

597597
prj.add_test(
@@ -959,7 +959,7 @@ contract SetupFailureTest is Test {
959959

960960
// https://github.com/foundry-rs/foundry/issues/7530
961961
forgetest_init!(should_show_precompile_labels, |prj, cmd| {
962-
prj.write_config(Config { optimizer: true, ..Default::default() });
962+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
963963
prj.wipe_contracts();
964964

965965
prj.add_test(
@@ -1417,7 +1417,7 @@ contract DeterministicRandomnessTest is Test {
14171417
// Tests that `pauseGasMetering` used at the end of test does not produce meaningless values.
14181418
// https://github.com/foundry-rs/foundry/issues/5491
14191419
forgetest_init!(gas_metering_pause_last_call, |prj, cmd| {
1420-
prj.write_config(Config { optimizer: true, ..Default::default() });
1420+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
14211421
prj.wipe_contracts();
14221422

14231423
prj.add_test(
@@ -1503,7 +1503,7 @@ Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests)
15031503

15041504
// https://github.com/foundry-rs/foundry/issues/4523
15051505
forgetest_init!(gas_metering_gasleft, |prj, cmd| {
1506-
prj.write_config(Config { optimizer: true, ..Default::default() });
1506+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
15071507
prj.wipe_contracts();
15081508

15091509
prj.add_test(
@@ -1582,7 +1582,7 @@ contract ATest is Test {
15821582
// tests `pauseTracing` and `resumeTracing` functions
15831583
#[cfg(not(feature = "isolate-by-default"))]
15841584
forgetest_init!(pause_tracing, |prj, cmd| {
1585-
prj.write_config(Config { optimizer: true, ..Default::default() });
1585+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
15861586
prj.wipe_contracts();
15871587
prj.insert_ds_test();
15881588
prj.insert_vm();
@@ -2331,7 +2331,7 @@ Logs:
23312331

23322332
// <https://github.com/foundry-rs/foundry/issues/8995>
23332333
forgetest_init!(metadata_bytecode_traces, |prj, cmd| {
2334-
prj.write_config(Config { optimizer: true, ..Default::default() });
2334+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
23352335
prj.add_source(
23362336
"ParentProxy.sol",
23372337
r#"
@@ -2454,7 +2454,7 @@ forgetest_async!(can_get_broadcast_txs, |prj, cmd| {
24542454

24552455
let (_api, handle) = spawn(NodeConfig::test().silent()).await;
24562456

2457-
prj.write_config(Config { optimizer: true, ..Default::default() });
2457+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
24582458
prj.insert_vm();
24592459
prj.insert_ds_test();
24602460
prj.insert_console();
@@ -2703,7 +2703,7 @@ Suite result: FAILED. 0 passed; 1 failed; 0 skipped; [ELAPSED]
27032703
// Tests that test traces display state changes when running with verbosity.
27042704
#[cfg(not(feature = "isolate-by-default"))]
27052705
forgetest_init!(should_show_state_changes, |prj, cmd| {
2706-
prj.write_config(Config { optimizer: true, ..Default::default() });
2706+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
27072707

27082708
cmd.args(["test", "--mt", "test_Increment", "-vvvvv"]).assert_success().stdout_eq(str![[r#"
27092709
...
@@ -2764,7 +2764,7 @@ Encountered a total of 1 failing tests, 0 tests succeeded
27642764
// Tests that `start/stopAndReturn` debugTraceRecording does not panic when running with
27652765
// verbosity > 3. <https://github.com/foundry-rs/foundry/issues/9526>
27662766
forgetest_init!(should_not_panic_on_debug_trace_verbose, |prj, cmd| {
2767-
prj.write_config(Config { optimizer: true, ..Default::default() });
2767+
prj.write_config(Config { optimizer: Some(true), ..Default::default() });
27682768
prj.add_test(
27692769
"DebugTraceRecordingTest.t.sol",
27702770
r#"

0 commit comments

Comments
 (0)