Merge pull request #11730 from ethereum/deduplicate-cli-optimizer-initialization

[CLI] Deduplicate optimizer initialization

Changelog.md

@@ -12,11 +12,12 @@ Compiler Features:
  * SMTChecker: Support low level ``call`` as external calls to unknown code.
  * SMTChecker: Add constraints to better correlate ``address(this).balance`` and ``msg.value``.
  * SMTChecker: Support the ``value`` option for external function calls.
- * Commandline Interface: Disallowed the ``--experimental-via-ir`` option to be used with Standard Json, Assembler and Linker modes.
 
 
 Bugfixes:
  * Code Generator: Use stable source order for ABI functions.
+ * Commandline Interface: Report optimizer options as invalid in Standard JSON and linker modes instead of ignoring them.
+ * Commandline Interface: Disallow the ``--experimental-via-ir`` option in Standard JSON, Assembler and Linker modes.
  * Opcode Optimizer: Prevent the optimizer from running multiple times to avoid potential bytecode differences for referenced code.
  * Name Resolver: Fix that when importing an aliased symbol using ``import {AliasedName} from "a.sol"`` it would use the original name of the symbol and not the aliased one.
  * SMTChecker: Fix false negative caused by ``push`` on storage array references returned by internal functions.

solc/CommandLineInterface.cpp

@@ -551,13 +551,7 @@ bool CommandLineInterface::processInput()
 	}
 	case InputMode::Assembler:
 	{
-		return assemble(
-			m_options.assembly.inputLanguage,
-			m_options.assembly.targetMachine,
-			m_options.optimizer.enabled,
-			m_options.optimizer.expectedExecutionsPerDeployment,
-			m_options.optimizer.yulSteps
-		);
+		return assemble(m_options.assembly.inputLanguage, m_options.assembly.targetMachine);
 	}
 	case InputMode::Linker:
 		return link();
@@ -595,16 +589,7 @@ bool CommandLineInterface::compile()
 		m_compiler->enableIRGeneration(m_options.compiler.outputs.ir || m_options.compiler.outputs.irOptimized);
 		m_compiler->enableEwasmGeneration(m_options.compiler.outputs.ewasm);
 
