Merge pull request #11859 from ethereum/revert-making-error-a-keyword

Revert making `error` a keyword

Changelog.md

@@ -1,7 +1,6 @@
 ### 0.9.0 (unreleased)
 
 Breaking changes:
- * `error` is now a keyword that can only be used for defining errors.
  * Inline Assembly: Consider functions, function parameters and return variables for shadowing checks.
 
 

docs/090-breaking-changes.rst

@@ -15,7 +15,7 @@ Silent Changes of the Semantics
 New Restrictions
 ================
 
-- `error` is now a keyword and cannot be used as identifier anymore.
+...
 
 Interface Changes
 =================

docs/grammar/SolidityLexer.g4

@@ -29,7 +29,7 @@ Do: 'do';
 Else: 'else';
 Emit: 'emit';
 Enum: 'enum';
-Error: 'error';
+Error: 'error'; // not a real keyword
 Revert: 'revert'; // not a real keyword
 Event: 'event';
 External: 'external';

docs/grammar/SolidityParser.g4

@@ -13,7 +13,6 @@ sourceUnit: (
 	pragmaDirective
 	| importDirective
 	| contractDefinition
-	| errorDefinition
 	| interfaceDefinition
 	| libraryDefinition
 	| functionDefinition
@@ -380,9 +379,9 @@ tupleExpression: LParen (expression? ( Comma expression?)* ) RParen;
 inlineArrayExpression: LBrack (expression ( Comma expression)* ) RBrack;
 
 /**
- * Besides regular non-keyword Identifiers, some keywords like 'from' can also be used as identifiers.
+ * Besides regular non-keyword Identifiers, some keywords like 'from' and 'error' can also be used as identifiers.
  */
-identifier: Identifier | From | Revert;
+identifier: Identifier | From | Error | Revert;
 
 literal: stringLiteral | numberLiteral | booleanLiteral | hexStringLiteral | unicodeStringLiteral;
 booleanLiteral: True | False;

liblangutil/Token.h

@@ -156,7 +156,6 @@ namespace solidity::langutil
 	K(Else, "else", 0)                                                 \
 	K(Enum, "enum", 0)                                                 \
 	K(Emit, "emit", 0)                                                 \
-	K(Error, "error", 0)                                               \
 	K(Event, "event", 0)                                               \
 	K(External, "external", 0)                                         \
 	K(Fallback, "fallback", 0)                                         \

libsolidity/parsing/Parser.cpp

@@ -116,12 +116,17 @@ ASTPointer<SourceUnit> Parser::parse(CharStream& _charStream)
 			case Token::Function:
 				nodes.push_back(parseFunctionDefinition(true));
 				break;
-			case Token::Error:
-				nodes.push_back(parseErrorDefinition());
-				break;
 			default:
+				if (
+					// Workaround because `error` is not a keyword.
+					m_scanner->currentToken() == Token::Identifier &&
+					currentLiteral() == "error" &&
+					m_scanner->peekNextToken() == Token::Identifier &&
+					m_scanner->peekNextNextToken() == Token::LParen
+				)
+					nodes.push_back(parseErrorDefinition());
 				// Constant variable.
-				if (variableDeclarationStart() && m_scanner->peekNextToken() != Token::EOS)
+				else if (variableDeclarationStart() && m_scanner->peekNextToken() != Token::EOS)
 				{
 					VarDeclParserOptions options;
 					options.kind = VarDeclKind::FileLevel;
@@ -359,7 +364,13 @@ ASTPointer<ContractDefinition> Parser::parseContractDefinition()
 				subNodes.push_back(parseStructDefinition());
 			else if (currentTokenValue == Token::Enum)
 				subNodes.push_back(parseEnumDefinition());
-			else if (currentTokenValue == Token::Error)
+			else if (
+				// Workaround because `error` is not a keyword.
+				currentTokenValue == Token::Identifier &&
+				currentLiteral() == "error" &&
+				m_scanner->peekNextToken() == Token::Identifier &&
+				m_scanner->peekNextNextToken() == Token::LParen
+			)
 				subNodes.push_back(parseErrorDefinition());
 			else if (variableDeclarationStart())
 			{
@@ -930,7 +941,7 @@ ASTPointer<ErrorDefinition> Parser::parseErrorDefinition()
 	ASTNodeFactory nodeFactory(*this);
 	ASTPointer<StructuredDocumentation> documentation = parseStructuredDocumentation();
 
-	expectToken(Token::Error);
+	solAssert(*expectIdentifierToken() == "error", "");
 	auto&& [name, nameLocation] = expectIdentifierWithLocation();
 
 	ASTPointer<ParameterList> parameters = parseParameterList({});

test/libsolidity/semanticTests/errors/weird_name.sol

@@ -0,0 +1,11 @@
+error error(uint a);
+contract C {
+    function f() public pure {
+        revert error(2);
+    }
+}
+// ====
+// compileViaYul: also
+// compileToEwasm: also
+// ----
+// f() -> FAILURE, hex"b48fb6cf", hex"0000000000000000000000000000000000000000000000000000000000000002"

test/libsolidity/semanticTests/reverts/error_struct.sol

@@ -0,0 +1,18 @@
+struct error { uint error; }
+contract C {
+	error test();
+	error _struct;
+	function f() public {
+		revert test();
+	}
+	function g(uint x) public returns (uint) {
+		_struct.error = x;
+		return _struct.error;
+	}
+}
+// ====
+// compileViaYul: also
+// compileToEwasm: also
+// ----
+// f() -> FAILURE, hex"f8a8fd6d"
+// g(uint256): 7 -> 7

test/libsolidity/syntaxTests/errors/struct_named_error.sol

@@ -1,6 +1,6 @@
+// Test that the parser workaround is not breaking.
 struct error {uint a;}
 contract C {
     error x;
 }
 // ----
-// ParserError 2314: (7-12): Expected identifier but got 'error'

test/libsolidity/syntaxTests/errors/weird_name.sol

@@ -1,8 +0,0 @@
-error error(uint a);
-contract C {
-    function f() public pure {
-        revert error(2);
-    }
-}
-// ----
-// ParserError 2314: (6-11): Expected identifier but got 'error'