From e8d050a1cefbf8e03da085df264990bbdeac24ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Wed, 14 Jun 2023 06:30:25 +0200 Subject: [PATCH 1/4] Simplify redundant state check in CompilerStack::analyze() --- libsolidity/interface/CompilerStack.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 234ae5d00..320f2e9c5 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -440,7 +440,7 @@ void CompilerStack::importASTs(std::map const& _source bool CompilerStack::analyze() { - if (m_stackState != ParsedAndImported || m_stackState >= AnalysisPerformed) + if (m_stackState != ParsedAndImported) solThrow(CompilerError, "Must call analyze only after parsing was performed."); if (!resolveImports()) From a15ef59eece9df631599e9fa6a899f32e742102a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 12 Jun 2023 12:03:34 +0200 Subject: [PATCH 2/4] Remove the ability to produce an AST in presence of errors in --error-recovery mode This reverts commit 7fd7cc1e765e7e907604dd96fde85be22bde4f80. --- libsolidity/ast/AST.h | 12 +- libsolidity/ast/ASTJsonExporter.cpp | 4 +- libsolidity/interface/CompilerStack.cpp | 89 ++++----- libsolidity/interface/CompilerStack.h | 11 +- libsolidity/interface/StandardCompiler.cpp | 17 +- libsolidity/lsp/LanguageServer.cpp | 4 +- scripts/check_style.sh | 2 +- solc/CommandLineInterface.cpp | 104 +++++----- .../cmdlineTests/recovery_ast_constructor/err | 3 - .../recovery_ast_constructor/exit | 1 + .../recovery_ast_constructor/input.sol | 7 +- .../recovery_ast_constructor/output | 188 ------------------ .../recovery_ast_empty_contract/err | 3 - .../recovery_ast_empty_contract/exit | 1 + .../recovery_standard_json/output.json | 58 +----- test/libsolidity/ASTJSONTest.cpp | 4 +- .../analysis/FunctionCallGraph.cpp | 2 +- 17 files changed, 118 insertions(+), 392 deletions(-) create mode 100644 test/cmdlineTests/recovery_ast_constructor/exit delete mode 100644 test/cmdlineTests/recovery_ast_constructor/output create mode 100644 test/cmdlineTests/recovery_ast_empty_contract/exit diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 9d754202d..ca23b0d61 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -87,15 +87,19 @@ public: static void listAccept(std::vector const& _list, ASTVisitor& _visitor) { for (T const& element: _list) - if (element) - element->accept(_visitor); + { + solAssert(element); + element->accept(_visitor); + } } template static void listAccept(std::vector const& _list, ASTConstVisitor& _visitor) { for (T const& element: _list) - if (element) - element->accept(_visitor); + { + solAssert(element); + element->accept(_visitor); + } } /// @returns a copy of the vector containing only the nodes which derive from T. diff --git a/libsolidity/ast/ASTJsonExporter.cpp b/libsolidity/ast/ASTJsonExporter.cpp index ac847a339..b15917809 100644 --- a/libsolidity/ast/ASTJsonExporter.cpp +++ b/libsolidity/ast/ASTJsonExporter.cpp @@ -560,7 +560,7 @@ bool ASTJsonExporter::visit(EventDefinition const& _node) std::make_pair("parameters", toJson(_node.parameterList())), std::make_pair("anonymous", _node.isAnonymous()) }; - if (m_stackState >= CompilerStack::State::AnalysisPerformed) + if (m_stackState >= CompilerStack::State::AnalysisSuccessful) _attributes.emplace_back( std::make_pair( "eventSelector", @@ -579,7 +579,7 @@ bool ASTJsonExporter::visit(ErrorDefinition const& _node) std::make_pair("documentation", _node.documentation() ? toJson(*_node.documentation()) : Json::nullValue), std::make_pair("parameters", toJson(_node.parameterList())) }; - if (m_stackState >= CompilerStack::State::AnalysisPerformed) + if (m_stackState >= CompilerStack::State::AnalysisSuccessful) _attributes.emplace_back(std::make_pair("errorSelector", _node.functionType(true)->externalIdentifierHex())); setJsonNode(_node, "ErrorDefinition", std::move(_attributes)); diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 320f2e9c5..8151a87e2 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -307,7 +307,6 @@ void CompilerStack::addSMTLib2Response(h256 const& _hash, std::string const& _re void CompilerStack::reset(bool _keepSettings) { m_stackState = Empty; - m_hasError = false; m_sources.clear(); m_smtlib2Responses.clear(); m_unhandledSMTLib2Queries.clear(); @@ -403,16 +402,12 @@ bool CompilerStack::parse() } } - if (m_stopAfter <= Parsed) - m_stackState = Parsed; - else - m_stackState = ParsedAndImported; if (Error::containsErrors(m_errorReporter.errors())) - m_hasError = true; + return false; + m_stackState = (m_stopAfter <= Parsed ? Parsed : ParsedAndImported); storeContractDefinitions(); - - return !m_hasError; + return true; } void CompilerStack::importASTs(std::map const& _sources) @@ -441,7 +436,7 @@ void CompilerStack::importASTs(std::map const& _source bool CompilerStack::analyze() { if (m_stackState != ParsedAndImported) - solThrow(CompilerError, "Must call analyze only after parsing was performed."); + solThrow(CompilerError, "Must call analyze only after parsing was successful."); if (!resolveImports()) return false; @@ -628,11 +623,11 @@ bool CompilerStack::analyze() noErrors = false; } - m_stackState = AnalysisPerformed; if (!noErrors) - m_hasError = true; + return false; - return !m_hasError; + m_stackState = AnalysisSuccessful; + return true; } bool CompilerStack::parseAndAnalyze(State _stopAfter) @@ -642,7 +637,7 @@ bool CompilerStack::parseAndAnalyze(State _stopAfter) bool success = parse(); if (m_stackState >= m_stopAfter) return success; - if (success || m_parserErrorRecovery) + if (success) success = analyze(); return success; } @@ -675,16 +670,13 @@ bool CompilerStack::isRequestedContract(ContractDefinition const& _contract) con bool CompilerStack::compile(State _stopAfter) { m_stopAfter = _stopAfter; - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) if (!parseAndAnalyze(_stopAfter)) return false; if (m_stackState >= m_stopAfter) return true; - if (m_hasError) - solThrow(CompilerError, "Called compile with errors."); - // Only compile contracts individually which have been requested. std::map> otherCompilers; @@ -763,7 +755,7 @@ std::vector CompilerStack::contractNames() const std::string const CompilerStack::lastContractName(std::optional const& _sourceName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Parsing was not successful."); // try to find some user-supplied contract std::string contractName; @@ -866,7 +858,7 @@ std::string const* CompilerStack::runtimeSourceMapping(std::string const& _contr std::string const CompilerStack::filesystemFriendlyName(std::string const& _contractName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "No compiled contracts found."); // Look up the contract (by its fully-qualified name) @@ -980,7 +972,7 @@ std::map CompilerStack::sourceIndices() const Json::Value const& CompilerStack::contractABI(std::string const& _contractName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); return contractABI(contract(_contractName)); @@ -988,7 +980,7 @@ Json::Value const& CompilerStack::contractABI(std::string const& _contractName) Json::Value const& CompilerStack::contractABI(Contract const& _contract) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); solAssert(_contract.contract, ""); @@ -998,7 +990,7 @@ Json::Value const& CompilerStack::contractABI(Contract const& _contract) const Json::Value const& CompilerStack::storageLayout(std::string const& _contractName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); return storageLayout(contract(_contractName)); @@ -1006,7 +998,7 @@ Json::Value const& CompilerStack::storageLayout(std::string const& _contractName Json::Value const& CompilerStack::storageLayout(Contract const& _contract) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); solAssert(_contract.contract, ""); @@ -1016,7 +1008,7 @@ Json::Value const& CompilerStack::storageLayout(Contract const& _contract) const Json::Value const& CompilerStack::natspecUser(std::string const& _contractName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); return natspecUser(contract(_contractName)); @@ -1024,7 +1016,7 @@ Json::Value const& CompilerStack::natspecUser(std::string const& _contractName) Json::Value const& CompilerStack::natspecUser(Contract const& _contract) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); solAssert(_contract.contract, ""); @@ -1034,7 +1026,7 @@ Json::Value const& CompilerStack::natspecUser(Contract const& _contract) const Json::Value const& CompilerStack::natspecDev(std::string const& _contractName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); return natspecDev(contract(_contractName)); @@ -1042,7 +1034,7 @@ Json::Value const& CompilerStack::natspecDev(std::string const& _contractName) c Json::Value const& CompilerStack::natspecDev(Contract const& _contract) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); solAssert(_contract.contract, ""); @@ -1052,7 +1044,7 @@ Json::Value const& CompilerStack::natspecDev(Contract const& _contract) const Json::Value CompilerStack::interfaceSymbols(std::string const& _contractName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); Json::Value interfaceSymbols(Json::objectValue); @@ -1082,7 +1074,7 @@ Json::Value CompilerStack::interfaceSymbols(std::string const& _contractName) co bytes CompilerStack::cborMetadata(std::string const& _contractName, bool _forIR) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); return createCBORMetadata(contract(_contractName), _forIR); @@ -1090,7 +1082,7 @@ bytes CompilerStack::cborMetadata(std::string const& _contractName, bool _forIR) std::string const& CompilerStack::metadata(Contract const& _contract) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); solAssert(_contract.contract, ""); @@ -1120,7 +1112,7 @@ SourceUnit const& CompilerStack::ast(std::string const& _sourceName) const ContractDefinition const& CompilerStack::contractDefinition(std::string const& _contractName) const { - if (m_stackState < AnalysisPerformed) + if (m_stackState < AnalysisSuccessful) solThrow(CompilerError, "Analysis was not successful."); return *contract(_contractName).contract; @@ -1219,15 +1211,15 @@ bool CompilerStack::resolveImports() if (sourcesSeen.count(_source)) return; sourcesSeen.insert(_source); - if (_source->ast) - for (ASTPointer const& node: _source->ast->nodes()) - if (ImportDirective const* import = dynamic_cast(node.get())) - { - std::string const& path = *import->annotation().absolutePath; - solAssert(m_sources.count(path), ""); - import->annotation().sourceUnit = m_sources[path].ast.get(); - toposort(&m_sources[path]); - } + solAssert(_source->ast); + for (ASTPointer const& node: _source->ast->nodes()) + if (ImportDirective const* import = dynamic_cast(node.get())) + { + std::string const& path = *import->annotation().absolutePath; + solAssert(m_sources.count(path), ""); + import->annotation().sourceUnit = m_sources[path].ast.get(); + toposort(&m_sources[path]); + } sourceOrder.push_back(_source); }; @@ -1326,8 +1318,7 @@ void CompilerStack::assembleYul( std::shared_ptr _runtimeAssembly ) { - solAssert(m_stackState >= AnalysisPerformed, ""); - solAssert(!m_hasError, ""); + solAssert(m_stackState >= AnalysisSuccessful, ""); Contract& compiledContract = m_contracts.at(_contract.fullyQualifiedName()); @@ -1400,9 +1391,7 @@ void CompilerStack::compileContract( { solAssert(!m_viaIR, ""); solUnimplementedAssert(!m_eofVersion.has_value(), "Experimental EOF support is only available for via-IR compilation."); - solAssert(m_stackState >= AnalysisPerformed, ""); - if (m_hasError) - solThrow(CompilerError, "Called compile with errors."); + solAssert(m_stackState >= AnalysisSuccessful, ""); if (_otherCompilers.count(&_contract)) return; @@ -1438,9 +1427,7 @@ void CompilerStack::compileContract( void CompilerStack::generateIR(ContractDefinition const& _contract) { - solAssert(m_stackState >= AnalysisPerformed, ""); - if (m_hasError) - solThrow(CompilerError, "Called generateIR with errors."); + solAssert(m_stackState >= AnalysisSuccessful, ""); Contract& compiledContract = m_contracts.at(_contract.fullyQualifiedName()); if (!compiledContract.yulIR.empty()) @@ -1502,9 +1489,7 @@ void CompilerStack::generateIR(ContractDefinition const& _contract) void CompilerStack::generateEVMFromIR(ContractDefinition const& _contract) { - solAssert(m_stackState >= AnalysisPerformed, ""); - if (m_hasError) - solThrow(CompilerError, "Called generateEVMFromIR with errors."); + solAssert(m_stackState >= AnalysisSuccessful, ""); if (!_contract.canBeDeployed()) return; @@ -1535,7 +1520,7 @@ void CompilerStack::generateEVMFromIR(ContractDefinition const& _contract) CompilerStack::Contract const& CompilerStack::contract(std::string const& _contractName) const { - solAssert(m_stackState >= AnalysisPerformed, ""); + solAssert(m_stackState >= AnalysisSuccessful, ""); auto it = m_contracts.find(_contractName); if (it != m_contracts.end()) diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index 8880770b8..072333f9f 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -86,8 +86,6 @@ class DeclarationContainer; * Easy to use and self-contained Solidity compiler with as few header dependencies as possible. * It holds state and can be used to either step through the compilation stages (and abort e.g. * before compilation to bytecode) or run the whole compilation in one call. - * If error recovery is active, it is possible to progress through the stages even when - * there are errors. In any case, producing code is only possible without errors. */ class CompilerStack: public langutil::CharStreamProvider { @@ -101,7 +99,7 @@ public: SourcesSet, Parsed, ParsedAndImported, - AnalysisPerformed, + AnalysisSuccessful, CompilationSuccessful }; @@ -137,10 +135,6 @@ public: /// @returns the current state. State state() const { return m_stackState; } - bool hasError() const { return m_hasError; } - - bool compilationSuccessful() const { return m_stackState >= CompilationSuccessful; } - /// Resets the compiler to an empty state. Unless @a _keepSettings is set to true, /// all settings are reset as well. void reset(bool _keepSettings = false); @@ -520,9 +514,6 @@ private: bool m_parserErrorRecovery = false; State m_stackState = Empty; CompilationSourceType m_compilationSourceType = CompilationSourceType::Solidity; - /// Whether or not there has been an error during processing. - /// If this is true, the stack will refuse to generate code. - bool m_hasError = false; MetadataFormat m_metadataFormat = defaultMetadataFormat(); }; diff --git a/libsolidity/interface/StandardCompiler.cpp b/libsolidity/interface/StandardCompiler.cpp index 20cec68ae..d16a2742f 100644 --- a/libsolidity/interface/StandardCompiler.cpp +++ b/libsolidity/interface/StandardCompiler.cpp @@ -1317,21 +1317,18 @@ Json::Value StandardCompiler::compileSolidity(StandardCompiler::InputsAndSetting } bool parsingSuccess = compilerStack.state() >= CompilerStack::State::Parsed; - bool analysisPerformed = compilerStack.state() >= CompilerStack::State::AnalysisPerformed; + bool analysisSuccess = compilerStack.state() >= CompilerStack::State::AnalysisSuccessful; bool compilationSuccess = compilerStack.state() == CompilerStack::State::CompilationSuccessful; - if (compilerStack.hasError() && !_inputsAndSettings.parserErrorRecovery) - analysisPerformed = false; - // If analysis fails, the artifacts inside CompilerStack are potentially incomplete and must not be returned. // Note that not completing analysis due to stopAfter does not count as a failure. It's neither failure nor success. - bool analysisFailed = !analysisPerformed && _inputsAndSettings.stopAfter >= CompilerStack::State::AnalysisPerformed; + bool analysisFailed = !analysisSuccess && _inputsAndSettings.stopAfter >= CompilerStack::State::AnalysisSuccessful; bool compilationFailed = !compilationSuccess && binariesRequested; /// Inconsistent state - stop here to receive error reports from users if ( - (compilationFailed || !analysisPerformed) && - (errors.empty() && _inputsAndSettings.stopAfter >= CompilerStack::State::AnalysisPerformed) + (compilationFailed || !analysisSuccess) && + (errors.empty() && _inputsAndSettings.stopAfter >= CompilerStack::State::AnalysisSuccessful) ) return formatFatalError(Error::Type::InternalCompilerError, "No error reported, but compilation failed."); @@ -1348,7 +1345,9 @@ Json::Value StandardCompiler::compileSolidity(StandardCompiler::InputsAndSetting output["sources"] = Json::objectValue; unsigned sourceIndex = 0; - if (parsingSuccess && !analysisFailed && (!compilerStack.hasError() || _inputsAndSettings.parserErrorRecovery)) + // NOTE: A case that will pass `parsingSuccess && !analysisFailed` but not `analysisSuccess` is + // stopAfter: parsing with no parsing errors. + if (parsingSuccess && !analysisFailed) for (std::string const& sourceName: compilerStack.sourceNames()) { Json::Value sourceResult = Json::objectValue; @@ -1359,7 +1358,7 @@ Json::Value StandardCompiler::compileSolidity(StandardCompiler::InputsAndSetting } Json::Value contractsOutput = Json::objectValue; - for (std::string const& contractName: analysisPerformed ? compilerStack.contractNames() : std::vector()) + for (std::string const& contractName: analysisSuccess ? compilerStack.contractNames() : std::vector()) { size_t colon = contractName.rfind(':'); solAssert(colon != std::string::npos, ""); diff --git a/libsolidity/lsp/LanguageServer.cpp b/libsolidity/lsp/LanguageServer.cpp index 81b95e05a..2b8b62706 100644 --- a/libsolidity/lsp/LanguageServer.cpp +++ b/libsolidity/lsp/LanguageServer.cpp @@ -265,7 +265,7 @@ void LanguageServer::compile() m_compilerStack.reset(false); m_compilerStack.setSources(m_fileRepository.sourceUnits()); - m_compilerStack.compile(CompilerStack::State::AnalysisPerformed); + m_compilerStack.compile(CompilerStack::State::AnalysisSuccessful); } void LanguageServer::compileAndUpdateDiagnostics() @@ -554,7 +554,7 @@ ASTNode const* LanguageServer::astNodeAtSourceLocation(std::string const& _sourc std::tuple LanguageServer::astNodeAndOffsetAtSourceLocation(std::string const& _sourceUnitName, LineColumn const& _filePos) { - if (m_compilerStack.state() < CompilerStack::AnalysisPerformed) + if (m_compilerStack.state() < CompilerStack::AnalysisSuccessful) return {nullptr, -1}; if (!m_fileRepository.sourceUnits().count(_sourceUnitName)) return {nullptr, -1}; diff --git a/scripts/check_style.sh b/scripts/check_style.sh index 5288cb9a5..3a268e777 100755 --- a/scripts/check_style.sh +++ b/scripts/check_style.sh @@ -41,7 +41,7 @@ REPO_ROOT="$(dirname "$0")"/.. cd "$REPO_ROOT" || exit 1 WHITESPACE=$(git grep -n -I -E "^.*[[:space:]]+$" | - grep -v "test/libsolidity/ASTJSON\|test/libsolidity/ASTRecoveryTests\|test/compilationTests/zeppelin/LICENSE\|${EXCLUDE_FILES_JOINED}" || true + grep -v "test/libsolidity/ASTJSON\|test/compilationTests/zeppelin/LICENSE\|${EXCLUDE_FILES_JOINED}" || true ) if [[ "$WHITESPACE" != "" ]] diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index 509279a71..8cef23bac 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -805,7 +805,7 @@ void CommandLineInterface::compile() formatter.printErrorInformation(*error); } - if (!successful && !m_options.input.errorRecovery) + if (!successful) solThrow(CommandLineExecutionError, ""); } catch (CompilerError const& _exception) @@ -844,6 +844,7 @@ void CommandLineInterface::handleCombinedJSON() output[g_strVersion] = frontend::VersionString; vector contracts = m_compiler->contractNames(); + bool compilationSuccess = m_compiler->state() >= CompilerStack::State::CompilationSuccessful; if (!contracts.empty()) output[g_strContracts] = Json::Value(Json::objectValue); @@ -854,35 +855,35 @@ void CommandLineInterface::handleCombinedJSON() contractData[g_strAbi] = m_compiler->contractABI(contractName); if (m_options.compiler.combinedJsonRequests->metadata) contractData["metadata"] = m_compiler->metadata(contractName); - if (m_options.compiler.combinedJsonRequests->binary && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->binary && compilationSuccess) contractData[g_strBinary] = m_compiler->object(contractName).toHex(); - if (m_options.compiler.combinedJsonRequests->binaryRuntime && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->binaryRuntime && compilationSuccess) contractData[g_strBinaryRuntime] = m_compiler->runtimeObject(contractName).toHex(); - if (m_options.compiler.combinedJsonRequests->opcodes && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->opcodes && compilationSuccess) contractData[g_strOpcodes] = evmasm::disassemble(m_compiler->object(contractName).bytecode, m_options.output.evmVersion); - if (m_options.compiler.combinedJsonRequests->asm_ && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->asm_ && compilationSuccess) contractData[g_strAsm] = m_compiler->assemblyJSON(contractName); - if (m_options.compiler.combinedJsonRequests->storageLayout && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->storageLayout && compilationSuccess) contractData[g_strStorageLayout] = m_compiler->storageLayout(contractName); - if (m_options.compiler.combinedJsonRequests->generatedSources && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->generatedSources && compilationSuccess) contractData[g_strGeneratedSources] = m_compiler->generatedSources(contractName, false); - if (m_options.compiler.combinedJsonRequests->generatedSourcesRuntime && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->generatedSourcesRuntime && compilationSuccess) contractData[g_strGeneratedSourcesRuntime] = m_compiler->generatedSources(contractName, true); - if (m_options.compiler.combinedJsonRequests->srcMap && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->srcMap && compilationSuccess) { auto map = m_compiler->sourceMapping(contractName); contractData[g_strSrcMap] = map ? *map : ""; } - if (m_options.compiler.combinedJsonRequests->srcMapRuntime && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->srcMapRuntime && compilationSuccess) { auto map = m_compiler->runtimeSourceMapping(contractName); contractData[g_strSrcMapRuntime] = map ? *map : ""; } - if (m_options.compiler.combinedJsonRequests->funDebug && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->funDebug && compilationSuccess) contractData[g_strFunDebug] = StandardCompiler::formatFunctionDebugData( m_compiler->object(contractName).functionDebugData ); - if (m_options.compiler.combinedJsonRequests->funDebugRuntime && m_compiler->compilationSuccessful()) + if (m_options.compiler.combinedJsonRequests->funDebugRuntime && compilationSuccess) contractData[g_strFunDebugRuntime] = StandardCompiler::formatFunctionDebugData( m_compiler->runtimeObject(contractName).functionDebugData ); @@ -1164,57 +1165,56 @@ void CommandLineInterface::outputCompilationResults() // do we need AST output? handleAst(); - if ( - !m_compiler->compilationSuccessful() && - m_options.output.stopAfter == CompilerStack::State::CompilationSuccessful - ) + CompilerOutputs astOutputSelection; + astOutputSelection.astCompactJson = true; + if (m_options.compiler.outputs != CompilerOutputs() && m_options.compiler.outputs != astOutputSelection) { - serr() << endl << "Compilation halted after AST generation due to errors." << endl; - return; - } + // Currently AST is the only output allowed with --stop-after parsing. For all of the others + // we can safely assume that full compilation was performed and successful. + solAssert(m_options.output.stopAfter >= CompilerStack::State::CompilationSuccessful); - vector contracts = m_compiler->contractNames(); - for (string const& contract: contracts) - { - if (needsHumanTargetedStdout(m_options)) - sout() << endl << "======= " << contract << " =======" << endl; - - // do we need EVM assembly? - if (m_options.compiler.outputs.asm_ || m_options.compiler.outputs.asmJson) + for (string const& contract: m_compiler->contractNames()) { - string ret; - if (m_options.compiler.outputs.asmJson) - ret = util::jsonPrint(removeNullMembers(m_compiler->assemblyJSON(contract)), m_options.formatting.json); - else - ret = m_compiler->assemblyString(contract, m_fileReader.sourceUnits()); + if (needsHumanTargetedStdout(m_options)) + sout() << endl << "======= " << contract << " =======" << endl; - if (!m_options.output.dir.empty()) - createFile(m_compiler->filesystemFriendlyName(contract) + (m_options.compiler.outputs.asmJson ? "_evm.json" : ".evm"), ret); - else - sout() << "EVM assembly:" << endl << ret << endl; - } + // do we need EVM assembly? + if (m_options.compiler.outputs.asm_ || m_options.compiler.outputs.asmJson) + { + string ret; + if (m_options.compiler.outputs.asmJson) + ret = util::jsonPrint(removeNullMembers(m_compiler->assemblyJSON(contract)), m_options.formatting.json); + else + ret = m_compiler->assemblyString(contract, m_fileReader.sourceUnits()); - if (m_options.compiler.estimateGas) - handleGasEstimation(contract); + if (!m_options.output.dir.empty()) + createFile(m_compiler->filesystemFriendlyName(contract) + (m_options.compiler.outputs.asmJson ? "_evm.json" : ".evm"), ret); + else + sout() << "EVM assembly:" << endl << ret << endl; + } - handleBytecode(contract); - handleIR(contract); - handleIRAst(contract); - handleIROptimized(contract); - handleIROptimizedAst(contract); - handleSignatureHashes(contract); - handleMetadata(contract); - handleABI(contract); - handleStorageLayout(contract); - handleNatspec(true, contract); - handleNatspec(false, contract); - } // end of contracts iteration + if (m_options.compiler.estimateGas) + handleGasEstimation(contract); + + handleBytecode(contract); + handleIR(contract); + handleIRAst(contract); + handleIROptimized(contract); + handleIROptimizedAst(contract); + handleSignatureHashes(contract); + handleMetadata(contract); + handleABI(contract); + handleStorageLayout(contract); + handleNatspec(true, contract); + handleNatspec(false, contract); + } // end of contracts iteration + } if (!m_hasOutput) { if (!m_options.output.dir.empty()) sout() << "Compiler run successful. Artifact(s) can be found in directory " << m_options.output.dir << "." << endl; - else if (contracts.empty()) + else if (m_compiler->contractNames().empty()) sout() << "Compiler run successful. No contracts to compile." << endl; else sout() << "Compiler run successful. No output generated." << endl; diff --git a/test/cmdlineTests/recovery_ast_constructor/err b/test/cmdlineTests/recovery_ast_constructor/err index ecff59f7a..41d3be048 100644 --- a/test/cmdlineTests/recovery_ast_constructor/err +++ b/test/cmdlineTests/recovery_ast_constructor/err @@ -9,6 +9,3 @@ Warning: Recovered in Statement at ';'. | 6 | balances[tx.origin] = ; // missing RHS. | ^ - - -Compilation halted after AST generation due to errors. diff --git a/test/cmdlineTests/recovery_ast_constructor/exit b/test/cmdlineTests/recovery_ast_constructor/exit new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/test/cmdlineTests/recovery_ast_constructor/exit @@ -0,0 +1 @@ +1 diff --git a/test/cmdlineTests/recovery_ast_constructor/input.sol b/test/cmdlineTests/recovery_ast_constructor/input.sol index 8da09638a..ff516d2ae 100644 --- a/test/cmdlineTests/recovery_ast_constructor/input.sol +++ b/test/cmdlineTests/recovery_ast_constructor/input.sol @@ -6,12 +6,7 @@ contract Error1 { balances[tx.origin] = ; // missing RHS. } - // Without error recovery we stop due to the above error. - // Error recovery however recovers at the above ';' - // There should be an AST for the above, albeit with error - // nodes. - - // This function parses properly and should give AST info. + // This function parses properly function five() public view returns(uint) { return 5; } diff --git a/test/cmdlineTests/recovery_ast_constructor/output b/test/cmdlineTests/recovery_ast_constructor/output deleted file mode 100644 index fd92fb7fa..000000000 --- a/test/cmdlineTests/recovery_ast_constructor/output +++ /dev/null @@ -1,188 +0,0 @@ -JSON AST (compact format): - - -======= recovery_ast_constructor/input.sol ======= -{ - "absolutePath": "recovery_ast_constructor/input.sol", - "exportedSymbols": - { - "Error1": - [ - 18 - ] - }, - "id": 19, - "license": "GPL-3.0", - "nodeType": "SourceUnit", - "nodes": - [ - { - "id": 1, - "literals": - [ - "solidity", - ">=", - "0.0", - ".0" - ], - "nodeType": "PragmaDirective", - "src": "36:24:0" - }, - { - "abstract": false, - "baseContracts": [], - "canonicalName": "Error1", - "contractDependencies": [], - "contractKind": "contract", - "fullyImplemented": true, - "id": 18, - "linearizedBaseContracts": - [ - 18 - ], - "name": "Error1", - "nameLocation": "71:6:0", - "nodeType": "ContractDefinition", - "nodes": - [ - { - "body": - { - "id": 8, - "nodeType": "Block", - "src": "96:49:0", - "statements": - [ - null - ] - }, - "id": 9, - "implemented": true, - "kind": "constructor", - "modifiers": [], - "name": "", - "nameLocation": "-1:-1:-1", - "nodeType": "FunctionDefinition", - "parameters": - { - "id": 2, - "nodeType": "ParameterList", - "parameters": [], - "src": "93:2:0" - }, - "returnParameters": - { - "id": 3, - "nodeType": "ParameterList", - "parameters": [], - "src": "96:0:0" - }, - "scope": 18, - "src": "82:63:0", - "stateMutability": "nonpayable", - "virtual": false, - "visibility": "public" - }, - { - "body": - { - "id": 16, - "nodeType": "Block", - "src": "440:19:0", - "statements": - [ - { - "expression": - { - "hexValue": "35", - "id": 14, - "isConstant": false, - "isLValue": false, - "isPure": true, - "kind": "number", - "lValueRequested": false, - "nodeType": "Literal", - "src": "453:1:0", - "typeDescriptions": - { - "typeIdentifier": "t_rational_5_by_1", - "typeString": "int_const 5" - }, - "value": "5" - }, - "functionReturnParameters": 13, - "id": 15, - "nodeType": "Return", - "src": "446:8:0" - } - ] - }, - "functionSelector": "af11c34c", - "id": 17, - "implemented": true, - "kind": "function", - "modifiers": [], - "name": "five", - "nameLocation": "407:4:0", - "nodeType": "FunctionDefinition", - "parameters": - { - "id": 10, - "nodeType": "ParameterList", - "parameters": [], - "src": "411:2:0" - }, - "returnParameters": - { - "id": 13, - "nodeType": "ParameterList", - "parameters": - [ - { - "constant": false, - "id": 12, - "mutability": "mutable", - "name": "", - "nameLocation": "-1:-1:-1", - "nodeType": "VariableDeclaration", - "scope": 17, - "src": "434:4:0", - "stateVariable": false, - "storageLocation": "default", - "typeDescriptions": - { - "typeIdentifier": "t_uint256", - "typeString": "uint256" - }, - "typeName": - { - "id": 11, - "name": "uint", - "nodeType": "ElementaryTypeName", - "src": "434:4:0", - "typeDescriptions": - { - "typeIdentifier": "t_uint256", - "typeString": "uint256" - } - }, - "visibility": "internal" - } - ], - "src": "433:6:0" - }, - "scope": 18, - "src": "398:61:0", - "stateMutability": "view", - "virtual": false, - "visibility": "public" - } - ], - "scope": 19, - "src": "62:399:0", - "usedErrors": [], - "usedEvents": [] - } - ], - "src": "36:426:0" -} diff --git a/test/cmdlineTests/recovery_ast_empty_contract/err b/test/cmdlineTests/recovery_ast_empty_contract/err index fed054470..c047cc82a 100644 --- a/test/cmdlineTests/recovery_ast_empty_contract/err +++ b/test/cmdlineTests/recovery_ast_empty_contract/err @@ -3,6 +3,3 @@ Error: Expected pragma, import directive or contract/interface/library/struct/en | 3 | c | ^ - - -Compilation halted after AST generation due to errors. diff --git a/test/cmdlineTests/recovery_ast_empty_contract/exit b/test/cmdlineTests/recovery_ast_empty_contract/exit new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/test/cmdlineTests/recovery_ast_empty_contract/exit @@ -0,0 +1 @@ +1 diff --git a/test/cmdlineTests/recovery_standard_json/output.json b/test/cmdlineTests/recovery_standard_json/output.json index 110b70616..fa23fde25 100644 --- a/test/cmdlineTests/recovery_standard_json/output.json +++ b/test/cmdlineTests/recovery_standard_json/output.json @@ -42,61 +42,5 @@ "type": "Warning" } ], - "sources": - { - "A": - { - "ast": - { - "absolutePath": "A", - "exportedSymbols": - { - "Errort6": - [ - 3 - ] - }, - "id": 4, - "license": "GPL-3.0", - "nodeType": "SourceUnit", - "nodes": - [ - { - "id": 1, - "literals": - [ - "solidity", - ">=", - "0.0" - ], - "nodeType": "PragmaDirective", - "src": "36:22:0" - }, - { - "abstract": false, - "baseContracts": [], - "canonicalName": "Errort6", - "contractDependencies": [], - "contractKind": "contract", - "fullyImplemented": true, - "id": 3, - "linearizedBaseContracts": - [ - 3 - ], - "name": "Errort6", - "nameLocation": "68:7:0", - "nodeType": "ContractDefinition", - "nodes": [], - "scope": 4, - "src": "59:35:0", - "usedErrors": [], - "usedEvents": [] - } - ], - "src": "36:84:0" - }, - "id": 0 - } - } + "sources": {} } diff --git a/test/libsolidity/ASTJSONTest.cpp b/test/libsolidity/ASTJSONTest.cpp index 6f264fc13..8018d71b3 100644 --- a/test/libsolidity/ASTJSONTest.cpp +++ b/test/libsolidity/ASTJSONTest.cpp @@ -59,7 +59,7 @@ string compilerStateToString(CompilerStack::State _state) case CompilerStack::State::SourcesSet: return "SourcesSet"; case CompilerStack::State::Parsed: return "Parsed"; case CompilerStack::State::ParsedAndImported: return "ParsedAndImported"; - case CompilerStack::State::AnalysisPerformed: return "AnalysisPerformed"; + case CompilerStack::State::AnalysisSuccessful: return "AnalysisSuccessful"; case CompilerStack::State::CompilationSuccessful: return "CompilationSuccessful"; } soltestAssert(false, "Unexpected value of state parameter"); @@ -102,7 +102,7 @@ void ASTJSONTest::generateTestVariants(string const& _filename) const std::vector variantCompileStates = { CompilerStack::State::Parsed, - CompilerStack::State::AnalysisPerformed + CompilerStack::State::AnalysisSuccessful, }; for (const auto state: variantCompileStates) diff --git a/test/libsolidity/analysis/FunctionCallGraph.cpp b/test/libsolidity/analysis/FunctionCallGraph.cpp index da17fc433..9de3441e6 100644 --- a/test/libsolidity/analysis/FunctionCallGraph.cpp +++ b/test/libsolidity/analysis/FunctionCallGraph.cpp @@ -102,7 +102,7 @@ EdgeNames edgeNames(EdgeMap const& _edgeMap) tuple collectGraphs(CompilerStack const& _compilerStack) { - soltestAssert(!_compilerStack.hasError(), ""); + soltestAssert(_compilerStack.state() >= CompilerStack::State::AnalysisSuccessful); tuple graphs; From b17757ea1765b8c332816424162b1906d06d1108 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Wed, 14 Jun 2023 13:02:41 +0200 Subject: [PATCH 3/4] CompilerStack: Extend inconsistent error sanity check to cover stopAfter: parsing as well --- libsolidity/interface/StandardCompiler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libsolidity/interface/StandardCompiler.cpp b/libsolidity/interface/StandardCompiler.cpp index d16a2742f..4d04a4157 100644 --- a/libsolidity/interface/StandardCompiler.cpp +++ b/libsolidity/interface/StandardCompiler.cpp @@ -1327,8 +1327,8 @@ Json::Value StandardCompiler::compileSolidity(StandardCompiler::InputsAndSetting /// Inconsistent state - stop here to receive error reports from users if ( - (compilationFailed || !analysisSuccess) && - (errors.empty() && _inputsAndSettings.stopAfter >= CompilerStack::State::AnalysisSuccessful) + (compilationFailed || analysisFailed || !parsingSuccess) && + errors.empty() ) return formatFatalError(Error::Type::InternalCompilerError, "No error reported, but compilation failed."); From 36429e397c86ee85bee8e7fc10e142977273f3c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Wed, 14 Jun 2023 13:41:41 +0200 Subject: [PATCH 4/4] Fix ICE when requesting `--combined-json` outputs other than AST with `--stop-after parsing` --- solc/CommandLineInterface.cpp | 13 +++-- .../combined_json_stop_after_parsing/args | 1 + .../input.sol | 4 ++ .../combined_json_stop_after_parsing/output | 53 +++++++++++++++++++ 4 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 test/cmdlineTests/combined_json_stop_after_parsing/args create mode 100644 test/cmdlineTests/combined_json_stop_after_parsing/input.sol create mode 100644 test/cmdlineTests/combined_json_stop_after_parsing/output diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index 8cef23bac..e27df9f45 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -844,6 +844,9 @@ void CommandLineInterface::handleCombinedJSON() output[g_strVersion] = frontend::VersionString; vector contracts = m_compiler->contractNames(); + + // NOTE: The state checks here are more strict that in Standard JSON. There we allow + // requesting certain outputs even if compilation fails as long as analysis went ok. bool compilationSuccess = m_compiler->state() >= CompilerStack::State::CompilationSuccessful; if (!contracts.empty()) @@ -851,9 +854,9 @@ void CommandLineInterface::handleCombinedJSON() for (string const& contractName: contracts) { Json::Value& contractData = output[g_strContracts][contractName] = Json::objectValue; - if (m_options.compiler.combinedJsonRequests->abi) + if (m_options.compiler.combinedJsonRequests->abi && compilationSuccess) contractData[g_strAbi] = m_compiler->contractABI(contractName); - if (m_options.compiler.combinedJsonRequests->metadata) + if (m_options.compiler.combinedJsonRequests->metadata && compilationSuccess) contractData["metadata"] = m_compiler->metadata(contractName); if (m_options.compiler.combinedJsonRequests->binary && compilationSuccess) contractData[g_strBinary] = m_compiler->object(contractName).toHex(); @@ -887,11 +890,11 @@ void CommandLineInterface::handleCombinedJSON() contractData[g_strFunDebugRuntime] = StandardCompiler::formatFunctionDebugData( m_compiler->runtimeObject(contractName).functionDebugData ); - if (m_options.compiler.combinedJsonRequests->signatureHashes) + if (m_options.compiler.combinedJsonRequests->signatureHashes && compilationSuccess) contractData[g_strSignatureHashes] = m_compiler->interfaceSymbols(contractName)["methods"]; - if (m_options.compiler.combinedJsonRequests->natspecDev) + if (m_options.compiler.combinedJsonRequests->natspecDev && compilationSuccess) contractData[g_strNatspecDev] = m_compiler->natspecDev(contractName); - if (m_options.compiler.combinedJsonRequests->natspecUser) + if (m_options.compiler.combinedJsonRequests->natspecUser && compilationSuccess) contractData[g_strNatspecUser] = m_compiler->natspecUser(contractName); } diff --git a/test/cmdlineTests/combined_json_stop_after_parsing/args b/test/cmdlineTests/combined_json_stop_after_parsing/args new file mode 100644 index 000000000..692bfd79e --- /dev/null +++ b/test/cmdlineTests/combined_json_stop_after_parsing/args @@ -0,0 +1 @@ +--pretty-json --json-indent 4 --stop-after parsing --combined-json abi,asm,ast,bin,bin-runtime,devdoc,function-debug,function-debug-runtime,generated-sources,generated-sources-runtime,hashes,metadata,opcodes,srcmap,srcmap-runtime,storage-layout,userdoc diff --git a/test/cmdlineTests/combined_json_stop_after_parsing/input.sol b/test/cmdlineTests/combined_json_stop_after_parsing/input.sol new file mode 100644 index 000000000..a3a86cc8d --- /dev/null +++ b/test/cmdlineTests/combined_json_stop_after_parsing/input.sol @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity *; + +contract C {} diff --git a/test/cmdlineTests/combined_json_stop_after_parsing/output b/test/cmdlineTests/combined_json_stop_after_parsing/output new file mode 100644 index 000000000..32cdd019b --- /dev/null +++ b/test/cmdlineTests/combined_json_stop_after_parsing/output @@ -0,0 +1,53 @@ +{ + "contracts": + { + "combined_json_stop_after_parsing/input.sol:C": {} + }, + "sourceList": + [ + "combined_json_stop_after_parsing/input.sol" + ], + "sources": + { + "combined_json_stop_after_parsing/input.sol": + { + "AST": + { + "absolutePath": "combined_json_stop_after_parsing/input.sol", + "id": 3, + "license": "GPL-3.0", + "nodeType": "SourceUnit", + "nodes": + [ + { + "id": 1, + "literals": + [ + "solidity", + "*" + ], + "nodeType": "PragmaDirective", + "src": "36:18:0" + }, + { + "abstract": false, + "baseContracts": [], + "contractDependencies": [], + "contractKind": "contract", + "id": 2, + "name": "C", + "nameLocation": "65:1:0", + "nodeType": "ContractDefinition", + "nodes": [], + "src": "56:13:0", + "usedErrors": [], + "usedEvents": [] + } + ], + "src": "36:34:0" + }, + "id": 0 + } + }, + "version": "" +}