diff --git a/Changelog.md b/Changelog.md index cc216d0ba..5da6cb372 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,7 @@ Language Features: Compiler Features: + * Yul: Raise warning for switch statements that only have a default and no other cases. Bugfixes: diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 30a4cb2a3..eee3beacb 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -361,7 +361,7 @@ void AsmAnalyzer::operator()(Switch const& _switch) if (_switch.cases.size() == 1 && !_switch.cases[0].value) m_errorReporter.warning( - 1878_error, + 9592_error, _switch.location, "\"switch\" statement with only a default case." ); diff --git a/test/libsolidity/ASTJSON/assembly/switch_default.json b/test/libsolidity/ASTJSON/assembly/switch_default.json index 2f15a82c5..3a6c71864 100644 --- a/test/libsolidity/ASTJSON/assembly/switch_default.json +++ b/test/libsolidity/ASTJSON/assembly/switch_default.json @@ -33,14 +33,14 @@ { "id": 4, "nodeType": "Block", - "src": "42:48:1", + "src": "42:58:1", "statements": [ { "AST": { "nodeType": "YulBlock", - "src": "61:23:1", + "src": "61:33:1", "statements": [ { @@ -50,11 +50,29 @@ "body": { "nodeType": "YulBlock", - "src": "80:2:1", + "src": "79:2:1", "statements": [] }, "nodeType": "YulCase", - "src": "72:10:1", + "src": "72:9:1", + "value": + { + "kind": "number", + "nodeType": "YulLiteral", + "src": "77:1:1", + "type": "", + "value": "0" + } + }, + { + "body": + { + "nodeType": "YulBlock", + "src": "90:2:1", + "statements": [] + }, + "nodeType": "YulCase", + "src": "82:10:1", "value": "default" } ], @@ -67,7 +85,7 @@ "value": "0" }, "nodeType": "YulSwitch", - "src": "63:19:1" + "src": "63:29:1" } ] }, @@ -75,7 +93,7 @@ "externalReferences": [], "id": 3, "nodeType": "InlineAssembly", - "src": "52:32:1" + "src": "52:42:1" } ] }, @@ -103,15 +121,15 @@ "src": "42:0:1" }, "scope": 6, - "src": "17:73:1", + "src": "17:83:1", "stateMutability": "view", "virtual": false, "visibility": "public" } ], "scope": 7, - "src": "0:92:1" + "src": "0:102:1" } ], - "src": "0:93:1" + "src": "0:103:1" } diff --git a/test/libsolidity/ASTJSON/assembly/switch_default.sol b/test/libsolidity/ASTJSON/assembly/switch_default.sol index 8bfba2c11..1f61896fe 100644 --- a/test/libsolidity/ASTJSON/assembly/switch_default.sol +++ b/test/libsolidity/ASTJSON/assembly/switch_default.sol @@ -1,6 +1,6 @@ contract C { function g() view public { - assembly { switch 0 default {} } + assembly { switch 0 case 0 {} default {} } } } diff --git a/test/libsolidity/ASTJSON/assembly/switch_default_legacy.json b/test/libsolidity/ASTJSON/assembly/switch_default_legacy.json index 18e7c860c..3b79ccf44 100644 --- a/test/libsolidity/ASTJSON/assembly/switch_default_legacy.json +++ b/test/libsolidity/ASTJSON/assembly/switch_default_legacy.json @@ -95,30 +95,30 @@ [ null ], - "operations": "{\n switch 0\n default { }\n}" + "operations": "{\n switch 0\n case 0 { }\n default { }\n}" }, "children": [], "id": 3, "name": "InlineAssembly", - "src": "52:32:1" + "src": "52:42:1" } ], "id": 4, "name": "Block", - "src": "42:48:1" + "src": "42:58:1" } ], "id": 5, "name": "FunctionDefinition", - "src": "17:73:1" + "src": "17:83:1" } ], "id": 6, "name": "ContractDefinition", - "src": "0:92:1" + "src": "0:102:1" } ], "id": 7, "name": "SourceUnit", - "src": "0:93:1" + "src": "0:103:1" } diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index 2a1b72e50..1d61484a9 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -314,9 +314,17 @@ BOOST_AUTO_TEST_CASE(switch_duplicate_case) BOOST_AUTO_TEST_CASE(switch_invalid_expression) { - CHECK_PARSE_ERROR("{ switch {} default {} }", ParserError, "Literal or identifier expected."); - CHECK_PARSE_ERROR("{ switch mload default {} }", ParserError, "Expected '(' but got reserved keyword 'default'"); - CHECK_PARSE_ERROR("{ switch mstore(1, 1) default {} }", TypeError, "Expected expression to evaluate to one value, but got 0 values instead."); + CHECK_PARSE_ERROR("{ switch {} case 1 {} default {} }", ParserError, "Literal or identifier expected."); + CHECK_PARSE_ERROR( + "{ switch mload case 1 {} default {} }", + ParserError, + "Expected '(' but got reserved keyword 'case'" + ); + CHECK_PARSE_ERROR( + "{ switch mstore(1, 1) case 1 {} default {} }", + TypeError, + "Expected expression to evaluate to one value, but got 0 values instead." + ); } BOOST_AUTO_TEST_CASE(switch_default_before_case) diff --git a/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/assembly/switch_declaration_fine.sol b/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/assembly/switch_declaration_fine.sol index d1d09fc98..a32599d7b 100644 --- a/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/assembly/switch_declaration_fine.sol +++ b/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/assembly/switch_declaration_fine.sol @@ -1,15 +1,7 @@ contract C { struct S { bool f; } S s; - function f(uint256 a) internal pure { - S storage c; - assembly { - switch a - default { c_slot := s_slot } - } - c; - } - function g(bool flag) internal pure { + function f(bool flag) internal pure { S storage c; assembly { switch flag @@ -18,7 +10,7 @@ contract C { } c; } - function h(uint256 a) internal pure { + function g(uint256 a) internal pure { S storage c; assembly { switch a @@ -29,4 +21,3 @@ contract C { } } // ---- -// Warning: (141-190): "switch" statement with only a default case. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_fine.sol index dfe6287a7..8d0b564fc 100644 --- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_fine.sol +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_fine.sol @@ -1,20 +1,14 @@ contract C { struct S { bool f; } S s; - function f(uint256 a) internal pure returns (S storage c) { - assembly { - switch a - default { c_slot := s_slot } - } - } - function g(bool flag) internal pure returns (S storage c) { + function f(bool flag) internal pure returns (S storage c) { assembly { switch flag case 0 { c_slot := s_slot } default { c_slot := s_slot } } } - function h(uint256 a) internal pure returns (S storage c) { + function g(uint256 a) internal pure returns (S storage c) { assembly { switch a case 0 { revert(0, 0) } @@ -23,4 +17,3 @@ contract C { } } // ---- -// Warning: (142-191): "switch" statement with only a default case. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_only_default_warn.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_only_default_warn.sol new file mode 100644 index 000000000..fff82c20d --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_only_default_warn.sol @@ -0,0 +1,12 @@ +contract C { + struct S { bool f; } + S s; + function f(uint256 a) internal pure returns (S storage c) { + assembly { + switch a + default { c_slot := s_slot } + } + } +} +// ---- +// Warning: (142-195): "switch" statement with only a default case. diff --git a/test/libyul/Common.cpp b/test/libyul/Common.cpp index ee6d77dab..57ee7ffa6 100644 --- a/test/libyul/Common.cpp +++ b/test/libyul/Common.cpp @@ -87,12 +87,12 @@ pair, shared_ptr> yul::test::parse( shared_ptr parserResult = yul::ObjectParser(errorReporter, _dialect).parse(scanner, false); if (!parserResult) return {}; - if (!parserResult->code || !errorReporter.errors().empty()) + if (!parserResult->code || errorReporter.hasErrors()) return {}; shared_ptr analysisInfo = make_shared(); AsmAnalyzer analyzer(*analysisInfo, errorReporter, _dialect, {}, parserResult->dataNames()); // TODO this should be done recursively. - if (!analyzer.analyze(*parserResult->code) || !errorReporter.errors().empty()) + if (!analyzer.analyze(*parserResult->code) || errorReporter.hasErrors()) return {}; return {std::move(parserResult->code), std::move(analysisInfo)}; } diff --git a/test/libyul/Metrics.cpp b/test/libyul/Metrics.cpp index 2d1cc1c4e..7ae9e44f5 100644 --- a/test/libyul/Metrics.cpp +++ b/test/libyul/Metrics.cpp @@ -332,7 +332,7 @@ BOOST_FIXTURE_TEST_CASE(if_statement_custom_weights, CustomWeightFixture) BOOST_AUTO_TEST_CASE(switch_statement_tiny) { BOOST_CHECK_EQUAL(codeSize( - "{ switch calldatasize() default {} }" + "{ switch calldatasize() case 0 {} }" ), 4); } diff --git a/test/libyul/YulOptimizerTest.cpp b/test/libyul/YulOptimizerTest.cpp index bda7c6514..a4507e180 100644 --- a/test/libyul/YulOptimizerTest.cpp +++ b/test/libyul/YulOptimizerTest.cpp @@ -362,7 +362,7 @@ bool YulOptimizerTest::parse(ostream& _stream, string const& _linePrefix, bool c ErrorList errors; soltestAssert(m_dialect, ""); std::tie(m_ast, m_analysisInfo) = yul::test::parse(m_source, *m_dialect, errors); - if (!m_ast || !m_analysisInfo || !errors.empty()) + if (!m_ast || !m_analysisInfo || !Error::containsOnlyWarnings(errors)) { AnsiColorized(_stream, _formatted, {formatting::BOLD, formatting::RED}) << _linePrefix << "Error parsing source." << endl; printErrors(_stream, errors);