Merge pull request #14145 from ethereum/small-refactors-extracted-from-asm-import

Trivial refactors extracted from the JSON assembly import PR

libevmasm/Assembly.cpp

@@ -244,7 +244,7 @@ Json::Value Assembly::assemblyJSON(map<string, unsigned> const& _sourceIndices,
 		jsonItem["end"] = item.location().end;
 		if (item.m_modifierDepth != 0)
 			jsonItem["modifierDepth"] = static_cast<int>(item.m_modifierDepth);
-		std::string jumpType = item.getJumpTypeAsString();
+		string jumpType = item.getJumpTypeAsString();
 		if (!jumpType.empty())
 			jsonItem["jumpType"] = jumpType;
 		if (name == "PUSHLIB")
@@ -286,7 +286,7 @@ Json::Value Assembly::assemblyJSON(map<string, unsigned> const& _sourceIndices,
 
 		for (size_t i = 0; i < m_subs.size(); ++i)
 		{
-			std::stringstream hexStr;
+			stringstream hexStr;
 			hexStr << hex << i;
 			data[hexStr.str()] = m_subs[i]->assemblyJSON(_sourceIndices, /*_includeSourceList = */false);
 		}
@@ -341,7 +341,7 @@ Assembly& Assembly::optimise(OptimiserSettings const& _settings)
 
 map<u256, u256> const& Assembly::optimiseInternal(
 	OptimiserSettings const& _settings,
-	std::set<size_t> _tagsReferencedFromOutside
+	set<size_t> _tagsReferencedFromOutside
 )
 {
 	if (m_tagReplacements)

libevmasm/AssemblyItem.cpp

@@ -42,8 +42,8 @@ namespace
 
 string toStringInHex(u256 _value)
 {
-	std::stringstream hexStr;
-	hexStr << std::uppercase << hex << _value;
+	stringstream hexStr;
+	hexStr << uppercase << hex << _value;
 	return hexStr.str();
 }
 
@@ -158,7 +158,7 @@ size_t AssemblyItem::bytesRequired(size_t _addressLength, Precision _precision)
 			return 2;
 	}
 	case VerbatimBytecode:
-		return std::get<2>(*m_verbatimBytecode).size();
+		return get<2>(*m_verbatimBytecode).size();
 	default:
 		break;
 	}
@@ -411,7 +411,7 @@ size_t AssemblyItem::opcodeCount() const noexcept
 	}
 }
 
-std::string AssemblyItem::computeSourceMapping(
+string AssemblyItem::computeSourceMapping(
 	AssemblyItems const& _items,
 	map<string, unsigned> const& _sourceIndicesMap
 )

libsolidity/interface/CompilerStack.cpp

@@ -82,6 +82,9 @@
 #include <boost/algorithm/string/replace.hpp>
 
 #include <range/v3/view/concat.hpp>
+#include <range/v3/view/map.hpp>
+
+#include <fmt/format.h>
 
 #include <utility>
 #include <map>
@@ -94,7 +97,6 @@ using namespace solidity::langutil;
 using namespace solidity::frontend;
 
 using solidity::util::errinfo_comment;
-using solidity::util::toHex;
 
 static int g_compilerStackCounts = 0;
 
@@ -241,7 +243,7 @@ void CompilerStack::setModelCheckerSettings(ModelCheckerSettings _settings)
 	m_modelCheckerSettings = _settings;
 }
 
-void CompilerStack::setLibraries(std::map<std::string, util::h160> const& _libraries)
+void CompilerStack::setLibraries(map<string, util::h160> const& _libraries)
 {
 	if (m_stackState >= ParsedAndImported)
 		solThrow(CompilerError, "Must set libraries before parsing.");
@@ -405,15 +407,14 @@ void CompilerStack::importASTs(map<string, Json::Value> const& _sources)
 {
 	if (m_stackState != Empty)
 		solThrow(CompilerError, "Must call importASTs only before the SourcesSet state.");
-	m_sourceJsons = _sources;
-	map<string, ASTPointer<SourceUnit>> reconstructedSources = ASTJsonImporter(m_evmVersion).jsonToSourceUnit(m_sourceJsons);
+	map<string, ASTPointer<SourceUnit>> reconstructedSources = ASTJsonImporter(m_evmVersion).jsonToSourceUnit(_sources);
 	for (auto& src: reconstructedSources)
 	{
 		string const& path = src.first;
 		Source source;
 		source.ast = src.second;
 		source.charStream = make_shared<CharStream>(
-			util::jsonCompactPrint(m_sourceJsons[src.first]),
+			util::jsonCompactPrint(_sources.at(src.first)),
 			src.first,
 			true // imported from AST
 		);
@@ -711,10 +712,11 @@ bool CompilerStack::compile(State _stopAfter)
 								1834_error,
 								Error::Type::CodeGenerationError,
 								*sourceLocation,
-								"Unimplemented feature error" +
-								((comment && !comment->empty()) ? ": " + *comment : string{}) +
-								" in " +
-								_unimplementedError.lineInfo()
+								fmt::format(
+									"Unimplemented feature error {} in {}",
+									(comment && !comment->empty()) ? ": " + *comment : "",
+									_unimplementedError.lineInfo()
+								)
 							);
 							return false;
 						}
@@ -814,7 +816,6 @@ Json::Value CompilerStack::generatedSources(string const& _contractName, bool _r
 				sources[0]["id"] = sourceIndex;
 				sources[0]["language"] = "Yul";
 				sources[0]["contents"] = std::move(source);
-
 			}
 		}
 		return sources;
