Merge pull request #8930 from ethereum/switch-single-default-warn

Warn on YUL switch statement with only default statement

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:

libyul/AsmAnalysis.cpp

@@ -359,6 +359,13 @@ void AsmAnalyzer::operator()(Switch const& _switch)
 {
 	yulAssert(_switch.expression, "");
 
+	if (_switch.cases.size() == 1 && !_switch.cases[0].value)
+		m_errorReporter.warning(
+			9592_error,
+			_switch.location,
+			"\"switch\" statement with only a default case."
+		);
+
 	YulString valueType = expectExpression(*_switch.expression);
 
 	set<u256> cases;

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"
 }

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 {} }
     }
 }
 

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"
 }

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 {} case 1 {} default {} }", ParserError, "Literal or identifier expected.");
-	CHECK_PARSE_ERROR("{ switch mload default {} }", ParserError, "Expected '(' but got reserved keyword 'default'");
+	CHECK_PARSE_ERROR(
-	CHECK_PARSE_ERROR("{ switch mstore(1, 1) default {} }", TypeError, "Expected expression to evaluate to one value, but got 0 values instead.");
+		"{ 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)

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 {
+    function f(bool flag) internal pure {
-        S storage c;
-        assembly {
-            switch a
-            default { c_slot := s_slot }
-        }
-        c;
-    }
-    function g(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

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) {
+    function f(bool flag) internal pure returns (S storage c) {
-        assembly {
-            switch a
-            default { c_slot := s_slot }
-        }
-    }
-    function g(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) }

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.

test/libyul/Common.cpp

@@ -87,12 +87,12 @@ pair<shared_ptr<Block>, shared_ptr<yul::AsmAnalysisInfo>> yul::test::parse(
 	shared_ptr<Object> 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<AsmAnalysisInfo> analysisInfo = make_shared<AsmAnalysisInfo>();
 	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)};
 }

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);
 }
 

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);