-		OptimiserSettings settings = m_options.optimizer.enabled ? OptimiserSettings::standard() : OptimiserSettings::minimal();
-		if (m_options.optimizer.expectedExecutionsPerDeployment.has_value())
-			settings.expectedExecutionsPerDeployment = m_options.optimizer.expectedExecutionsPerDeployment.value();
-		if (m_options.optimizer.noOptimizeYul)
-			settings.runYulOptimiser = false;
-
-		if (m_options.optimizer.yulSteps.has_value())
-			settings.yulOptimiserSteps = m_options.optimizer.yulSteps.value();
-		settings.optimizeStackAllocation = settings.runYulOptimiser;
-		m_compiler->setOptimiserSettings(settings);
+		m_compiler->setOptimiserSettings(m_options.optimiserSettings());
 
 		if (m_options.input.mode == InputMode::CompilerWithASTImport)
 		{
@@ -939,27 +924,21 @@ string CommandLineInterface::objectWithLinkRefsHex(evmasm::LinkerObject const& _
 	return out;
 }
 
-bool CommandLineInterface::assemble(
-	yul::AssemblyStack::Language _language,
-	yul::AssemblyStack::Machine _targetMachine,
-	bool _optimize,
-	optional<unsigned int> _expectedExecutionsPerDeployment,
-	optional<string> _yulOptimiserSteps
-)
+bool CommandLineInterface::assemble(yul::AssemblyStack::Language _language, yul::AssemblyStack::Machine _targetMachine)
 {
-	solAssert(_optimize || !_yulOptimiserSteps.has_value(), "");
-
 	bool successful = true;
 	map<string, yul::AssemblyStack> assemblyStacks;
 	for (auto const& src: m_fileReader.sourceCodes())
 	{
-		OptimiserSettings settings = _optimize ? OptimiserSettings::full() : OptimiserSettings::minimal();
-		if (_expectedExecutionsPerDeployment.has_value())
-			settings.expectedExecutionsPerDeployment = _expectedExecutionsPerDeployment.value();
-		if (_yulOptimiserSteps.has_value())
-			settings.yulOptimiserSteps = _yulOptimiserSteps.value();
+		// --no-optimize-yul option is not accepted in assembly mode.
+		solAssert(!m_options.optimizer.noOptimizeYul, "");
+
+		auto& stack = assemblyStacks[src.first] = yul::AssemblyStack(
+			m_options.output.evmVersion,
+			_language,
+			m_options.optimiserSettings()
+		);
 
-		auto& stack = assemblyStacks[src.first] = yul::AssemblyStack(m_options.output.evmVersion, _language, settings);
 		try
 		{
 			if (!stack.parseAndAnalyze(src.first, src.second))

solc/CommandLineInterface.h

@@ -74,13 +74,7 @@ private:
 	/// @returns the full object with library placeholder hints in hex.
 	static std::string objectWithLinkRefsHex(evmasm::LinkerObject const& _obj);
 
-	bool assemble(
-		yul::AssemblyStack::Language _language,
-		yul::AssemblyStack::Machine _targetMachine,
-		bool _optimize,
-		std::optional<unsigned int> _expectedExecutionsPerDeployment = std::nullopt,
-		std::optional<std::string> _yulOptimiserSteps = std::nullopt
-	);
+	bool assemble(yul::AssemblyStack::Language _language, yul::AssemblyStack::Machine _targetMachine);
 
 	void outputCompilationResults();
 

solc/CommandLineParser.cpp

@@ -302,6 +302,27 @@ bool CommandLineOptions::operator==(CommandLineOptions const& _other) const noex
 		modelChecker.settings == _other.modelChecker.settings;
 }
 
+OptimiserSettings CommandLineOptions::optimiserSettings() const
+{
+	OptimiserSettings settings;
+
+	if (optimizer.enabled)
+		settings = OptimiserSettings::standard();
+	else
+		settings = OptimiserSettings::minimal();
+
+	if (optimizer.noOptimizeYul)
+		settings.runYulOptimiser = false;
+
+	if (optimizer.expectedExecutionsPerDeployment.has_value())
+		settings.expectedExecutionsPerDeployment = optimizer.expectedExecutionsPerDeployment.value();
+
+	if (optimizer.yulSteps.has_value())
+		settings.yulOptimiserSteps = optimizer.yulSteps.value();
+
+	return settings;
+}
+
 bool CommandLineParser::parseInputPathsAndRemappings()
 {
 	m_options.input.ignoreMissingFiles = (m_args.count(g_strIgnoreMissingFiles) > 0);
@@ -940,6 +961,26 @@ General Information)").c_str(),
 	if (!parseInputPathsAndRemappings())
 		return false;
 
+	if (
+		m_options.input.mode != InputMode::Compiler &&
+		m_options.input.mode != InputMode::CompilerWithASTImport &&
+		m_options.input.mode != InputMode::Assembler
+	)
+	{
+		if (!m_args[g_strOptimizeRuns].defaulted())
+		{
+			serr() << "Option --" << g_strOptimizeRuns << " is only valid in compiler and assembler modes." << endl;
+			return false;
+		}
+
+		for (string const& option: {g_strOptimize, g_strNoOptimizeYul, g_strOptimizeYul, g_strYulOptimizations})
+			if (m_args.count(option) > 0)
+			{
+				serr() << "Option --" << option << " is only valid in compiler and assembler modes." << endl;
+				return false;
+			}
+	}
+
 	if (m_options.input.mode == InputMode::StandardJson)
 		return true;
 
@@ -948,6 +989,9 @@ General Information)").c_str(),
 			if (!parseLibraryOption(library))
 				return false;
 
+	if (m_options.input.mode == InputMode::Linker)
+		return true;
+
 	if (m_args.count(g_strEVMVersion))
 	{
 		string versionOptionStr = m_args[g_strEVMVersion].as<string>();
@@ -960,6 +1004,33 @@ General Information)").c_str(),
 		m_options.output.evmVersion = *versionOption;
 	}
 
+	m_options.optimizer.enabled = (m_args.count(g_strOptimize) > 0);
+	m_options.optimizer.noOptimizeYul = (m_args.count(g_strNoOptimizeYul) > 0);
+	if (!m_args[g_strOptimizeRuns].defaulted())
+		m_options.optimizer.expectedExecutionsPerDeployment = m_args.at(g_strOptimizeRuns).as<unsigned>();
+
+	if (m_args.count(g_strYulOptimizations))
+	{
+		OptimiserSettings optimiserSettings = m_options.optimiserSettings();
+		if (!optimiserSettings.runYulOptimiser)
+		{
+			serr() << "--" << g_strYulOptimizations << " is invalid if Yul optimizer is disabled" << endl;
+			return false;
+		}
+
+		try
+		{
+			yul::OptimiserSuite::validateSequence(m_args[g_strYulOptimizations].as<string>());
+		}
+		catch (yul::OptimizerException const& _exception)
+		{
+			serr() << "Invalid optimizer step sequence in --" << g_strYulOptimizations << ": " << _exception.what() << endl;
+			return false;
+		}
+
+		m_options.optimizer.yulSteps = m_args[g_strYulOptimizations].as<string>();
+	}
+
 	if (m_options.input.mode == InputMode::Assembler)
 	{
 		vector<string> const nonAssemblyModeOptions = {
@@ -987,32 +1058,6 @@ General Information)").c_str(),
 		using Input = yul::AssemblyStack::Language;
 		using Machine = yul::AssemblyStack::Machine;
 		m_options.assembly.inputLanguage = m_args.count(g_strYul) ? Input::Yul : (m_args.count(g_strStrictAssembly) ? Input::StrictAssembly : Input::Assembly);
-		m_options.optimizer.enabled = (m_args.count(g_strOptimize) > 0);
-		m_options.optimizer.noOptimizeYul = (m_args.count(g_strNoOptimizeYul) > 0);
-
-		if (!m_args[g_strOptimizeRuns].defaulted())
-			m_options.optimizer.expectedExecutionsPerDeployment = m_args.at(g_strOptimizeRuns).as<unsigned>();
-
-		if (m_args.count(g_strYulOptimizations))
-		{
-			if (!m_options.optimizer.enabled)
-			{
-				serr() << "--" << g_strYulOptimizations << " is invalid if Yul optimizer is disabled" << endl;
-				return false;
-			}
-
-			try
-			{
-				yul::OptimiserSuite::validateSequence(m_args[g_strYulOptimizations].as<string>());
-			}
-			catch (yul::OptimizerException const& _exception)
-			{
-				serr() << "Invalid optimizer step sequence in --" << g_strYulOptimizations << ": " << _exception.what() << endl;
-				return false;
-			}
-
-			m_options.optimizer.yulSteps = m_args[g_strYulOptimizations].as<string>();
-		}
 
 		if (m_args.count(g_strMachine))
 		{
@@ -1078,9 +1123,6 @@ General Information)").c_str(),
 		return false;
 	}
 
-	if (m_options.input.mode == InputMode::Linker)
-		return true;
-
 	if (m_args.count(g_strMetadataHash))
 	{
 		string hashStr = m_args[g_strMetadataHash].as<string>();
@@ -1164,36 +1206,6 @@ General Information)").c_str(),
 		m_args.count(g_strModelCheckerTargets) ||
 		m_args.count(g_strModelCheckerTimeout);
 	m_options.output.experimentalViaIR = (m_args.count(g_strExperimentalViaIR) > 0);
-	if (!m_args[g_strOptimizeRuns].defaulted())
-		m_options.optimizer.expectedExecutionsPerDeployment = m_args.at(g_strOptimizeRuns).as<unsigned>();
-
-	m_options.optimizer.enabled = (m_args.count(g_strOptimize) > 0);
-	m_options.optimizer.noOptimizeYul = (m_args.count(g_strNoOptimizeYul) > 0);
-
-	OptimiserSettings settings = m_options.optimizer.enabled ? OptimiserSettings::standard() : OptimiserSettings::minimal();
-	if (m_options.optimizer.noOptimizeYul)
-		settings.runYulOptimiser = false;
-	if (m_args.count(g_strYulOptimizations))
-	{
-		if (!settings.runYulOptimiser)
-		{
-			serr() << "--" << g_strYulOptimizations << " is invalid if Yul optimizer is disabled" << endl;
-			return false;
-		}
-
-		try
-		{
-			yul::OptimiserSuite::validateSequence(m_args[g_strYulOptimizations].as<string>());
-		}
-		catch (yul::OptimizerException const& _exception)
-		{
-			serr() << "Invalid optimizer step sequence in --" << g_strYulOptimizations << ": " << _exception.what() << endl;
-			return false;
-		}
-
-		m_options.optimizer.yulSteps = m_args[g_strYulOptimizations].as<string>();
-	}
-
 	if (m_options.input.mode == InputMode::Compiler)
 		m_options.input.errorRecovery = (m_args.count(g_strErrorRecovery) > 0);
 

solc/CommandLineParser.h

@@ -102,6 +102,7 @@ struct CommandLineOptions
 	bool operator==(CommandLineOptions const& _other) const noexcept;
 	bool operator!=(CommandLineOptions const& _other) const noexcept { return !(*this == _other); }
 
+	OptimiserSettings optimiserSettings() const;
 
 	struct
 	{
@@ -169,7 +170,6 @@ struct CommandLineOptions
 		bool initialize = false;
 		ModelCheckerSettings settings;
 	} modelChecker;
-
 };
 
 /// Parses the command-line arguments and produces a filled-out CommandLineOptions structure.

