Merge pull request #14479 from ethereum/remove-error-recovery

Remove parser error recovery mode

Changelog.md

@@ -4,6 +4,7 @@ Language Features:
 
 
 Compiler Features:
+ * Parser: Remove the experimental error recovery mode (``--error-recovery`` / ``settings.parserErrorRecovery``).
  * Yul Optimizer: If ``PUSH0`` is supported, favor zero literals over storing zero values in variables.
 
 

liblangutil/ParserBase.cpp

@@ -74,56 +74,10 @@ void ParserBase::expectToken(Token _value, bool _advance)
 {
 	Token tok = m_scanner->currentToken();
 	if (tok != _value)
-	{
-		std::string const expectedToken = ParserBase::tokenName(_value);
-		if (m_parserErrorRecovery)
-			parserError(6635_error, "Expected " + expectedToken + " but got " + tokenName(tok));
-		else
-			fatalParserError(2314_error, "Expected " + expectedToken + " but got " + tokenName(tok));
-		// Do not advance so that recovery can sync or make use of the current token.
-		// This is especially useful if the expected token
-		// is the only one that is missing and is at the end of a construct.
-		// "{ ... ; }" is such an example.
-		//        ^
-		_advance = false;
-	}
-	if (_advance)
-		advance();
-}
-
-void ParserBase::expectTokenOrConsumeUntil(Token _value, std::string const& _currentNodeName, bool _advance)
-{
-	solAssert(m_inParserRecovery, "The function is supposed to be called during parser recovery only.");
-
-	Token tok = m_scanner->currentToken();
-	if (tok != _value)
-	{
-		SourceLocation errorLoc = currentLocation();
-		int startPosition = errorLoc.start;
-		while (m_scanner->currentToken() != _value && m_scanner->currentToken() != Token::EOS)
-			advance();
-
-		std::string const expectedToken = ParserBase::tokenName(_value);
-		if (m_scanner->currentToken() == Token::EOS)
-		{
-			// rollback to where the token started, and raise exception to be caught at a higher level.
-			m_scanner->setPosition(static_cast<size_t>(startPosition));
-			std::string const msg = "In " + _currentNodeName + ", " + expectedToken + "is expected; got " + ParserBase::tokenName(tok) + " instead.";
-			fatalParserError(1957_error, errorLoc, msg);
-		}
-		else
-		{
-			parserWarning(3796_error, "Recovered in " + _currentNodeName + " at " + expectedToken + ".");
-			m_inParserRecovery = false;
-		}
-	}
-	else
-	{
-		std::string expectedToken = ParserBase::tokenName(_value);
-		parserWarning(3347_error, "Recovered in " + _currentNodeName + " at " + expectedToken + ".");
-		m_inParserRecovery = false;
-	}
-
+		fatalParserError(
+			2314_error,
+			"Expected " + ParserBase::tokenName(_value) + " but got " + tokenName(tok)
+		);
 	if (_advance)
 		advance();
 }

liblangutil/ParserBase.h

@@ -38,14 +38,9 @@ struct ErrorId;
 class ParserBase
 {
 public:
-	/// Set @a _parserErrorRecovery to true for additional error
-	/// recovery.  This is experimental and intended for use
-	/// by front-end tools that need partial AST information even
-	/// when errors occur.
-	explicit ParserBase(ErrorReporter& errorReporter, bool _parserErrorRecovery = false): m_errorReporter(errorReporter)
-	{
-		m_parserErrorRecovery = _parserErrorRecovery;
-	}
+	explicit ParserBase(ErrorReporter& errorReporter):
+		m_errorReporter(errorReporter)
+	{}
 
 	virtual ~ParserBase() = default;
 
@@ -70,13 +65,9 @@ class ParserBase
 	///@{
 	///@name Helper functions
 	/// If current token value is not @a _value, throw exception otherwise advance token
-	//  @a if _advance is true and error recovery is in effect.
+	//  if @a _advance is true
 	void expectToken(Token _value, bool _advance = true);
 
-	/// Like expectToken but if there is an error ignores tokens until
-	/// the expected token or EOS is seen. If EOS is encountered, back up to the error point,
-	/// and throw an exception so that a higher grammar rule has an opportunity to recover.
-	void expectTokenOrConsumeUntil(Token _value, std::string const& _currentNodeName, bool _advance = true);
 	Token currentToken() const;
 	Token peekNextToken() const;
 	std::string tokenName(Token _token);
@@ -108,10 +99,6 @@ class ParserBase
 	ErrorReporter& m_errorReporter;
 	/// Current recursion depth during parsing.
 	size_t m_recursionDepth = 0;
-	/// True if we are in parser error recovery. Usually this means we are scanning for
-	/// a synchronization token like ';', or '}'. We use this to reduce cascaded error messages.
-	bool m_inParserRecovery = false;
-	bool m_parserErrorRecovery = false;
 };
 
 }

libsolidity/analysis/SyntaxChecker.cpp

@@ -159,14 +159,7 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma)
 			SemVerMatchExpressionParser parser(tokens, literals);
 			SemVerMatchExpression matchExpression = parser.parse();
 			static SemVerVersion const currentVersion{std::string(VersionString)};