@@ -851,7 +852,7 @@ string const* CompilerStack::runtimeSourceMapping(string const& _contractName) c
 	return c.runtimeSourceMapping ? &*c.runtimeSourceMapping : nullptr;
 }
 
-std::string const CompilerStack::filesystemFriendlyName(string const& _contractName) const
+string const CompilerStack::filesystemFriendlyName(string const& _contractName) const
 {
 	if (m_stackState < AnalysisPerformed)
 		solThrow(CompilerError, "No compiled contracts found.");
@@ -865,7 +866,7 @@ std::string const CompilerStack::filesystemFriendlyName(string const& _contractN
 				contract.second.contract != matchContract.contract)
 		{
 			// If it does, then return its fully-qualified name, made fs-friendly
-			std::string friendlyName = boost::algorithm::replace_all_copy(_contractName, "/", "_");
+			string friendlyName = boost::algorithm::replace_all_copy(_contractName, "/", "_");
 			boost::algorithm::replace_all(friendlyName, ":", "_");
 			boost::algorithm::replace_all(friendlyName, ".", "_");
 			return friendlyName;
@@ -951,10 +952,7 @@ Json::Value CompilerStack::assemblyJSON(string const& _contractName) const
 
 vector<string> CompilerStack::sourceNames() const
 {
-	vector<string> names;
-	for (auto const& s: m_sources)
-		names.push_back(s.first);
-	return names;
+	return ranges::to<vector>(m_sources | ranges::views::keys);
 }
 
 map<string, unsigned> CompilerStack::sourceIndices() const
@@ -1117,7 +1115,7 @@ ContractDefinition const& CompilerStack::contractDefinition(string const& _contr
 }
 
 size_t CompilerStack::functionEntryPoint(
-	std::string const& _contractName,
+	string const& _contractName,
 	FunctionDefinition const& _function
 ) const
 {
@@ -1285,10 +1283,10 @@ bool onlySafeExperimentalFeaturesActivated(set<ExperimentalFeature> const& featu
 }
 }
 
-void CompilerStack::assemble(
+void CompilerStack::assembleYul(
 	ContractDefinition const& _contract,
-	std::shared_ptr<evmasm::Assembly> _assembly,
-	std::shared_ptr<evmasm::Assembly> _runtimeAssembly
+	shared_ptr<evmasm::Assembly> _assembly,
+	shared_ptr<evmasm::Assembly> _runtimeAssembly
 )
 {
 	solAssert(m_stackState >= AnalysisPerformed, "");
@@ -1398,7 +1396,7 @@ void CompilerStack::compileContract(
 
 	_otherCompilers[compiledContract.contract] = compiler;
 
-	assemble(_contract, compiler->assemblyPtr(), compiler->runtimeAssemblyPtr());
+	assembleYul(_contract, compiler->assemblyPtr(), compiler->runtimeAssemblyPtr());
 }
 
 void CompilerStack::generateIR(ContractDefinition const& _contract)
@@ -1476,7 +1474,7 @@ void CompilerStack::generateEVMFromIR(ContractDefinition const& _contract)
 	string deployedName = IRNames::deployedObject(_contract);
 	solAssert(!deployedName.empty(), "");
 	tie(compiledContract.evmAssembly, compiledContract.evmRuntimeAssembly) = stack.assembleEVMWithDeployed(deployedName);
-	assemble(_contract, compiledContract.evmAssembly, compiledContract.evmRuntimeAssembly);
+	assembleYul(_contract, compiledContract.evmAssembly, compiledContract.evmRuntimeAssembly);
 }
 
 void CompilerStack::generateEwasm(ContractDefinition const& _contract)
@@ -1599,7 +1597,7 @@ string CompilerStack::createMetadata(Contract const& _contract, bool _forIR) con
 	}
 
 	static_assert(sizeof(m_optimiserSettings.expectedExecutionsPerDeployment) <= sizeof(Json::LargestUInt), "Invalid word size.");
-	solAssert(static_cast<Json::LargestUInt>(m_optimiserSettings.expectedExecutionsPerDeployment) < std::numeric_limits<Json::LargestUInt>::max(), "");
+	solAssert(static_cast<Json::LargestUInt>(m_optimiserSettings.expectedExecutionsPerDeployment) < numeric_limits<Json::LargestUInt>::max(), "");
 	meta["settings"]["optimizer"]["runs"] = Json::Value(Json::LargestUInt(m_optimiserSettings.expectedExecutionsPerDeployment));
 
 	/// Backwards compatibility: If set to one of the default settings, do not provide details.

libsolidity/interface/CompilerStack.h

@@ -426,7 +426,7 @@ private:
 
 	/// Assembles the contract.
 	/// This function should only be internally called by compileContract and generateEVMFromIR.
-	void assemble(
+	void assembleYul(
 		ContractDefinition const& _contract,
 		std::shared_ptr<evmasm::Assembly> _assembly,
 		std::shared_ptr<evmasm::Assembly> _runtimeAssembly
@@ -514,8 +514,6 @@ private:
 	std::map<std::string, util::h160> m_libraries;
 	ImportRemapper m_importRemapper;
 	std::map<std::string const, Source> m_sources;
-	// if imported, store AST-JSONS for each filename
-	std::map<std::string, Json::Value> m_sourceJsons;
 	std::vector<std::string> m_unhandledSMTLib2Queries;
 	std::map<util::h256, std::string> m_smtlib2Responses;
 	std::shared_ptr<GlobalContext> m_globalContext;

libsolutil/Exceptions.h

@@ -54,6 +54,15 @@ struct Exception: virtual std::exception, virtual boost::exception
 		::boost::throw_line(__LINE__) \
 	)
 
+/// Throws an exception if condition is not met with a given description and extra information about the location the
+/// exception was thrown from.
+/// @param _condition if condition is not met, specified exception will be thrown.
+/// @param _exceptionType The type of the exception to throw (not an instance).
+/// @param _description The message that describes the error.
+#define solRequire(_condition, _exceptionType, _description) \
+	if (!(_condition)) \
+		solThrow(_exceptionType, (_description))
+
 /// Defines an exception type that's meant to signal a specific condition and be caught rather than
 /// unwind the stack all the way to the top-level exception handler and interrupt the program.
 /// As such it does not carry a message - the code catching it is expected to handle it without

scripts/ASTImportTest.sh

@@ -19,6 +19,7 @@
 # (c) solidity contributors.
 # ------------------------------------------------------------------------------
 # Bash script to test the import/exports.
+#
 # ast import/export tests:
 #   - first exporting a .sol file to JSON, then loading it into the compiler
 #     and exporting it again. The second JSON should be identical to the first.

solc/CommandLineInterface.cpp

@@ -92,7 +92,7 @@ namespace
 
 set<frontend::InputMode> const CompilerInputModes{
 	frontend::InputMode::Compiler,
-	frontend::InputMode::CompilerWithASTImport
+	frontend::InputMode::CompilerWithASTImport,
 };
 
 } // anonymous namespace
@@ -202,7 +202,7 @@ void CommandLineInterface::handleOpcode(string const& _contract)
 	else
 	{
 		sout() << "Opcodes:" << endl;
-		sout() << std::uppercase << evmasm::disassemble(m_compiler->object(_contract).bytecode, m_options.output.evmVersion);
+		sout() << uppercase << evmasm::disassemble(m_compiler->object(_contract).bytecode, m_options.output.evmVersion);
 		sout() << endl;
 	}
 }
@@ -351,8 +351,8 @@ void CommandLineInterface::handleNatspec(bool _natspecDev, string const& _contra
 	solAssert(CompilerInputModes.count(m_options.input.mode) == 1);
 
 	bool enabled = false;
-	std::string suffix;
-	std::string title;
+	string suffix;
+	string title;
 
 	if (_natspecDev)
 	{
@@ -369,7 +369,7 @@ void CommandLineInterface::handleNatspec(bool _natspecDev, string const& _contra
 
 	if (enabled)
 	{
-		std::string output = jsonPrint(
+		string output = jsonPrint(
 			removeNullMembers(
 				_natspecDev ?
 				m_compiler->natspecDev(_contract) :
@@ -461,7 +461,7 @@ void CommandLineInterface::readInputFiles()
 	for (boost::filesystem::path const& allowedDirectory: m_options.input.allowedDirectories)
 		m_fileReader.allowDirectory(allowedDirectory);
 
-	map<std::string, set<boost::filesystem::path>> collisions =
+	map<string, set<boost::filesystem::path>> collisions =
 		m_fileReader.detectSourceUnitNameCollisions(m_options.input.paths);
 	if (!collisions.empty())
 	{
@@ -551,7 +551,7 @@ map<string, Json::Value> CommandLineInterface::parseAstFromInput()
 
 		for (auto& src: ast["sources"].getMemberNames())
 		{
-			std::string astKey = ast["sources"][src].isMember("ast") ? "ast" : "AST";
+			string astKey = ast["sources"][src].isMember("ast") ? "ast" : "AST";
 
 			astAssert(ast["sources"][src].isMember(astKey), "astkey is not member");
 			astAssert(ast["sources"][src][astKey]["nodeType"].asString() == "SourceUnit",  "Top-level node should be a 'SourceUnit'");
@@ -661,7 +661,7 @@ void CommandLineInterface::processInput()
 		serveLSP();
 		break;
 	case InputMode::Assembler:
-		assemble(m_options.assembly.inputLanguage, m_options.assembly.targetMachine);
+		assembleYul(m_options.assembly.inputLanguage, m_options.assembly.targetMachine);
 		break;
 	case InputMode::Linker:
 		link();
@@ -671,6 +671,7 @@ void CommandLineInterface::processInput()
 	case InputMode::CompilerWithASTImport:
 		compile();
 		outputCompilationResults();
+		break;
 	}
 }
 
@@ -1029,7 +1030,7 @@ string CommandLineInterface::objectWithLinkRefsHex(evmasm::LinkerObject const& _
 	return out;
 }
 
-void CommandLineInterface::assemble(yul::YulStack::Language _language, yul::YulStack::Machine _targetMachine)
+void CommandLineInterface::assembleYul(yul::YulStack::Language _language, yul::YulStack::Machine _targetMachine)
 {
 	solAssert(m_options.input.mode == InputMode::Assembler);
 

solc/CommandLineInterface.h

@@ -27,8 +27,8 @@
 #include <libsolidity/interface/CompilerStack.h>
 #include <libsolidity/interface/DebugSettings.h>
 #include <libsolidity/interface/FileReader.h>
-#include <libsolidity/interface/UniversalCallback.h>
 #include <libsolidity/interface/SMTSolverCommand.h>
+#include <libsolidity/interface/UniversalCallback.h>
 #include <libyul/YulStack.h>
 
 #include <iostream>
@@ -92,7 +92,7 @@ private:
 	/// @returns the full object with library placeholder hints in hex.
 	static std::string objectWithLinkRefsHex(evmasm::LinkerObject const& _obj);
 
-	void assemble(yul::YulStack::Language _language, yul::YulStack::Machine _targetMachine);
+	void assembleYul(yul::YulStack::Language _language, yul::YulStack::Machine _targetMachine);
 
 	void outputCompilationResults();
 

solc/CommandLineParser.cpp

@@ -926,7 +926,7 @@ void CommandLineParser::processArgs()
 		g_strStrictAssembly,
 		g_strYul,
 		g_strImportAst,
-		g_strLSP
+		g_strLSP,
 	});
 
 	if (m_args.count(g_strHelp) > 0)
@@ -1048,7 +1048,7 @@ void CommandLineParser::processArgs()
 	if (m_args.count(g_strRevertStrings))
 	{
 		string revertStringsString = m_args[g_strRevertStrings].as<string>();
-		std::optional<RevertStrings> revertStrings = revertStringsFromString(revertStringsString);
+		optional<RevertStrings> revertStrings = revertStringsFromString(revertStringsString);
 		if (!revertStrings)
 			solThrow(
 				CommandLineValidationError,
@@ -1142,7 +1142,7 @@ void CommandLineParser::processArgs()
 	if (m_args.count(g_strEVMVersion))
 	{
 		string versionOptionStr = m_args[g_strEVMVersion].as<string>();
-		std::optional<langutil::EVMVersion> versionOption = langutil::EVMVersion::fromString(versionOptionStr);
+		optional<langutil::EVMVersion> versionOption = langutil::EVMVersion::fromString(versionOptionStr);
 		if (!versionOption)
 			solThrow(CommandLineValidationError, "Invalid option for --" + g_strEVMVersion + ": " + versionOptionStr);
 		m_options.output.evmVersion = *versionOption;

solc/CommandLineParser.h

@@ -56,7 +56,7 @@ enum class InputMode
 	StandardJson,
 	Linker,
 	Assembler,
-	LanguageServer
+	LanguageServer,
 };
 
 struct CompilerOutputs
@@ -295,4 +295,4 @@ private:
 	boost::program_options::variables_map m_args;
 };
 
-}
+} // namespace solidity::frontend