test/cmdlineTests/linker_mode_invalid_option_no_optimize_yul/args

@@ -0,0 +1 @@
+--no-optimize-yul --link --libraries input.sol:L=0x1234567890123456789012345678901234567890

test/cmdlineTests/linker_mode_invalid_option_no_optimize_yul/err

@@ -0,0 +1 @@
+Option --no-optimize-yul is only valid in compiler and assembler modes.

test/cmdlineTests/linker_mode_invalid_option_no_optimize_yul/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/linker_mode_invalid_option_optimize/args

@@ -0,0 +1 @@
+--optimize --link --libraries input.sol:L=0x1234567890123456789012345678901234567890

test/cmdlineTests/linker_mode_invalid_option_optimize/err

@@ -0,0 +1 @@
+Option --optimize is only valid in compiler and assembler modes.

test/cmdlineTests/linker_mode_invalid_option_optimize/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/linker_mode_invalid_option_optimize_runs/args

@@ -0,0 +1 @@
+--optimize-runs 1000 --link --libraries input.sol:L=0x1234567890123456789012345678901234567890

test/cmdlineTests/linker_mode_invalid_option_optimize_runs/err

@@ -0,0 +1 @@
+Option --optimize-runs is only valid in compiler and assembler modes.

test/cmdlineTests/linker_mode_invalid_option_optimize_runs/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/linker_mode_invalid_option_optimize_yul/args