-			if (!matchExpression.matches(currentVersion))
-				m_errorReporter.syntaxError(
-					3997_error,
-					_pragma.location(),
-					"Source file requires different compiler version (current compiler is " +
-					std::string(VersionString) + ") - note that nightly builds are considered to be "
-					"strictly less than the released version"
-				);
+			solAssert(matchExpression.matches(currentVersion));
 			m_versionPragmaFound = true;
 		}
 		catch (SemVerError const&)

libsolidity/interface/CompilerStack.cpp

@@ -352,7 +352,7 @@ bool CompilerStack::parse()
 	if (SemVerVersion{std::string(VersionString)}.isPrerelease())
 		m_errorReporter.warning(3805_error, "This is a pre-release compiler version, please do not use it in production.");
 
-	Parser parser{m_errorReporter, m_evmVersion, m_parserErrorRecovery};
+	Parser parser{m_errorReporter, m_evmVersion};
 
 	std::vector<std::string> sourcesToParse;
 	for (auto const& s: m_sources)
@@ -1104,7 +1104,7 @@ SourceUnit const& CompilerStack::ast(std::string const& _sourceName) const
 {
 	if (m_stackState < Parsed)
 		solThrow(CompilerError, "Parsing not yet performed.");
-	if (!source(_sourceName).ast && !m_parserErrorRecovery)
+	if (!source(_sourceName).ast)
 		solThrow(CompilerError, "Parsing was not successful.");
 
 	return *source(_sourceName).ast;

libsolidity/interface/CompilerStack.h

@@ -158,14 +158,6 @@ class CompilerStack: public langutil::CharStreamProvider
 	/// Sets whether to strip revert strings, add additional strings or do nothing at all.
 	void setRevertStringBehaviour(RevertStrings _revertStrings);
 
-	/// Set whether or not parser error is desired.
-	/// When called without an argument it will revert to the default.
-	/// Must be set before parsing.
-	void setParserErrorRecovery(bool _wantErrorRecovery = false)
-	{
-		m_parserErrorRecovery = _wantErrorRecovery;
-	}
-
 	/// Sets the pipeline to go through the Yul IR or not.
 	/// Must be set before parsing.
 	void setViaIR(bool _viaIR);
@@ -511,7 +503,6 @@ class CompilerStack: public langutil::CharStreamProvider
 	bool m_metadataLiteralSources = false;
 	MetadataHash m_metadataHash = MetadataHash::IPFS;
 	langutil::DebugInfoSelection m_debugInfoSelection = langutil::DebugInfoSelection::Default();
-	bool m_parserErrorRecovery = false;
 	State m_stackState = Empty;
 	CompilationSourceType m_compilationSourceType = CompilationSourceType::Solidity;
 	MetadataFormat m_metadataFormat = defaultMetadataFormat();

libsolidity/interface/StandardCompiler.cpp

@@ -423,7 +423,7 @@ std::optional<Json::Value> checkAuxiliaryInputKeys(Json::Value const& _input)
 
 std::optional<Json::Value> checkSettingsKeys(Json::Value const& _input)
 {
-	static std::set<std::string> keys{"parserErrorRecovery", "debug", "evmVersion", "libraries", "metadata", "modelChecker", "optimizer", "outputSelection", "remappings", "stopAfter", "viaIR"};
+	static std::set<std::string> keys{"debug", "evmVersion", "libraries", "metadata", "modelChecker", "optimizer", "outputSelection", "remappings", "stopAfter", "viaIR"};
 	return checkKeys(_input, keys, "settings");
 }
 
@@ -773,13 +773,6 @@ std::variant<StandardCompiler::InputsAndSettings, Json::Value> StandardCompiler:
 		ret.stopAfter = CompilerStack::State::Parsed;
 	}
 
-	if (settings.isMember("parserErrorRecovery"))
-	{
-		if (!settings["parserErrorRecovery"].isBool())
-			return formatFatalError(Error::Type::JSONError, "\"settings.parserErrorRecovery\" must be a Boolean.");
-		ret.parserErrorRecovery = settings["parserErrorRecovery"].asBool();
-	}
-
 	if (settings.isMember("viaIR"))
 	{
 		if (!settings["viaIR"].isBool())
@@ -1166,7 +1159,6 @@ Json::Value StandardCompiler::compileSolidity(StandardCompiler::InputsAndSetting
 		compilerStack.addSMTLib2Response(smtLib2Response.first, smtLib2Response.second);
 	compilerStack.setViaIR(_inputsAndSettings.viaIR);
 	compilerStack.setEVMVersion(_inputsAndSettings.evmVersion);
-	compilerStack.setParserErrorRecovery(_inputsAndSettings.parserErrorRecovery);
 	compilerStack.setRemappings(std::move(_inputsAndSettings.remappings));
 	compilerStack.setOptimiserSettings(std::move(_inputsAndSettings.optimiserSettings));
 	compilerStack.setRevertStringBehaviour(_inputsAndSettings.revertStrings);

libsolidity/interface/StandardCompiler.h

@@ -72,7 +72,6 @@ class StandardCompiler
 	{
 		std::string language;
 		Json::Value errors;
-		bool parserErrorRecovery = false;
 		CompilerStack::State stopAfter = CompilerStack::State::CompilationSuccessful;
 		std::map<std::string, std::string> sources;
 		std::map<util::h256, std::string> smtLib2Responses;