From b5080860d5f2d141b8fccceaa635378a86c996a8 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 28 Apr 2017 14:27:56 +0100 Subject: Implement switch statement in the assembly parser/printer --- libsolidity/inlineasm/AsmAnalysis.cpp | 32 ++++++++++++++++++++++++++++++++ libsolidity/inlineasm/AsmAnalysis.h | 2 ++ libsolidity/inlineasm/AsmAnalysisInfo.h | 3 ++- libsolidity/inlineasm/AsmCodeGen.cpp | 4 ++++ libsolidity/inlineasm/AsmData.h | 7 ++++++- libsolidity/inlineasm/AsmParser.cpp | 30 ++++++++++++++++++++++++++++++ libsolidity/inlineasm/AsmParser.h | 1 + libsolidity/inlineasm/AsmPrinter.cpp | 14 ++++++++++++++ libsolidity/inlineasm/AsmPrinter.h | 2 ++ libsolidity/inlineasm/AsmScopeFiller.h | 2 ++ 10 files changed, 95 insertions(+), 2 deletions(-) diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index 65b935f2..ecc63372 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -285,6 +285,38 @@ bool AsmAnalyzer::operator()(assembly::FunctionCall const& _funCall) return success; } +bool AsmAnalyzer::operator()(Switch const& _switch) +{ + int const initialStackHeight = m_stackHeight; + if (!boost::apply_visitor(*this, *_switch.expression)) + return false; + expectDeposit(1, initialStackHeight, locationOf(*_switch.expression)); + + map caseNames; + for (auto const& _case: _switch.cases) + { + /// Note: the parser ensures there is only one default case + if (caseNames[_case.name]) + { + m_errors.push_back(make_shared( + Error::Type::DeclarationError, + "Duplicate case defined: " + _case.name, + _case.location + )); + return false; + } + else + caseNames[_case.name] = true; + + if (!(*this)(_case.body)) + return false; + } + + m_stackHeight--; + + return true; +} + bool AsmAnalyzer::operator()(Block const& _block) { bool success = true; diff --git a/libsolidity/inlineasm/AsmAnalysis.h b/libsolidity/inlineasm/AsmAnalysis.h index f09e4b59..9f022b12 100644 --- a/libsolidity/inlineasm/AsmAnalysis.h +++ b/libsolidity/inlineasm/AsmAnalysis.h @@ -47,6 +47,7 @@ struct Identifier; struct StackAssignment; struct FunctionDefinition; struct FunctionCall; +struct Switch; struct Scope; @@ -78,6 +79,7 @@ public: bool operator()(assembly::VariableDeclaration const& _variableDeclaration); bool operator()(assembly::FunctionDefinition const& _functionDefinition); bool operator()(assembly::FunctionCall const& _functionCall); + bool operator()(assembly::Switch const& _switch); bool operator()(assembly::Block const& _block); private: diff --git a/libsolidity/inlineasm/AsmAnalysisInfo.h b/libsolidity/inlineasm/AsmAnalysisInfo.h index d2253c78..18382db0 100644 --- a/libsolidity/inlineasm/AsmAnalysisInfo.h +++ b/libsolidity/inlineasm/AsmAnalysisInfo.h @@ -43,10 +43,11 @@ struct Identifier; struct StackAssignment; struct FunctionDefinition; struct FunctionCall; +struct Switch; struct Scope; -using Statement = boost::variant; +using Statement = boost::variant; struct AsmAnalysisInfo { diff --git a/libsolidity/inlineasm/AsmCodeGen.cpp b/libsolidity/inlineasm/AsmCodeGen.cpp index 53eafc96..5c66b125 100644 --- a/libsolidity/inlineasm/AsmCodeGen.cpp +++ b/libsolidity/inlineasm/AsmCodeGen.cpp @@ -266,6 +266,10 @@ public: CodeTransform(m_state, m_assembly, _block, m_identifierAccess, m_initialStackHeight); checkStackHeight(&_block); } + void operator()(assembly::Switch const&) + { + solAssert(false, "Switch not removed during desugaring phase."); + } void operator()(assembly::FunctionDefinition const&) { solAssert(false, "Function definition not removed during desugaring phase."); diff --git a/libsolidity/inlineasm/AsmData.h b/libsolidity/inlineasm/AsmData.h index 3b4048c3..92ff1c5a 100644 --- a/libsolidity/inlineasm/AsmData.h +++ b/libsolidity/inlineasm/AsmData.h @@ -50,9 +50,10 @@ struct VariableDeclaration; struct FunctionalInstruction; struct FunctionDefinition; struct FunctionCall; +struct Switch; struct Block; -using Statement = boost::variant; +using Statement = boost::variant; /// Direct EVM instruction (except PUSHi and JUMPDEST) struct Instruction { SourceLocation location; solidity::Instruction instruction; }; @@ -77,6 +78,10 @@ struct VariableDeclaration { SourceLocation location; TypedNameList variables; s struct Block { SourceLocation location; std::vector statements; }; /// Function definition ("function f(a, b) -> (d, e) { ... }") struct FunctionDefinition { SourceLocation location; std::string name; TypedNameList arguments; TypedNameList returns; Block body; }; +/// Switch case or default case +struct Case { SourceLocation location; std::string name; Block body; }; +/// Switch statement +struct Switch { SourceLocation location; std::shared_ptr expression; std::vector cases; }; struct LocationExtractor: boost::static_visitor { diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index 530cd726..a3a25a42 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -67,6 +67,20 @@ assembly::Statement Parser::parseStatement() return parseFunctionDefinition(); case Token::LBrace: return parseBlock(); + case Token::Switch: + { + assembly::Switch _switch = createWithLocation(); + m_scanner->next(); + _switch.expression = make_shared(parseExpression()); + while (m_scanner->currentToken() == Token::Case) + _switch.cases.emplace_back(parseCase()); + if (m_scanner->currentToken() == Token::Default) + _switch.cases.emplace_back(parseCase(true)); + if (_switch.cases.size() == 0) + fatalParserError("Switch statement without any cases."); + _switch.location.end = _switch.cases.back().body.location.end; + return _switch; + } case Token::Assign: { if (m_julia) @@ -134,6 +148,22 @@ assembly::Statement Parser::parseStatement() return statement; } +assembly::Case Parser::parseCase(bool _defaultCase) +{ + assembly::Case _case = createWithLocation(); + if (_defaultCase) + expectToken(Token::Default); + else + { + expectToken(Token::Case); + _case.name = expectAsmIdentifier(); + } + expectToken(Token::Colon); + _case.body = parseBlock(); + _case.location.end = _case.body.location.end; + return _case; +} + assembly::Statement Parser::parseExpression() { Statement operation = parseElementaryOperation(true); diff --git a/libsolidity/inlineasm/AsmParser.h b/libsolidity/inlineasm/AsmParser.h index 812762a6..d1d0c1cc 100644 --- a/libsolidity/inlineasm/AsmParser.h +++ b/libsolidity/inlineasm/AsmParser.h @@ -62,6 +62,7 @@ protected: Block parseBlock(); Statement parseStatement(); + Case parseCase(bool _defaultCase = false); /// Parses a functional expression that has to push exactly one stack element Statement parseExpression(); std::map const& instructions(); diff --git a/libsolidity/inlineasm/AsmPrinter.cpp b/libsolidity/inlineasm/AsmPrinter.cpp index 92b12423..92200f84 100644 --- a/libsolidity/inlineasm/AsmPrinter.cpp +++ b/libsolidity/inlineasm/AsmPrinter.cpp @@ -167,6 +167,20 @@ string AsmPrinter::operator()(assembly::FunctionCall const& _functionCall) ")"; } +string AsmPrinter::operator()(Switch const& _switch) +{ + string out = "switch " + boost::apply_visitor(*this, *_switch.expression); + for (auto const& _case: _switch.cases) + { + if (_case.name.empty()) + out += "\ndefault: "; + else + out += "\ncase " + _case.name + ": "; + out += (*this)(_case.body); + } + return out; +} + string AsmPrinter::operator()(Block const& _block) { if (_block.statements.empty()) diff --git a/libsolidity/inlineasm/AsmPrinter.h b/libsolidity/inlineasm/AsmPrinter.h index 423eeefa..b0d7fc09 100644 --- a/libsolidity/inlineasm/AsmPrinter.h +++ b/libsolidity/inlineasm/AsmPrinter.h @@ -40,6 +40,7 @@ struct Assignment; struct VariableDeclaration; struct FunctionDefinition; struct FunctionCall; +struct Switch; struct Block; class AsmPrinter: public boost::static_visitor @@ -57,6 +58,7 @@ public: std::string operator()(assembly::VariableDeclaration const& _variableDeclaration); std::string operator()(assembly::FunctionDefinition const& _functionDefinition); std::string operator()(assembly::FunctionCall const& _functionCall); + std::string operator()(assembly::Switch const& _switch); std::string operator()(assembly::Block const& _block); private: diff --git a/libsolidity/inlineasm/AsmScopeFiller.h b/libsolidity/inlineasm/AsmScopeFiller.h index b1b0833b..0f5a5dd6 100644 --- a/libsolidity/inlineasm/AsmScopeFiller.h +++ b/libsolidity/inlineasm/AsmScopeFiller.h @@ -46,6 +46,7 @@ struct Identifier; struct StackAssignment; struct FunctionDefinition; struct FunctionCall; +struct Switch; struct Scope; @@ -69,6 +70,7 @@ public: bool operator()(assembly::VariableDeclaration const& _variableDeclaration); bool operator()(assembly::FunctionDefinition const& _functionDefinition); bool operator()(assembly::FunctionCall const&) { return true; } + bool operator()(assembly::Switch const&) { return true; }; bool operator()(assembly::Block const& _block); private: -- cgit v1.2.3 From 66eab1caf63f9221a279abf71de953524fe9c2ad Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 17 May 2017 11:21:37 +0100 Subject: Change switch case string to Literal --- libsolidity/inlineasm/AsmAnalysis.cpp | 30 +++++++++++++++++++----------- libsolidity/inlineasm/AsmData.h | 2 +- libsolidity/inlineasm/AsmParser.cpp | 5 ++++- libsolidity/inlineasm/AsmPrinter.cpp | 4 ++-- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index ecc63372..a83a93a8 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -292,21 +292,29 @@ bool AsmAnalyzer::operator()(Switch const& _switch) return false; expectDeposit(1, initialStackHeight, locationOf(*_switch.expression)); - map caseNames; + set> cases; for (auto const& _case: _switch.cases) { - /// Note: the parser ensures there is only one default case - if (caseNames[_case.name]) + if (_case.value) { - m_errors.push_back(make_shared( - Error::Type::DeclarationError, - "Duplicate case defined: " + _case.name, - _case.location - )); - return false; + int const initialStackHeight = m_stackHeight; + if (!(*this)(*_case.value)) + return false; + expectDeposit(1, initialStackHeight, _case.value->location); + m_stackHeight--; + + /// Note: the parser ensures there is only one default case + auto val = make_tuple(_case.value->kind, _case.value->value); + if (!cases.insert(val).second) + { + m_errors.push_back(make_shared( + Error::Type::DeclarationError, + "Duplicate case defined", + _case.location + )); + return false; + } } - else - caseNames[_case.name] = true; if (!(*this)(_case.body)) return false; diff --git a/libsolidity/inlineasm/AsmData.h b/libsolidity/inlineasm/AsmData.h index 92ff1c5a..72afeef1 100644 --- a/libsolidity/inlineasm/AsmData.h +++ b/libsolidity/inlineasm/AsmData.h @@ -79,7 +79,7 @@ struct Block { SourceLocation location; std::vector statements; }; /// Function definition ("function f(a, b) -> (d, e) { ... }") struct FunctionDefinition { SourceLocation location; std::string name; TypedNameList arguments; TypedNameList returns; Block body; }; /// Switch case or default case -struct Case { SourceLocation location; std::string name; Block body; }; +struct Case { SourceLocation location; std::shared_ptr value; Block body; }; /// Switch statement struct Switch { SourceLocation location; std::shared_ptr expression; std::vector cases; }; diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index a3a25a42..11b33218 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -156,7 +156,10 @@ assembly::Case Parser::parseCase(bool _defaultCase) else { expectToken(Token::Case); - _case.name = expectAsmIdentifier(); + assembly::Statement statement = parseElementaryOperation(); + if (statement.type() != typeid(assembly::Literal)) + fatalParserError("Literal expected."); + _case.value = make_shared(std::move(boost::get(statement))); } expectToken(Token::Colon); _case.body = parseBlock(); diff --git a/libsolidity/inlineasm/AsmPrinter.cpp b/libsolidity/inlineasm/AsmPrinter.cpp index 92200f84..1ef9d071 100644 --- a/libsolidity/inlineasm/AsmPrinter.cpp +++ b/libsolidity/inlineasm/AsmPrinter.cpp @@ -172,10 +172,10 @@ string AsmPrinter::operator()(Switch const& _switch) string out = "switch " + boost::apply_visitor(*this, *_switch.expression); for (auto const& _case: _switch.cases) { - if (_case.name.empty()) + if (!_case.value) out += "\ndefault: "; else - out += "\ncase " + _case.name + ": "; + out += "\ncase " + (*this)(*_case.value) + ": "; out += (*this)(_case.body); } return out; -- cgit v1.2.3 From db3d9e0416e7ac69ddf13eb6e2fd4e0a35dd1a63 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 19 May 2017 17:54:55 +0100 Subject: Switch cases are not followed by colon --- libsolidity/inlineasm/AsmParser.cpp | 1 - libsolidity/inlineasm/AsmPrinter.cpp | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index 11b33218..63c08b15 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -161,7 +161,6 @@ assembly::Case Parser::parseCase(bool _defaultCase) fatalParserError("Literal expected."); _case.value = make_shared(std::move(boost::get(statement))); } - expectToken(Token::Colon); _case.body = parseBlock(); _case.location.end = _case.body.location.end; return _case; diff --git a/libsolidity/inlineasm/AsmPrinter.cpp b/libsolidity/inlineasm/AsmPrinter.cpp index 1ef9d071..e282e5e8 100644 --- a/libsolidity/inlineasm/AsmPrinter.cpp +++ b/libsolidity/inlineasm/AsmPrinter.cpp @@ -173,9 +173,9 @@ string AsmPrinter::operator()(Switch const& _switch) for (auto const& _case: _switch.cases) { if (!_case.value) - out += "\ndefault: "; + out += "\ndefault "; else - out += "\ncase " + (*this)(*_case.value) + ": "; + out += "\ncase " + (*this)(*_case.value) + " "; out += (*this)(_case.body); } return out; -- cgit v1.2.3 From e48e84ca2bfdf863a798c88c329796284b2a87cb Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 19 May 2017 18:04:40 +0100 Subject: Check token within parseCase --- libsolidity/inlineasm/AsmParser.cpp | 14 ++++++++------ libsolidity/inlineasm/AsmParser.h | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index 63c08b15..537a39a1 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -75,7 +75,7 @@ assembly::Statement Parser::parseStatement() while (m_scanner->currentToken() == Token::Case) _switch.cases.emplace_back(parseCase()); if (m_scanner->currentToken() == Token::Default) - _switch.cases.emplace_back(parseCase(true)); + _switch.cases.emplace_back(parseCase()); if (_switch.cases.size() == 0) fatalParserError("Switch statement without any cases."); _switch.location.end = _switch.cases.back().body.location.end; @@ -148,19 +148,21 @@ assembly::Statement Parser::parseStatement() return statement; } -assembly::Case Parser::parseCase(bool _defaultCase) +assembly::Case Parser::parseCase() { assembly::Case _case = createWithLocation(); - if (_defaultCase) - expectToken(Token::Default); - else + if (m_scanner->currentToken() == Token::Default) + m_scanner->next(); + else if (m_scanner->currentToken() == Token::Case) { - expectToken(Token::Case); + m_scanner->next(); assembly::Statement statement = parseElementaryOperation(); if (statement.type() != typeid(assembly::Literal)) fatalParserError("Literal expected."); _case.value = make_shared(std::move(boost::get(statement))); } + else + fatalParserError("Case or default case expected."); _case.body = parseBlock(); _case.location.end = _case.body.location.end; return _case; diff --git a/libsolidity/inlineasm/AsmParser.h b/libsolidity/inlineasm/AsmParser.h index d1d0c1cc..138af337 100644 --- a/libsolidity/inlineasm/AsmParser.h +++ b/libsolidity/inlineasm/AsmParser.h @@ -62,7 +62,7 @@ protected: Block parseBlock(); Statement parseStatement(); - Case parseCase(bool _defaultCase = false); + Case parseCase(); /// Parses a functional expression that has to push exactly one stack element Statement parseExpression(); std::map const& instructions(); -- cgit v1.2.3 From ba8a79c600e45618a5cd0bbfc044edbeab1e551c Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 19 May 2017 18:33:25 +0100 Subject: Do not stop on first switch error --- libsolidity/inlineasm/AsmAnalysis.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index a83a93a8..d022ecf6 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -287,9 +287,11 @@ bool AsmAnalyzer::operator()(assembly::FunctionCall const& _funCall) bool AsmAnalyzer::operator()(Switch const& _switch) { + bool success = true; + int const initialStackHeight = m_stackHeight; if (!boost::apply_visitor(*this, *_switch.expression)) - return false; + success = false; expectDeposit(1, initialStackHeight, locationOf(*_switch.expression)); set> cases; @@ -299,7 +301,7 @@ bool AsmAnalyzer::operator()(Switch const& _switch) { int const initialStackHeight = m_stackHeight; if (!(*this)(*_case.value)) - return false; + success = false; expectDeposit(1, initialStackHeight, _case.value->location); m_stackHeight--; @@ -312,17 +314,17 @@ bool AsmAnalyzer::operator()(Switch const& _switch) "Duplicate case defined", _case.location )); - return false; + success = false; } } if (!(*this)(_case.body)) - return false; + success = false; } m_stackHeight--; - return true; + return success; } bool AsmAnalyzer::operator()(Block const& _block) -- cgit v1.2.3 From 0c5c1ca911de534a7fbfb8cfd6559115dd43caef Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 23 May 2017 12:26:01 +0100 Subject: Add tests --- test/libsolidity/InlineAssembly.cpp | 52 +++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index 61892761..18ee9553 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -223,6 +223,53 @@ BOOST_AUTO_TEST_CASE(variable_use_before_decl) CHECK_PARSE_ERROR("{ let x := mul(2, x) }", DeclarationError, "Variable x used before it was declared."); } +BOOST_AUTO_TEST_CASE(switch_statement) +{ + BOOST_CHECK(successParse("{ switch 42 default {} }")); + BOOST_CHECK(successParse("{ switch 42 case 1 {} }")); + BOOST_CHECK(successParse("{ switch 42 case 1 {} case 2 {} }")); + BOOST_CHECK(successParse("{ switch 42 case 1 {} default {} }")); + BOOST_CHECK(successParse("{ switch 42 case 1 {} case 2 {} default {} }")); + BOOST_CHECK(successParse("{ 1 2 switch mul case 1 {} case 2 {} default {} }")); + BOOST_CHECK(successParse("{ switch mul(1, 2) case 1 {} case 2 {} default {} }")); + BOOST_CHECK(successParse("{ function f() -> x {} switch f() case 1 {} case 2 {} default {} }")); +} + +BOOST_AUTO_TEST_CASE(switch_no_cases) +{ + CHECK_PARSE_ERROR("{ switch 42 }", ParserError, "Switch statement without any cases."); +} + +BOOST_AUTO_TEST_CASE(switch_duplicate_case) +{ + CHECK_PARSE_ERROR("{ switch 42 case 1 {} case 1 {} default {} }", DeclarationError, "Duplicate case defined"); +} + +BOOST_AUTO_TEST_CASE(switch_invalid_expression) +{ + CHECK_PARSE_ERROR("{ switch {} default {} }", ParserError, "Expected elementary inline assembly operation."); +} + +BOOST_AUTO_TEST_CASE(switch_default_before_case) +{ + CHECK_PARSE_ERROR("{ switch 42 default {} case 1 {} }", ParserError, "Expected elementary inline assembly operation."); +} + +BOOST_AUTO_TEST_CASE(switch_duplicate_default_case) +{ + CHECK_PARSE_ERROR("{ switch 42 default {} default {} }", ParserError, "Expected elementary inline assembly operation."); +} + +BOOST_AUTO_TEST_CASE(switch_invalid_case) +{ + CHECK_PARSE_ERROR("{ switch 42 case mul(1, 2) {} case 2 {} default {} }", ParserError, "Literal expected."); +} + +BOOST_AUTO_TEST_CASE(switch_invalid_body) +{ + CHECK_PARSE_ERROR("{ switch 42 case 1 mul case 2 {} default {} }", ParserError, "Expected token LBrace got 'Identifier'"); +} + BOOST_AUTO_TEST_CASE(blocks) { BOOST_CHECK(successParse("{ let x := 7 { let y := 3 } { let z := 2 } }")); @@ -336,6 +383,11 @@ BOOST_AUTO_TEST_CASE(print_string_literal_unicode) parsePrintCompare(parsed); } +BOOST_AUTO_TEST_CASE(print_switch) +{ + parsePrintCompare("{\n switch 42\n case 1 {\n }\n case 2 {\n }\n default {\n }\n}"); +} + BOOST_AUTO_TEST_CASE(function_definitions_multiple_args) { parsePrintCompare("{\n function f(a, d)\n {\n mstore(a, d)\n }\n function g(a, d) -> x, y\n {\n }\n}"); -- cgit v1.2.3 From d745dd6542d488432453035763211fdad78c3a06 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Tue, 23 May 2017 20:51:33 +0100 Subject: Visit case bodies in scope filler --- libsolidity/inlineasm/AsmScopeFiller.cpp | 9 +++++++++ libsolidity/inlineasm/AsmScopeFiller.h | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/libsolidity/inlineasm/AsmScopeFiller.cpp b/libsolidity/inlineasm/AsmScopeFiller.cpp index 05b1b211..7eb6a9ed 100644 --- a/libsolidity/inlineasm/AsmScopeFiller.cpp +++ b/libsolidity/inlineasm/AsmScopeFiller.cpp @@ -97,6 +97,15 @@ bool ScopeFiller::operator()(assembly::FunctionDefinition const& _funDef) return success; } +bool ScopeFiller::operator()(Switch const& _switch) +{ + bool success = true; + for (auto const& _case: _switch.cases) + if (!(*this)(_case.body)) + success = false; + return success; +} + bool ScopeFiller::operator()(Block const& _block) { bool success = true; diff --git a/libsolidity/inlineasm/AsmScopeFiller.h b/libsolidity/inlineasm/AsmScopeFiller.h index 0f5a5dd6..c7179b3b 100644 --- a/libsolidity/inlineasm/AsmScopeFiller.h +++ b/libsolidity/inlineasm/AsmScopeFiller.h @@ -70,7 +70,7 @@ public: bool operator()(assembly::VariableDeclaration const& _variableDeclaration); bool operator()(assembly::FunctionDefinition const& _functionDefinition); bool operator()(assembly::FunctionCall const&) { return true; } - bool operator()(assembly::Switch const&) { return true; }; + bool operator()(assembly::Switch const& _switch); bool operator()(assembly::Block const& _block); private: -- cgit v1.2.3 From c64bd3378427b46b0ae37e5d39e8ca6586d697c2 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 24 May 2017 14:33:10 +0100 Subject: Disallow instructions as a switch expression --- libsolidity/inlineasm/AsmParser.cpp | 2 ++ test/libsolidity/InlineAssembly.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index 537a39a1..847735f4 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -72,6 +72,8 @@ assembly::Statement Parser::parseStatement() assembly::Switch _switch = createWithLocation(); m_scanner->next(); _switch.expression = make_shared(parseExpression()); + if (_switch.expression->type() == typeid(assembly::Instruction)) + fatalParserError("Instructions are not supported as expressions for switch."); while (m_scanner->currentToken() == Token::Case) _switch.cases.emplace_back(parseCase()); if (m_scanner->currentToken() == Token::Default) diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index 18ee9553..f695e1d1 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -230,7 +230,6 @@ BOOST_AUTO_TEST_CASE(switch_statement) BOOST_CHECK(successParse("{ switch 42 case 1 {} case 2 {} }")); BOOST_CHECK(successParse("{ switch 42 case 1 {} default {} }")); BOOST_CHECK(successParse("{ switch 42 case 1 {} case 2 {} default {} }")); - BOOST_CHECK(successParse("{ 1 2 switch mul case 1 {} case 2 {} default {} }")); BOOST_CHECK(successParse("{ switch mul(1, 2) case 1 {} case 2 {} default {} }")); BOOST_CHECK(successParse("{ function f() -> x {} switch f() case 1 {} case 2 {} default {} }")); } @@ -248,6 +247,7 @@ BOOST_AUTO_TEST_CASE(switch_duplicate_case) BOOST_AUTO_TEST_CASE(switch_invalid_expression) { CHECK_PARSE_ERROR("{ switch {} default {} }", ParserError, "Expected elementary inline assembly operation."); + CHECK_PARSE_ERROR("{ 1 2 switch mul default {} }", ParserError, "Instructions are not supported as expressions for switch."); } BOOST_AUTO_TEST_CASE(switch_default_before_case) -- cgit v1.2.3 From 05fcf1989ca619d197d22d3acab79b25ef7aa695 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 26 May 2017 01:49:32 +0100 Subject: Better error messages for invalid switch cases --- libsolidity/inlineasm/AsmParser.cpp | 4 ++++ test/libsolidity/InlineAssembly.cpp | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp index 847735f4..ccc735f7 100644 --- a/libsolidity/inlineasm/AsmParser.cpp +++ b/libsolidity/inlineasm/AsmParser.cpp @@ -78,6 +78,10 @@ assembly::Statement Parser::parseStatement() _switch.cases.emplace_back(parseCase()); if (m_scanner->currentToken() == Token::Default) _switch.cases.emplace_back(parseCase()); + if (m_scanner->currentToken() == Token::Default) + fatalParserError("Only one default case allowed."); + else if (m_scanner->currentToken() == Token::Case) + fatalParserError("Case not allowed after default case."); if (_switch.cases.size() == 0) fatalParserError("Switch statement without any cases."); _switch.location.end = _switch.cases.back().body.location.end; diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index f695e1d1..39cec731 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -252,12 +252,12 @@ BOOST_AUTO_TEST_CASE(switch_invalid_expression) BOOST_AUTO_TEST_CASE(switch_default_before_case) { - CHECK_PARSE_ERROR("{ switch 42 default {} case 1 {} }", ParserError, "Expected elementary inline assembly operation."); + CHECK_PARSE_ERROR("{ switch 42 default {} case 1 {} }", ParserError, "Case not allowed after default case."); } BOOST_AUTO_TEST_CASE(switch_duplicate_default_case) { - CHECK_PARSE_ERROR("{ switch 42 default {} default {} }", ParserError, "Expected elementary inline assembly operation."); + CHECK_PARSE_ERROR("{ switch 42 default {} default {} }", ParserError, "Only one default case allowed."); } BOOST_AUTO_TEST_CASE(switch_invalid_case) -- cgit v1.2.3