@@ -0,0 +1 @@
+--optimize-yul --link --libraries input.sol:L=0x1234567890123456789012345678901234567890

test/cmdlineTests/linker_mode_invalid_option_optimize_yul/err

@@ -0,0 +1 @@
+Option --optimize-yul is only valid in compiler and assembler modes.

test/cmdlineTests/linker_mode_invalid_option_optimize_yul/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/linker_mode_invalid_option_yul_optimizations/args

@@ -0,0 +1 @@
+--yul-optimizations a --link --libraries input.sol:L=0x1234567890123456789012345678901234567890

test/cmdlineTests/linker_mode_invalid_option_yul_optimizations/err

@@ -0,0 +1 @@
+Option --yul-optimizations is only valid in compiler and assembler modes.

test/cmdlineTests/linker_mode_invalid_option_yul_optimizations/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/standard_invalid_option_no_optimize_yul/args

@@ -0,0 +1 @@
+--no-optimize-yul

test/cmdlineTests/standard_invalid_option_no_optimize_yul/err

@@ -0,0 +1 @@
+Option --no-optimize-yul is only valid in compiler and assembler modes.

test/cmdlineTests/standard_invalid_option_no_optimize_yul/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/standard_invalid_option_optimize/args

@@ -0,0 +1 @@
+--optimize

test/cmdlineTests/standard_invalid_option_optimize/err

@@ -0,0 +1 @@
+Option --optimize is only valid in compiler and assembler modes.

test/cmdlineTests/standard_invalid_option_optimize/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/standard_invalid_option_optimize_runs/args

@@ -0,0 +1 @@
+--optimize-runs 1000

test/cmdlineTests/standard_invalid_option_optimize_runs/err

@@ -0,0 +1 @@
+Option --optimize-runs is only valid in compiler and assembler modes.

test/cmdlineTests/standard_invalid_option_optimize_runs/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/standard_invalid_option_optimize_yul/args

@@ -0,0 +1 @@
+--optimize-yul

test/cmdlineTests/standard_invalid_option_optimize_yul/err

@@ -0,0 +1 @@
+Option --optimize-yul is only valid in compiler and assembler modes.

test/cmdlineTests/standard_invalid_option_optimize_yul/exit

@@ -0,0 +1 @@
+1

test/cmdlineTests/standard_invalid_option_yul_optimizations/args

@@ -0,0 +1 @@
+--yul-optimizations a

test/cmdlineTests/standard_invalid_option_yul_optimizations/err

@@ -0,0 +1 @@
+Option --yul-optimizations is only valid in compiler and assembler modes.

test/cmdlineTests/standard_invalid_option_yul_optimizations/exit

@@ -0,0 +1 @@
+1

test/solc/CommandLineParser.cpp

@@ -368,9 +368,6 @@ BOOST_AUTO_TEST_CASE(standard_json_mode_options)
 		"--combined-json=abi,bin",         // Accepted but has no effect in Standard JSON mode
 		"--metadata-hash=swarm",           // Ignored in Standard JSON mode
 		"--metadata-literal",              // Ignored in Standard JSON mode
-		"--optimize",                      // Ignored in Standard JSON mode
-		"--optimize-runs=1000",            // Ignored in Standard JSON mode
-		"--yul-optimizations=agf",
 		"--model-checker-contracts="       // Ignored in Standard JSON mode
 			"contract1.yul:A,"
 			"contract2.yul:B",