Remove the ability to produce an AST in presence of errors in --error-recovery mode

This reverts commit 7fd7cc1e76.

libsolidity/ast/AST.h

@@ -87,15 +87,19 @@ public:
 	static void listAccept(std::vector<T> const& _list, ASTVisitor& _visitor)
 	{
 		for (T const& element: _list)
-			if (element)
-				element->accept(_visitor);
+		{
+			solAssert(element);
+			element->accept(_visitor);
+		}
 	}
 	template <class T>
 	static void listAccept(std::vector<T> 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.

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));

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<std::string, Json::Value> const& _sources)
@@ -441,7 +436,7 @@ void CompilerStack::importASTs(std::map<std::string, Json::Value> 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<ContractDefinition const*, std::shared_ptr<Compiler const>> otherCompilers;
 
@@ -763,7 +755,7 @@ std::vector<std::string> CompilerStack::contractNames() const
 
 std::string const CompilerStack::lastContractName(std::optional<std::string> 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<std::string, unsigned> 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<ASTNode> const& node: _source->ast->nodes())
-				if (ImportDirective const* import = dynamic_cast<ImportDirective*>(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<ASTNode> const& node: _source->ast->nodes())
+			if (ImportDirective const* import = dynamic_cast<ImportDirective*>(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<evmasm::Assembly> _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())

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();
 };
 

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<std::string>())
+	for (std::string const& contractName: analysisSuccess ? compilerStack.contractNames() : std::vector<std::string>())
 	{
 		size_t colon = contractName.rfind(':');
 		solAssert(colon != std::string::npos, "");

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<ASTNode const*, int> 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};