From 4ee703e6e59c12c7d9fb2c7df9d4c3bf2686fe16 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 28 May 2019 18:07:22 +0200 Subject: [PATCH] Revert "Add Steve Johnson-style parser recovery rules:" This reverts commit 97f8ee0d1bca525ac0fec19feeee5cab0bdcbdde. --- Changelog.md | 2 +- liblangutil/CharStream.cpp | 9 +- liblangutil/CharStream.h | 6 - liblangutil/ErrorReporter.cpp | 5 - liblangutil/ErrorReporter.h | 4 +- liblangutil/ParserBase.cpp | 102 ++----- liblangutil/ParserBase.h | 28 +- liblangutil/Scanner.cpp | 7 - liblangutil/Scanner.h | 3 - libsolidity/interface/CompilerStack.cpp | 2 +- libsolidity/interface/CompilerStack.h | 9 - libsolidity/parsing/Parser.cpp | 252 +++++++----------- libsolidity/parsing/Parser.h | 5 +- solc/CommandLineInterface.cpp | 5 - test/InteractiveTests.h | 2 +- test/liblangutil/CharStream.cpp | 53 ---- test/libsolidity/AnalysisFramework.cpp | 5 +- test/libsolidity/AnalysisFramework.h | 3 +- test/libsolidity/SMTChecker.cpp | 6 +- test/libsolidity/SyntaxTest.cpp | 4 +- test/libsolidity/SyntaxTest.h | 11 +- .../constructor_recovers.sol | 20 -- .../errorRecoveryTests/contract_recovery.sol | 7 - .../do_not_delete_at_error.sol | 13 - .../errorRecoveryTests/error_to_eos.sol | 23 -- .../errorRecoveryTests/missing_rhs.sol | 11 - .../errorRecoveryTests/multiple_errors.sol | 29 -- 27 files changed, 137 insertions(+), 489 deletions(-) delete mode 100644 test/liblangutil/CharStream.cpp delete mode 100644 test/libsolidity/errorRecoveryTests/constructor_recovers.sol delete mode 100644 test/libsolidity/errorRecoveryTests/contract_recovery.sol delete mode 100644 test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol delete mode 100644 test/libsolidity/errorRecoveryTests/error_to_eos.sol delete mode 100644 test/libsolidity/errorRecoveryTests/missing_rhs.sol delete mode 100644 test/libsolidity/errorRecoveryTests/multiple_errors.sol diff --git a/Changelog.md b/Changelog.md index ac635537d..4ccefe18d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,7 +8,6 @@ Language Features: Compiler Features: * Assembler: Encode the compiler version in the deployed bytecode. * Code Generator: Fix handling of structs of dynamic size as constructor parameters. - * Commandline Interface: Experimental parser error recovery via the ``--error-recovery`` commandline switch. * Inline Assembly: Disallow the combination of ``msize()`` and the Yul optimizer. * Metadata: Add IPFS hashes of source files. * Optimizer: Add rule to simplify SHL/SHR combinations. @@ -22,6 +21,7 @@ Compiler Features: * Yul Optimizer: Do not inline recursive functions. * Yul Optimizer: Do not remove instructions that affect ``msize()`` if ``msize()`` is used. + Bugfixes: * Code Generator: Explicitly turn uninitialized internal function pointers into invalid functions when loaded from storage. * Code Generator: Fix assertion failure when assigning structs containing array of mapping. diff --git a/liblangutil/CharStream.cpp b/liblangutil/CharStream.cpp index 58edced3b..2c10bd608 100644 --- a/liblangutil/CharStream.cpp +++ b/liblangutil/CharStream.cpp @@ -73,13 +73,6 @@ char CharStream::rollback(size_t _amount) return get(); } -char CharStream::setPosition(size_t _location) -{ - solAssert(_location <= m_source.size(), "Attempting to set position past end of source."); - m_position = _location; - return get(); -} - string CharStream::lineAtPosition(int _position) const { // if _position points to \n, it returns the line before the \n @@ -113,3 +106,5 @@ tuple CharStream::translatePositionToLineColumn(int _position) const } return tuple(lineNumber, searchPosition - lineStart); } + + diff --git a/liblangutil/CharStream.h b/liblangutil/CharStream.h index 504c39da5..c11340b21 100644 --- a/liblangutil/CharStream.h +++ b/liblangutil/CharStream.h @@ -76,13 +76,7 @@ public: char get(size_t _charsForward = 0) const { return m_source[m_position + _charsForward]; } char advanceAndGet(size_t _chars = 1); - /// Sets scanner position to @ _amount characters backwards in source text. - /// @returns The character of the current location after update is returned. char rollback(size_t _amount); - /// Sets scanner position to @ _location if it refers a valid offset in m_source. - /// If not, nothing is done. - /// @returns The character of the current location after update is returned. - char setPosition(size_t _location); void reset() { m_position = 0; } diff --git a/liblangutil/ErrorReporter.cpp b/liblangutil/ErrorReporter.cpp index 7f8a77f1a..0bf3a4167 100644 --- a/liblangutil/ErrorReporter.cpp +++ b/liblangutil/ErrorReporter.cpp @@ -86,11 +86,6 @@ void ErrorReporter::error(Error::Type _type, SourceLocation const& _location, Se m_errorList.push_back(err); } -bool ErrorReporter::hasExcessiveErrors() const -{ - return m_errorCount > c_maxErrorsAllowed; -} - bool ErrorReporter::checkForExcessiveErrors(Error::Type _type) { if (_type == Error::Type::Warning) diff --git a/liblangutil/ErrorReporter.h b/liblangutil/ErrorReporter.h index 6d6b4ef41..30b494a0f 100644 --- a/liblangutil/ErrorReporter.h +++ b/liblangutil/ErrorReporter.h @@ -118,9 +118,6 @@ public: return m_errorCount > 0; } - // @returns true if the maximum error count has been reached. - bool hasExcessiveErrors() const; - private: void error( Error::Type _type, @@ -152,3 +149,4 @@ private: }; } + diff --git a/liblangutil/ParserBase.cpp b/liblangutil/ParserBase.cpp index 2432ac711..edd23fbaf 100644 --- a/liblangutil/ParserBase.cpp +++ b/liblangutil/ParserBase.cpp @@ -47,7 +47,7 @@ Token ParserBase::peekNextToken() const return m_scanner->peekNextToken(); } -string ParserBase::currentLiteral() const +std::string ParserBase::currentLiteral() const { return m_scanner->currentLiteral(); } @@ -57,79 +57,30 @@ Token ParserBase::advance() return m_scanner->next(); } -string ParserBase::tokenName(Token _token) -{ - if (_token == Token::Identifier) - return "identifier"; - else if (_token == Token::EOS) - return "end of source"; - else if (TokenTraits::isReservedKeyword(_token)) - return "reserved keyword '" + TokenTraits::friendlyName(_token) + "'"; - else if (TokenTraits::isElementaryTypeName(_token)) //for the sake of accuracy in reporting - { - ElementaryTypeNameToken elemTypeName = m_scanner->currentElementaryTypeNameToken(); - return "'" + elemTypeName.toString() + "'"; - } - else - return "'" + TokenTraits::friendlyName(_token) + "'"; -} - void ParserBase::expectToken(Token _value, bool _advance) { Token tok = m_scanner->currentToken(); if (tok != _value) { - string const expectedToken = ParserBase::tokenName(_value); - if (m_parserErrorRecovery) - parserError("Expected " + expectedToken + " but got " + tokenName(tok)); - else - fatalParserError("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) - m_scanner->next(); -} - -void ParserBase::expectTokenOrConsumeUntil(Token _value, string const& _currentNodeName, bool _advance) -{ - Token tok = m_scanner->currentToken(); - if (tok != _value) - { - int startPosition = position(); - SourceLocation errorLoc = SourceLocation{startPosition, endPosition(), source()}; - while (m_scanner->currentToken() != _value && m_scanner->currentToken() != Token::EOS) - m_scanner->next(); - - string const expectedToken = ParserBase::tokenName(_value); - string const msg = "In " + _currentNodeName + ", " + expectedToken + "is expected; got " + ParserBase::tokenName(tok) + "instead."; - if (m_scanner->currentToken() == Token::EOS) + auto tokenName = [this](Token _token) { - // rollback to where the token started, and raise exception to be caught at a higher level. - m_scanner->setPosition(startPosition); - m_inParserRecovery = true; - fatalParserError(errorLoc, msg); - } - else - { - if (m_inParserRecovery) - parserWarning("Recovered in " + _currentNodeName + " at " + expectedToken + "."); + if (_token == Token::Identifier) + return string("identifier"); + else if (_token == Token::EOS) + return string("end of source"); + else if (TokenTraits::isReservedKeyword(_token)) + return string("reserved keyword '") + TokenTraits::friendlyName(_token) + "'"; + else if (TokenTraits::isElementaryTypeName(_token)) //for the sake of accuracy in reporting + { + ElementaryTypeNameToken elemTypeName = m_scanner->currentElementaryTypeNameToken(); + return string("'") + elemTypeName.toString() + "'"; + } else - parserError(errorLoc, msg + "Recovered at next " + expectedToken); - m_inParserRecovery = false; - } - } - else if (m_inParserRecovery) - { - string expectedToken = ParserBase::tokenName(_value); - parserWarning("Recovered in " + _currentNodeName + " at " + expectedToken + "."); - m_inParserRecovery = false; - } + return string("'") + TokenTraits::friendlyName(_token) + "'"; + }; + fatalParserError(string("Expected ") + tokenName(_value) + string(" but got ") + tokenName(tok)); + } if (_advance) m_scanner->next(); } @@ -147,27 +98,12 @@ void ParserBase::decreaseRecursionDepth() m_recursionDepth--; } -void ParserBase::parserWarning(string const& _description) -{ - m_errorReporter.warning(SourceLocation{position(), endPosition(), source()}, _description); -} - -void ParserBase::parserError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.parserError(_location, _description); -} - void ParserBase::parserError(string const& _description) { - parserError(SourceLocation{position(), endPosition(), source()}, _description); + m_errorReporter.parserError(SourceLocation{position(), endPosition(), source()}, _description); } void ParserBase::fatalParserError(string const& _description) { - fatalParserError(SourceLocation{position(), endPosition(), source()}, _description); -} - -void ParserBase::fatalParserError(SourceLocation const& _location, string const& _description) -{ - m_errorReporter.fatalParserError(_location, _description); + m_errorReporter.fatalParserError(SourceLocation{position(), endPosition(), source()}, _description); } diff --git a/liblangutil/ParserBase.h b/liblangutil/ParserBase.h index 98123b813..1c6f298c1 100644 --- a/liblangutil/ParserBase.h +++ b/liblangutil/ParserBase.h @@ -36,14 +36,7 @@ class Scanner; 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) {} std::shared_ptr source() const { return m_scanner->charStream(); } @@ -69,17 +62,10 @@ protected: ///@{ ///@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 current token value is not _value, throw exception otherwise advance token. 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); std::string currentLiteral() const; Token advance(); ///@} @@ -91,26 +77,16 @@ protected: /// Creates a @ref ParserError and annotates it with the current position and the /// given @a _description. void parserError(std::string const& _description); - void parserError(SourceLocation const& _location, std::string const& _description); - - /// Creates a @ref ParserWarning and annotates it with the current position and the - /// given @a _description. - void parserWarning(std::string const& _description); /// Creates a @ref ParserError and annotates it with the current position and the /// given @a _description. Throws the FatalError. void fatalParserError(std::string const& _description); - void fatalParserError(SourceLocation const& _location, std::string const& _description); std::shared_ptr m_scanner; /// The reference to the list of errors and warning to add errors/warnings during parsing 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; }; } diff --git a/liblangutil/Scanner.cpp b/liblangutil/Scanner.cpp index a317c79b6..852a92e9b 100644 --- a/liblangutil/Scanner.cpp +++ b/liblangutil/Scanner.cpp @@ -156,13 +156,6 @@ void Scanner::reset() next(); } -void Scanner::setPosition(size_t _offset) -{ - m_char = m_source->setPosition(_offset); - scanToken(); - next(); -} - void Scanner::supportPeriodInIdentifier(bool _value) { m_supportPeriodInIdentifier = _value; diff --git a/liblangutil/Scanner.h b/liblangutil/Scanner.h index 555575129..9a4a170a6 100644 --- a/liblangutil/Scanner.h +++ b/liblangutil/Scanner.h @@ -110,9 +110,6 @@ public: /// @returns the next token and advances input Token next(); - /// Set scanner to a specific offset. This is used in error recovery. - void setPosition(size_t _offset); - ///@{ ///@name Information about the current token diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index c7846c8e2..2910a9d95 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -217,7 +217,7 @@ bool CompilerStack::parse() string const& path = sourcesToParse[i]; Source& source = m_sources[path]; source.scanner->reset(); - source.ast = Parser(m_errorReporter, m_evmVersion, m_parserErrorRecovery).parse(source.scanner); + source.ast = Parser(m_errorReporter, m_evmVersion).parse(source.scanner); if (!source.ast) solAssert(!Error::containsOnlyWarnings(m_errorReporter.errors()), "Parser returned null but did not report error."); else diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index 25718e698..de90553cd 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -132,14 +132,6 @@ public: /// Must be set before parsing. void setOptimiserSettings(OptimiserSettings _settings); - /// 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; - } - /// Set the EVM version used before running compile. /// When called without an argument it will revert to the default version. /// Must be set before parsing. @@ -394,7 +386,6 @@ private: langutil::ErrorList m_errorList; langutil::ErrorReporter m_errorReporter; bool m_metadataLiteralSources = false; - bool m_parserErrorRecovery = false; State m_stackState = Empty; bool m_release = VersionIsRelease; }; diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index d8e0fa4bc..5b5c56523 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -258,75 +258,57 @@ ASTPointer Parser::parseContractDefinition() { RecursionGuard recursionGuard(*this); ASTNodeFactory nodeFactory(*this); - ASTPointer name = nullptr; ASTPointer docString; + if (m_scanner->currentCommentLiteral() != "") + docString = make_shared(m_scanner->currentCommentLiteral()); + ContractDefinition::ContractKind contractKind = parseContractKind(); + ASTPointer name = expectIdentifierToken(); vector> baseContracts; - vector> subNodes; - ContractDefinition::ContractKind contractKind = ContractDefinition::ContractKind::Contract; - try - { - if (m_scanner->currentCommentLiteral() != "") - docString = make_shared(m_scanner->currentCommentLiteral()); - contractKind = parseContractKind(); - name = expectIdentifierToken(); - if (m_scanner->currentToken() == Token::Is) - do - { - m_scanner->next(); - baseContracts.push_back(parseInheritanceSpecifier()); - } - while (m_scanner->currentToken() == Token::Comma); - expectToken(Token::LBrace); - while (true) + if (m_scanner->currentToken() == Token::Is) + do { - Token currentTokenValue = m_scanner->currentToken(); - if (currentTokenValue == Token::RBrace) - break; - else if (currentTokenValue == Token::Function || currentTokenValue == Token::Constructor) - // This can be a function or a state variable of function type (especially - // complicated to distinguish fallback function from function type state variable) - subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable()); - else if (currentTokenValue == Token::Struct) - subNodes.push_back(parseStructDefinition()); - else if (currentTokenValue == Token::Enum) - subNodes.push_back(parseEnumDefinition()); - else if ( - currentTokenValue == Token::Identifier || - currentTokenValue == Token::Mapping || - TokenTraits::isElementaryTypeName(currentTokenValue) - ) - { - VarDeclParserOptions options; - options.isStateVariable = true; - options.allowInitialValue = true; - subNodes.push_back(parseVariableDeclaration(options)); - expectToken(Token::Semicolon); - } - else if (currentTokenValue == Token::Modifier) - subNodes.push_back(parseModifierDefinition()); - else if (currentTokenValue == Token::Event) - subNodes.push_back(parseEventDefinition()); - else if (currentTokenValue == Token::Using) - subNodes.push_back(parseUsingDirective()); - else - fatalParserError(string("Function, variable, struct or modifier declaration expected.")); + m_scanner->next(); + baseContracts.push_back(parseInheritanceSpecifier()); } - } - catch (FatalError const&) + while (m_scanner->currentToken() == Token::Comma); + vector> subNodes; + expectToken(Token::LBrace); + while (true) { - if ( - !m_errorReporter.hasErrors() || - !m_parserErrorRecovery || - m_errorReporter.hasExcessiveErrors() + Token currentTokenValue = m_scanner->currentToken(); + if (currentTokenValue == Token::RBrace) + break; + else if (currentTokenValue == Token::Function || currentTokenValue == Token::Constructor) + // This can be a function or a state variable of function type (especially + // complicated to distinguish fallback function from function type state variable) + subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable()); + else if (currentTokenValue == Token::Struct) + subNodes.push_back(parseStructDefinition()); + else if (currentTokenValue == Token::Enum) + subNodes.push_back(parseEnumDefinition()); + else if ( + currentTokenValue == Token::Identifier || + currentTokenValue == Token::Mapping || + TokenTraits::isElementaryTypeName(currentTokenValue) ) - BOOST_THROW_EXCEPTION(FatalError()); /* Don't try to recover here. */ - m_inParserRecovery = true; + { + VarDeclParserOptions options; + options.isStateVariable = true; + options.allowInitialValue = true; + subNodes.push_back(parseVariableDeclaration(options)); + expectToken(Token::Semicolon); + } + else if (currentTokenValue == Token::Modifier) + subNodes.push_back(parseModifierDefinition()); + else if (currentTokenValue == Token::Event) + subNodes.push_back(parseEventDefinition()); + else if (currentTokenValue == Token::Using) + subNodes.push_back(parseUsingDirective()); + else + fatalParserError(string("Function, variable, struct or modifier declaration expected.")); } nodeFactory.markEndPosition(); - if (m_inParserRecovery) - expectTokenOrConsumeUntil(Token::RBrace, "ContractDefinition"); - else - expectToken(Token::RBrace); + expectToken(Token::RBrace); return nodeFactory.createNode( name, docString, @@ -977,26 +959,10 @@ ASTPointer Parser::parseBlock(ASTPointer const& _docString) ASTNodeFactory nodeFactory(*this); expectToken(Token::LBrace); vector> statements; - try - { - while (m_scanner->currentToken() != Token::RBrace) - statements.push_back(parseStatement()); - nodeFactory.markEndPosition(); - } - catch (FatalError const&) - { - if ( - !m_errorReporter.hasErrors() || - !m_parserErrorRecovery || - m_errorReporter.hasExcessiveErrors() - ) - BOOST_THROW_EXCEPTION(FatalError()); /* Don't try to recover here. */ - m_inParserRecovery = true; - } - if (m_parserErrorRecovery) - expectTokenOrConsumeUntil(Token::RBrace, "Block"); - else - expectToken(Token::RBrace); + while (m_scanner->currentToken() != Token::RBrace) + statements.push_back(parseStatement()); + nodeFactory.markEndPosition(); + expectToken(Token::RBrace); return nodeFactory.createNode(_docString, statements); } @@ -1004,83 +970,67 @@ ASTPointer Parser::parseStatement() { RecursionGuard recursionGuard(*this); ASTPointer docString; + if (m_scanner->currentCommentLiteral() != "") + docString = make_shared(m_scanner->currentCommentLiteral()); ASTPointer statement; - try + switch (m_scanner->currentToken()) { - if (m_scanner->currentCommentLiteral() != "") - docString = make_shared(m_scanner->currentCommentLiteral()); - switch (m_scanner->currentToken()) + case Token::If: + return parseIfStatement(docString); + case Token::While: + return parseWhileStatement(docString); + case Token::Do: + return parseDoWhileStatement(docString); + case Token::For: + return parseForStatement(docString); + case Token::LBrace: + return parseBlock(docString); + // starting from here, all statements must be terminated by a semicolon + case Token::Continue: + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + break; + case Token::Break: + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + break; + case Token::Return: + { + ASTNodeFactory nodeFactory(*this); + ASTPointer expression; + if (m_scanner->next() != Token::Semicolon) { - case Token::If: - return parseIfStatement(docString); - case Token::While: - return parseWhileStatement(docString); - case Token::Do: - return parseDoWhileStatement(docString); - case Token::For: - return parseForStatement(docString); - case Token::LBrace: - return parseBlock(docString); - // starting from here, all statements must be terminated by a semicolon - case Token::Continue: - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - break; - case Token::Break: - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - break; - case Token::Return: - { - ASTNodeFactory nodeFactory(*this); - ASTPointer expression; - if (m_scanner->next() != Token::Semicolon) - { - expression = parseExpression(); - nodeFactory.setEndPositionFromNode(expression); - } - statement = nodeFactory.createNode(docString, expression); - break; - } - case Token::Throw: - { - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - break; - } - case Token::Assembly: - return parseInlineAssembly(docString); - case Token::Emit: - statement = parseEmitStatement(docString); - break; - case Token::Identifier: - if (m_insideModifier && m_scanner->currentLiteral() == "_") - { - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - } - else - statement = parseSimpleStatement(docString); - break; - default: - statement = parseSimpleStatement(docString); - break; + expression = parseExpression(); + nodeFactory.setEndPositionFromNode(expression); } + statement = nodeFactory.createNode(docString, expression); + break; } - catch (FatalError const&) + case Token::Throw: { - if ( - !m_errorReporter.hasErrors() || - !m_parserErrorRecovery || - m_errorReporter.hasExcessiveErrors() - ) - BOOST_THROW_EXCEPTION(FatalError()); /* Don't try to recover here. */ - m_inParserRecovery = true; + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + break; } - if (m_inParserRecovery) - expectTokenOrConsumeUntil(Token::Semicolon, "Statement"); - else - expectToken(Token::Semicolon); + case Token::Assembly: + return parseInlineAssembly(docString); + case Token::Emit: + statement = parseEmitStatement(docString); + break; + case Token::Identifier: + if (m_insideModifier && m_scanner->currentLiteral() == "_") + { + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + } + else + statement = parseSimpleStatement(docString); + break; + default: + statement = parseSimpleStatement(docString); + break; + } + expectToken(Token::Semicolon); return statement; } diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h index cb3d75f51..51b3b5be4 100644 --- a/libsolidity/parsing/Parser.h +++ b/libsolidity/parsing/Parser.h @@ -41,10 +41,9 @@ class Parser: public langutil::ParserBase public: explicit Parser( langutil::ErrorReporter& _errorReporter, - langutil::EVMVersion _evmVersion, - bool _errorRecovery = false + langutil::EVMVersion _evmVersion ): - ParserBase(_errorReporter, _errorRecovery), + ParserBase(_errorReporter), m_evmVersion(_evmVersion) {} diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index a6e2f6025..764376236 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -112,7 +112,6 @@ static string const g_strBinaryRuntime = "bin-runtime"; static string const g_strCombinedJson = "combined-json"; static string const g_strCompactJSON = "compact-format"; static string const g_strContracts = "contracts"; -static string const g_strErrorRecovery = "error-recovery"; static string const g_strEVM = "evm"; static string const g_strEVM15 = "evm15"; static string const g_strEVMVersion = "evm-version"; @@ -164,7 +163,6 @@ static string const g_argBinary = g_strBinary; static string const g_argBinaryRuntime = g_strBinaryRuntime; static string const g_argCombinedJson = g_strCombinedJson; static string const g_argCompactJSON = g_strCompactJSON; -static string const g_argErrorRecovery = g_strErrorRecovery; static string const g_argGas = g_strGas; static string const g_argHelp = g_strHelp; static string const g_argInputFile = g_strInputFile; @@ -691,7 +689,6 @@ Allowed options)", (g_argColor.c_str(), "Force colored output.") (g_argNoColor.c_str(), "Explicitly disable colored output, disabling terminal auto-detection.") (g_argNewReporter.c_str(), "Enables new diagnostics reporter.") - (g_argErrorRecovery.c_str(), "Enables additional parser error recovery.") (g_argIgnoreMissingFiles.c_str(), "Ignore missing files."); po::options_description outputComponents("Output Components"); outputComponents.add_options() @@ -926,8 +923,6 @@ bool CommandLineInterface::processInput() m_compiler->setSources(m_sourceCodes); if (m_args.count(g_argLibraries)) m_compiler->setLibraries(m_libraries); - if (m_args.count(g_argErrorRecovery)) - m_compiler->setParserErrorRecovery(true); m_compiler->setEVMVersion(m_evmVersion); // TODO: Perhaps we should not compile unless requested diff --git a/test/InteractiveTests.h b/test/InteractiveTests.h index 09cc08a29..24a5ad69e 100644 --- a/test/InteractiveTests.h +++ b/test/InteractiveTests.h @@ -56,7 +56,6 @@ Testsuite const g_interactiveTestsuites[] = { {"Yul Interpreter", "libyul", "yulInterpreterTests", false, false, &yul::test::YulInterpreterTest::create}, {"Yul Object Compiler", "libyul", "objectCompiler", false, false, &yul::test::ObjectCompilerTest::create}, {"Syntax", "libsolidity", "syntaxTests", false, false, &SyntaxTest::create}, - {"ErrorRecovery", "libsolidity", "errorRecoveryTests", false, false, &SyntaxTest::createErrorRecovery}, {"Semantic", "libsolidity", "semanticTests", false, true, &SemanticTest::create}, {"JSON AST", "libsolidity", "ASTJSON", false, false, &ASTJSONTest::create}, {"SMT Checker", "libsolidity", "smtCheckerTests", true, false, &SyntaxTest::create}, @@ -67,3 +66,4 @@ Testsuite const g_interactiveTestsuites[] = { } } } + diff --git a/test/liblangutil/CharStream.cpp b/test/liblangutil/CharStream.cpp deleted file mode 100644 index abd3d02db..000000000 --- a/test/liblangutil/CharStream.cpp +++ /dev/null @@ -1,53 +0,0 @@ -/* - This file is part of solidity. - - solidity is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - solidity is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with solidity. If not, see . -*/ -/** - * @author Rocky Bernstein - * @date 2019 - * Unit tests for the CharStream class. - */ - -#include -#include - -#include - -namespace langutil -{ -namespace test -{ - -BOOST_AUTO_TEST_SUITE(CharStreamtest) - -BOOST_AUTO_TEST_CASE(test_fail) -{ - auto const source = std::make_shared("now is the time for testing", "source"); - - BOOST_CHECK('n' == source->get()); - BOOST_CHECK('n' == source->get()); - BOOST_CHECK('o' == source->advanceAndGet()); - BOOST_CHECK('n' == source->rollback(1)); - BOOST_CHECK('w' == source->setPosition(2)); - BOOST_REQUIRE_THROW( - source->setPosition(200), - ::langutil::InternalCompilerError - ); -} - -BOOST_AUTO_TEST_SUITE_END() - -} -} // end namespaces diff --git a/test/libsolidity/AnalysisFramework.cpp b/test/libsolidity/AnalysisFramework.cpp index c94e6c745..faebfc3f5 100644 --- a/test/libsolidity/AnalysisFramework.cpp +++ b/test/libsolidity/AnalysisFramework.cpp @@ -44,15 +44,12 @@ AnalysisFramework::parseAnalyseAndReturnError( string const& _source, bool _reportWarnings, bool _insertVersionPragma, - bool _allowMultipleErrors, - bool _allowRecoveryErrors + bool _allowMultipleErrors ) { compiler().reset(); compiler().setSources({{"", _insertVersionPragma ? "pragma solidity >=0.0;\n" + _source : _source}}); compiler().setEVMVersion(dev::test::Options::get().evmVersion()); - compiler().setParserErrorRecovery(_allowRecoveryErrors); - _allowMultipleErrors = _allowMultipleErrors || _allowRecoveryErrors; if (!compiler().parse()) { BOOST_FAIL("Parsing contract failed in analysis test suite:" + formatErrors()); diff --git a/test/libsolidity/AnalysisFramework.h b/test/libsolidity/AnalysisFramework.h index 3fcb9e74a..cb590674f 100644 --- a/test/libsolidity/AnalysisFramework.h +++ b/test/libsolidity/AnalysisFramework.h @@ -50,8 +50,7 @@ protected: std::string const& _source, bool _reportWarnings = false, bool _insertVersionPragma = true, - bool _allowMultipleErrors = false, - bool _allowRecoveryErrors = false + bool _allowMultipleErrors = false ); virtual ~AnalysisFramework() = default; diff --git a/test/libsolidity/SMTChecker.cpp b/test/libsolidity/SMTChecker.cpp index 4cb7b6667..a08721f17 100644 --- a/test/libsolidity/SMTChecker.cpp +++ b/test/libsolidity/SMTChecker.cpp @@ -42,16 +42,14 @@ protected: std::string const& _source, bool _reportWarnings = false, bool _insertVersionPragma = true, - bool _allowMultipleErrors = false, - bool _allowRecoveryErrors = false + bool _allowMultipleErrors = false ) { return AnalysisFramework::parseAnalyseAndReturnError( "pragma experimental SMTChecker;\n" + _source, _reportWarnings, _insertVersionPragma, - _allowMultipleErrors, - _allowRecoveryErrors + _allowMultipleErrors ); } }; diff --git a/test/libsolidity/SyntaxTest.cpp b/test/libsolidity/SyntaxTest.cpp index 8845cd7e6..61d1d20e6 100644 --- a/test/libsolidity/SyntaxTest.cpp +++ b/test/libsolidity/SyntaxTest.cpp @@ -52,7 +52,7 @@ int parseUnsignedInteger(string::iterator& _it, string::iterator _end) } -SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion, bool _errorRecovery): m_evmVersion(_evmVersion) +SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion): m_evmVersion(_evmVersion) { ifstream file(_filename); if (!file) @@ -67,7 +67,6 @@ SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion m_settings.erase("optimize-yul"); } m_expectations = parseExpectations(file); - m_errorRecovery = _errorRecovery; } TestCase::TestResult SyntaxTest::run(ostream& _stream, string const& _linePrefix, bool _formatted) @@ -76,7 +75,6 @@ TestCase::TestResult SyntaxTest::run(ostream& _stream, string const& _linePrefix compiler().reset(); compiler().setSources({{"", versionPragma + m_source}}); compiler().setEVMVersion(m_evmVersion); - compiler().setParserErrorRecovery(m_errorRecovery); compiler().setOptimiserSettings( m_optimiseYul ? OptimiserSettings::full() : diff --git a/test/libsolidity/SyntaxTest.h b/test/libsolidity/SyntaxTest.h index dcf922de7..37a078107 100644 --- a/test/libsolidity/SyntaxTest.h +++ b/test/libsolidity/SyntaxTest.h @@ -54,14 +54,8 @@ class SyntaxTest: AnalysisFramework, public EVMVersionRestrictedTestCase { public: static std::unique_ptr create(Config const& _config) - { - return std::make_unique(_config.filename, _config.evmVersion, false); - } - static std::unique_ptr createErrorRecovery(Config const& _config) - { - return std::make_unique(_config.filename, _config.evmVersion, true); - } - SyntaxTest(std::string const& _filename, langutil::EVMVersion _evmVersion, bool _errorRecovery = false); + { return std::make_unique(_config.filename, _config.evmVersion); } + SyntaxTest(std::string const& _filename, langutil::EVMVersion _evmVersion); TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool _formatted = false) override; @@ -90,7 +84,6 @@ protected: std::vector m_errorList; bool m_optimiseYul = false; langutil::EVMVersion const m_evmVersion; - bool m_errorRecovery = false; }; } diff --git a/test/libsolidity/errorRecoveryTests/constructor_recovers.sol b/test/libsolidity/errorRecoveryTests/constructor_recovers.sol deleted file mode 100644 index 804bb7283..000000000 --- a/test/libsolidity/errorRecoveryTests/constructor_recovers.sol +++ /dev/null @@ -1,20 +0,0 @@ -pragma solidity >=0.0.0; - -contract Error1 { - constructor() public { - 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. - function five() public view returns(uint) { - return 5; - } -} -// ---- -// ParserError: (95-96): Expected primary expression. -// Warning: (95-96): Recovered in Statement at ';'. diff --git a/test/libsolidity/errorRecoveryTests/contract_recovery.sol b/test/libsolidity/errorRecoveryTests/contract_recovery.sol deleted file mode 100644 index 4047adb31..000000000 --- a/test/libsolidity/errorRecoveryTests/contract_recovery.sol +++ /dev/null @@ -1,7 +0,0 @@ -contract Errort6 { - using foo for ; // missing type name -} - -// ---- -// ParserError: (36-37): Expected type name -// Warning: (59-60): Recovered in ContractDefinition at '}'. diff --git a/test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol b/test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol deleted file mode 100644 index a7e4254b0..000000000 --- a/test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol +++ /dev/null @@ -1,13 +0,0 @@ -pragma solidity >=0.0.0; - -// Example to show why deleting the token at the -// is bad when error recovery is in effect. Here, ")" is missing -// and there is a ";" instead. That causes us to -// not be able to synchronize to ';'. Advance again and -// '}' is deleted and then we can't synchronize the contract. -// There should be an an AST created this contract (with errors). -contract Error2 { - mapping (address => uint balances; // missing ) before "balances" -} -// ---- -// ParserError: (417-425): Expected ')' but got identifier diff --git a/test/libsolidity/errorRecoveryTests/error_to_eos.sol b/test/libsolidity/errorRecoveryTests/error_to_eos.sol deleted file mode 100644 index d396207b0..000000000 --- a/test/libsolidity/errorRecoveryTests/error_to_eos.sol +++ /dev/null @@ -1,23 +0,0 @@ -// Example which where scanning hits EOS, so we reset. -// Here we recover in the contractDefinition. -// There should be an an AST created this contract (with errors). -contract Error2 { - mapping (address => uint balances) // missing ; -} - -// There is no error in this contract -contract SendCoin { - function sendCoin(address receiver, uint amount) public returns(bool sufficient) { - if (balances[msg.sender] < amount) return false; - balances[msg.sender] -= amount; - balances[receiver] += amount; - emit Transfer(msg.sender, receiver, amount); - return true; - } -} - -// ---- -// ParserError: (212-220): Expected ')' but got identifier -// ParserError: (220-221): Expected ';' but got ')' -// ParserError: (220-221): Function, variable, struct or modifier declaration expected. -// Warning: (235-236): Recovered in ContractDefinition at '}'. diff --git a/test/libsolidity/errorRecoveryTests/missing_rhs.sol b/test/libsolidity/errorRecoveryTests/missing_rhs.sol deleted file mode 100644 index 991525b46..000000000 --- a/test/libsolidity/errorRecoveryTests/missing_rhs.sol +++ /dev/null @@ -1,11 +0,0 @@ -pragma solidity >=0.0.0; - -contract Error3 { - constructor() public { - balances[tx.origin] = ; // missing RHS. - } - -} -// ---- -// ParserError: (95-96): Expected primary expression. -// Warning: (95-96): Recovered in Statement at ';'. diff --git a/test/libsolidity/errorRecoveryTests/multiple_errors.sol b/test/libsolidity/errorRecoveryTests/multiple_errors.sol deleted file mode 100644 index 9127ef387..000000000 --- a/test/libsolidity/errorRecoveryTests/multiple_errors.sol +++ /dev/null @@ -1,29 +0,0 @@ -// An example with multiple errors. -// Most are caught by inserting an expected token. -// However some us S C Johnson recovery to -// skip over tokens. - -pragma solidity >=0.0.0; - -contract Error4 { - constructor() public { - balances[tx.origin] = 1 2; // missing operator - } - - function sendCoin(address receiver, uint amount) public returns(bool sufficient) { - if (balances[msg.sender] < amount) return false; - balances[msg.sender] -= amount // Missing ";" - balances[receiver] += amount // Another missing ";" - emit Transfer(msg.sender // truncated line - return true; - } - - -} -// ---- -// ParserError: (249-250): Expected ';' but got 'Number' -// ParserError: (471-479): Expected ';' but got identifier -// ParserError: (529-533): Expected ';' but got 'emit' -// ParserError: (577-583): Expected ',' but got 'return' -// ParserError: (577-583): Expected primary expression. -// Warning: (588-589): Recovered in Statement at ';'.