From 5d93c492fe8d499a1d6f1f752023ec8697f35b4d Mon Sep 17 00:00:00 2001 From: mingchuan Date: Thu, 18 Apr 2019 18:51:28 +0800 Subject: [PATCH] [Yul] More accurate error messages for break/continue --- libyul/AsmParser.cpp | 78 +++++++++++++++++++++++----------------- libyul/AsmParser.h | 13 +++++-- test/libyul/Parser.cpp | 82 ++++++++++++++++++++++++++++++++++++------ 3 files changed, 128 insertions(+), 45 deletions(-) diff --git a/libyul/AsmParser.cpp b/libyul/AsmParser.cpp index a7787ebe4..b8c786ac6 100644 --- a/libyul/AsmParser.cpp +++ b/libyul/AsmParser.cpp @@ -105,31 +105,19 @@ Statement Parser::parseStatement() case Token::For: return parseForLoop(); case Token::Break: - if (m_insideForLoopBody) - { - auto stmt = Statement{ createWithLocation() }; - m_scanner->next(); - return stmt; - } - else - { - m_errorReporter.syntaxError(location(), "Keyword break outside for-loop body is not allowed."); - m_scanner->next(); - return {}; - } + { + Statement stmt{createWithLocation()}; + checkBreakContinuePosition("break"); + m_scanner->next(); + return stmt; + } case Token::Continue: - if (m_insideForLoopBody) - { - auto stmt = Statement{ createWithLocation() }; - m_scanner->next(); - return stmt; - } - else - { - m_errorReporter.syntaxError(location(), "Keyword continue outside for-loop body is not allowed."); - m_scanner->next(); - return {}; - } + { + Statement stmt{createWithLocation()}; + checkBreakContinuePosition("continue"); + m_scanner->next(); + return stmt; + } case Token::Assign: { if (m_dialect->flavour != AsmFlavour::Loose) @@ -269,20 +257,24 @@ Case Parser::parseCase() ForLoop Parser::parseForLoop() { - bool outerForLoopBody = m_insideForLoopBody; - m_insideForLoopBody = false; - RecursionGuard recursionGuard(*this); + + ForLoopComponent outerForLoopComponent = m_currentForLoopComponent; + ForLoop forLoop = createWithLocation(); expectToken(Token::For); + m_currentForLoopComponent = ForLoopComponent::ForLoopPre; forLoop.pre = parseBlock(); + m_currentForLoopComponent = ForLoopComponent::None; forLoop.condition = make_unique(parseExpression()); + m_currentForLoopComponent = ForLoopComponent::ForLoopPost; forLoop.post = parseBlock(); - - m_insideForLoopBody = true; + m_currentForLoopComponent = ForLoopComponent::ForLoopBody; forLoop.body = parseBlock(); - m_insideForLoopBody = outerForLoopBody; forLoop.location.end = forLoop.body.location.end; + + m_currentForLoopComponent = outerForLoopComponent; + return forLoop; } @@ -487,9 +479,9 @@ VariableDeclaration Parser::parseVariableDeclaration() FunctionDefinition Parser::parseFunctionDefinition() { RecursionGuard recursionGuard(*this); - auto outerForLoopBody = m_insideForLoopBody; - m_insideForLoopBody = false; - ScopeGuard restoreInsideForLoopBody{[&]() { m_insideForLoopBody = outerForLoopBody; }}; + ForLoopComponent outerForLoopComponent = m_currentForLoopComponent; + m_currentForLoopComponent = ForLoopComponent::None; + FunctionDefinition funDef = createWithLocation(); expectToken(Token::Function); funDef.name = expectAsmIdentifier(); @@ -516,6 +508,8 @@ FunctionDefinition Parser::parseFunctionDefinition() } funDef.body = parseBlock(); funDef.location.end = funDef.body.location.end; + + m_currentForLoopComponent = outerForLoopComponent; return funDef; } @@ -642,6 +636,24 @@ YulString Parser::expectAsmIdentifier() return name; } +void Parser::checkBreakContinuePosition(string const& _which) +{ + switch (m_currentForLoopComponent) + { + case ForLoopComponent::None: + m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" needs to be inside a for-loop body."); + break; + case ForLoopComponent::ForLoopPre: + m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" in for-loop init block is not allowed."); + break; + case ForLoopComponent::ForLoopPost: + m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" in for-loop post block is not allowed."); + break; + case ForLoopComponent::ForLoopBody: + break; + } +} + bool Parser::isValidNumberLiteral(string const& _literal) { try diff --git a/libyul/AsmParser.h b/libyul/AsmParser.h index 5985f1f93..9fec2bb6c 100644 --- a/libyul/AsmParser.h +++ b/libyul/AsmParser.h @@ -32,14 +32,20 @@ #include #include + namespace yul { class Parser: public langutil::ParserBase { public: + enum class ForLoopComponent + { + None, ForLoopPre, ForLoopPost, ForLoopBody + }; + explicit Parser(langutil::ErrorReporter& _errorReporter, std::shared_ptr _dialect): - ParserBase(_errorReporter), m_dialect(std::move(_dialect)), m_insideForLoopBody{false} {} + ParserBase(_errorReporter), m_dialect(std::move(_dialect)) {} /// Parses an inline assembly block starting with `{` and ending with `}`. /// @param _reuseScanner if true, do check for end of input after the `}`. @@ -83,11 +89,14 @@ protected: TypedName parseTypedName(); YulString expectAsmIdentifier(); + /// Reports an error if we are currently not inside the body part of a for loop. + void checkBreakContinuePosition(std::string const& _which); + static bool isValidNumberLiteral(std::string const& _literal); private: std::shared_ptr m_dialect; - bool m_insideForLoopBody; + ForLoopComponent m_currentForLoopComponent = ForLoopComponent::None; }; } diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index 6b09cc178..62d56308b 100644 --- a/test/libyul/Parser.cpp +++ b/test/libyul/Parser.cpp @@ -295,6 +295,28 @@ BOOST_AUTO_TEST_CASE(if_statement) BOOST_CHECK(successParse("{ function f() -> x:bool {} if f() { let b:bool := f() } }")); } +BOOST_AUTO_TEST_CASE(break_outside_of_for_loop) +{ + auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople()); + CHECK_ERROR_DIALECT( + "{ let x if x { break } }", + SyntaxError, + "Keyword \"break\" needs to be inside a for-loop body.", + dialect + ); +} + +BOOST_AUTO_TEST_CASE(continue_outside_of_for_loop) +{ + auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople()); + CHECK_ERROR_DIALECT( + "{ let x if x { continue } }", + SyntaxError, + "Keyword \"continue\" needs to be inside a for-loop body.", + dialect + ); +} + BOOST_AUTO_TEST_CASE(for_statement) { auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople()); @@ -313,8 +335,9 @@ BOOST_AUTO_TEST_CASE(for_statement_break_init) CHECK_ERROR_DIALECT( "{ for {let i := 0 break} iszero(eq(i, 10)) {i := add(i, 1)} {} }", SyntaxError, - "Keyword break outside for-loop body is not allowed.", - dialect); + "Keyword \"break\" in for-loop init block is not allowed.", + dialect + ); } BOOST_AUTO_TEST_CASE(for_statement_break_post) @@ -323,8 +346,9 @@ BOOST_AUTO_TEST_CASE(for_statement_break_post) CHECK_ERROR_DIALECT( "{ for {let i := 0} iszero(eq(i, 10)) {i := add(i, 1) break} {} }", SyntaxError, - "Keyword break outside for-loop body is not allowed.", - dialect); + "Keyword \"break\" in for-loop post block is not allowed.", + dialect + ); } BOOST_AUTO_TEST_CASE(for_statement_nested_break) @@ -333,8 +357,9 @@ BOOST_AUTO_TEST_CASE(for_statement_nested_break) CHECK_ERROR_DIALECT( "{ for {let i := 0} iszero(eq(i, 10)) {} { function f() { break } } }", SyntaxError, - "Keyword break outside for-loop body is not allowed.", - dialect); + "Keyword \"break\" needs to be inside a for-loop body.", + dialect + ); } BOOST_AUTO_TEST_CASE(for_statement_continue) @@ -349,8 +374,9 @@ BOOST_AUTO_TEST_CASE(for_statement_continue_fail_init) CHECK_ERROR_DIALECT( "{ for {let i := 0 continue} iszero(eq(i, 10)) {i := add(i, 1)} {} }", SyntaxError, - "Keyword continue outside for-loop body is not allowed.", - dialect); + "Keyword \"continue\" in for-loop init block is not allowed.", + dialect + ); } BOOST_AUTO_TEST_CASE(for_statement_continue_fail_post) @@ -359,8 +385,44 @@ BOOST_AUTO_TEST_CASE(for_statement_continue_fail_post) CHECK_ERROR_DIALECT( "{ for {let i := 0} iszero(eq(i, 10)) {i := add(i, 1) continue} {} }", SyntaxError, - "Keyword continue outside for-loop body is not allowed.", - dialect); + "Keyword \"continue\" in for-loop post block is not allowed.", + dialect + ); +} + +BOOST_AUTO_TEST_CASE(for_statement_continue_nested_init_in_body) +{ + auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion::constantinople()); + CHECK_ERROR_DIALECT( + "{ for {} 1 {} {let x for { continue } x {} {}} }", + SyntaxError, + "Keyword \"continue\" in for-loop init block is not allowed.", + dialect + ); +} + +BOOST_AUTO_TEST_CASE(for_statement_continue_nested_body_in_init) +{ + auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{}); + BOOST_CHECK(successParse("{ for {let x for {} x {} { continue }} 1 {} {} }", dialect)); +} + +BOOST_AUTO_TEST_CASE(for_statement_break_nested_body_in_init) +{ + auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{}); + BOOST_CHECK(successParse("{ for {let x for {} x {} { break }} 1 {} {} }", dialect)); +} + +BOOST_AUTO_TEST_CASE(for_statement_continue_nested_body_in_post) +{ + auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{}); + BOOST_CHECK(successParse("{ for {} 1 {let x for {} x {} { continue }} {} }", dialect)); +} + +BOOST_AUTO_TEST_CASE(for_statement_break_nested_body_in_post) +{ + auto dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{}); + BOOST_CHECK(successParse("{ for {} 1 {let x for {} x {} { break }} {} }", dialect)); } BOOST_AUTO_TEST_CASE(if_statement_invalid)