Merge pull request #9555 from ethereum/yul-parser-refactor

Yul parser refactor

libyul/AsmParser.cpp

@@ -27,6 +27,7 @@
 #include <liblangutil/Scanner.h>
 #include <liblangutil/ErrorReporter.h>
 #include <libsolutil/Common.h>
+#include <libsolutil/Visitor.h>
 
 #include <boost/algorithm/string.hpp>
 
@@ -138,18 +139,18 @@ Statement Parser::parseStatement()
 	default:
 		break;
 	}
+
 	// Options left:
-	// Simple instruction (might turn into functional),
+	// Expression/FunctionCall
-	// literal,
+	// Assignment
-	// identifier (might turn into label or functional assignment)
+	variant<Literal, Identifier> elementary(parseLiteralOrIdentifier());
-	ElementaryOperation elementary(parseElementaryOperation());
 
 	switch (currentToken())
 	{
 	case Token::LParen:
 	{
 		Expression expr = parseCall(std::move(elementary));
-		return ExpressionStatement{locationOf(expr), expr};
+		return ExpressionStatement{locationOf(expr), move(expr)};
 	}
 	case Token::Comma:
 	case Token::AssemblyAssign:
@@ -184,7 +185,7 @@ Statement Parser::parseStatement()
 
 			expectToken(Token::Comma);
 
-			elementary = parseElementaryOperation();
+			elementary = parseLiteralOrIdentifier();
 		}
 
 		expectToken(Token::AssemblyAssign);
@@ -192,28 +193,15 @@ Statement Parser::parseStatement()
 		assignment.value = make_unique<Expression>(parseExpression());
 		assignment.location.end = locationOf(*assignment.value).end;
 
-		return Statement{std::move(assignment)};
+		return Statement{move(assignment)};
 	}
 	default:
 		fatalParserError(6913_error, "Call or assignment expected.");
 		break;
 	}
 
-	if (holds_alternative<Identifier>(elementary))
+	yulAssert(false, "");
-	{
+	return {};
-		Identifier& identifier = std::get<Identifier>(elementary);
-		return ExpressionStatement{identifier.location, { move(identifier) }};
-	}
-	else if (holds_alternative<Literal>(elementary))
-	{
-		Expression expr = std::get<Literal>(elementary);
-		return ExpressionStatement{locationOf(expr), expr};
-	}
-	else
-	{
-		yulAssert(false, "Invalid elementary operation.");
-		return {};
-	}
 }
 
 Case Parser::parseCase()
@@ -225,7 +213,7 @@ Case Parser::parseCase()
 	else if (currentToken() == Token::Case)
 	{
 		advance();
-		ElementaryOperation literal = parseElementaryOperation();
+		variant<Literal, Identifier> literal = parseLiteralOrIdentifier();
 		if (!holds_alternative<Literal>(literal))
 			fatalParserError(4805_error, "Literal expected.");
 		_case.value = make_unique<Literal>(std::get<Literal>(std::move(literal)));
@@ -264,38 +252,37 @@ Expression Parser::parseExpression()
 {
 	RecursionGuard recursionGuard(*this);
 
-	ElementaryOperation operation = parseElementaryOperation();
+	variant<Literal, Identifier> operation = parseLiteralOrIdentifier();
-	if (holds_alternative<FunctionCall>(operation) || currentToken() == Token::LParen)
+	return visit(GenericVisitor{
-		return parseCall(std::move(operation));
+		[&](Identifier& _identifier) -> Expression
-	else if (holds_alternative<Identifier>(operation))
+		{
-		return std::get<Identifier>(operation);
+			if (currentToken() == Token::LParen)
-	else
+				return parseCall(std::move(operation));
-	{
+			if (m_dialect.builtin(_identifier.name))
-		yulAssert(holds_alternative<Literal>(operation), "");
+				fatalParserError(
-		return std::get<Literal>(operation);
+					7104_error,
-	}
+					_identifier.location,
+					"Builtin function \"" + _identifier.name.str() + "\" must be called."
+				);
+			return move(_identifier);
+		},
+		[&](Literal& _literal) -> Expression
+		{
+			return move(_literal);
+		}
+	}, operation);
 }
 
-Parser::ElementaryOperation Parser::parseElementaryOperation()
+variant<Literal, Identifier> Parser::parseLiteralOrIdentifier()
 {
 	RecursionGuard recursionGuard(*this);
-	ElementaryOperation ret;
 	switch (currentToken())
 	{
 	case Token::Identifier:
 	{
-		YulString literal{currentLiteral()};
+		Identifier identifier{currentLocation(), YulString{currentLiteral()}};
-		if (m_dialect.builtin(literal))
-		{
-			Identifier identifier{currentLocation(), literal};
-			advance();
-			expectToken(Token::LParen, false);
-			return FunctionCall{identifier.location, identifier, {}};
-		}
-		else
-			ret = Identifier{currentLocation(), literal};
 		advance();
-		break;
+		return identifier;
 	}
 	case Token::StringLiteral:
 	case Token::Number:
@@ -335,8 +322,7 @@ Parser::ElementaryOperation Parser::parseElementaryOperation()
 			literal.type = expectAsmIdentifier();
 		}
 
-		ret = std::move(literal);
+		return literal;
-		break;
 	}
 	case Token::HexStringLiteral:
 		fatalParserError(3772_error, "Hex literals are not valid in this context.");
@@ -344,7 +330,7 @@ Parser::ElementaryOperation Parser::parseElementaryOperation()
 	default:
 		fatalParserError(1856_error, "Literal or identifier expected.");
 	}
-	return ret;
+	return {};
 }
 
 VariableDeclaration Parser::parseVariableDeclaration()
@@ -418,21 +404,17 @@ FunctionDefinition Parser::parseFunctionDefinition()
 	return funDef;
 }
 
-Expression Parser::parseCall(Parser::ElementaryOperation&& _initialOp)
+FunctionCall Parser::parseCall(variant<Literal, Identifier>&& _initialOp)
 {
 	RecursionGuard recursionGuard(*this);
 
-	FunctionCall ret;
+	if (!holds_alternative<Identifier>(_initialOp))
-	if (holds_alternative<Identifier>(_initialOp))
-	{
-		ret.functionName = std::move(std::get<Identifier>(_initialOp));
-		ret.location = ret.functionName.location;
-	}
-	else if (holds_alternative<FunctionCall>(_initialOp))
-		ret = std::move(std::get<FunctionCall>(_initialOp));
-	else
 		fatalParserError(9980_error, "Function name expected.");
 
+	FunctionCall ret;
+	ret.functionName = std::move(std::get<Identifier>(_initialOp));
+	ret.location = ret.functionName.location;
+
 	expectToken(Token::LParen);
 	if (currentToken() != Token::RParen)
 	{

libyul/AsmParser.h

@@ -61,8 +61,6 @@ public:
 	std::unique_ptr<Block> parse(std::shared_ptr<langutil::Scanner> const& _scanner, bool _reuseScanner);
 
 protected:
-	using ElementaryOperation = std::variant<Literal, Identifier, FunctionCall>;
-
 	langutil::SourceLocation currentLocation() const override
 	{
 		return m_locationOverride ? *m_locationOverride : ParserBase::currentLocation();
@@ -84,10 +82,10 @@ protected:
 	Expression parseExpression();
 	/// Parses an elementary operation, i.e. a literal, identifier, instruction or
 	/// builtin functian call (only the name).
-	ElementaryOperation parseElementaryOperation();
+	std::variant<Literal, Identifier> parseLiteralOrIdentifier();
 	VariableDeclaration parseVariableDeclaration();
 	FunctionDefinition parseFunctionDefinition();
-	Expression parseCall(ElementaryOperation&& _initialOp);
+	FunctionCall parseCall(std::variant<Literal, Identifier>&& _initialOp);
 	TypedName parseTypedName();
 	YulString expectAsmIdentifier();
 

scripts/error_codes.py

@@ -223,7 +223,7 @@ def examine_id_coverage(top_dir, source_id_to_file_names, new_ids_only=False):
         "1123", "1220", "1584", "1823",
         "1988", "2657", "2800", "3356",
         "3893", "3996", "4010", "4802",
-        "5073", "5272", "5622", "6272", "7128",
+        "5073", "5272", "5622", "7128",
         "7589", "7593", "7653", "8065", "8084", "8140",
         "8312", "8592", "9085", "9390"
     }

test/libsolidity/syntaxTests/inlineAssembly/assignment_from_opcode_like.sol

@@ -7,4 +7,4 @@ contract C {
     }
 }
 // ----
-// ParserError 2314: (118-119): Expected '(' but got '}'
+// ParserError 7104: (104-109): Builtin function "mload" must be called.

test/libsolidity/syntaxTests/inlineAssembly/assignment_to_opcode_like.sol

@@ -7,4 +7,4 @@ contract C {
     }
 }
 // ----
-// ParserError 2314: (101-103): Expected '(' but got ':='
+// ParserError 6272: (101-103): Cannot assign to builtin function "mload".

test/libsolidity/syntaxTests/inlineAssembly/invalid/bare_instructions_disallowed.sol

@@ -7,4 +7,4 @@ contract C {
     }
 }
 // ----
-// ParserError 2314: (95-98): Expected '(' but got identifier
+// ParserError 6913: (95-98): Call or assignment expected.

test/libsolidity/syntaxTests/inlineAssembly/invalid/unbalanced_negative_stack.sol

@@ -6,4 +6,4 @@ contract test {
     }
 }
 // ----
-// ParserError 2314: (85-86): Expected '(' but got '}'
+// ParserError 6913: (85-86): Call or assignment expected.

test/libyul/yulSyntaxTests/builtin_identifier_3.yul

@@ -2,4 +2,4 @@
 	function f(x) { f(add) }
 }
 // ----
-// ParserError 2314: (24-25): Expected '(' but got ')'
+// ParserError 7104: (21-24): Builtin function "add" must be called.

test/libyul/yulSyntaxTests/builtin_identifier_6.yul

@@ -1,6 +1,5 @@
 {
-	// Test for the unreachable 6272_error
 	add := 1
 }
 // ----
-// ParserError 2314: (47-49): Expected '(' but got ':='
+// ParserError 6272: (7-9): Cannot assign to builtin function "add".

test/libyul/yulSyntaxTests/builtin_identifier_7.yul

@@ -1,7 +1,6 @@
 {
-	// Test for the unreachable 6272_error
 	function f() -> a, b {}
 	add, mul := f()
 }
 // ----
-// ParserError 2314: (71-72): Expected '(' but got ','
+// ParserError 6272: (31-32): Cannot assign to builtin function "add".

test/libyul/yulSyntaxTests/for_expr_invalid_5.yul

@@ -2,4 +2,4 @@
 	for {} mload {} {}
 }
 // ----
-// ParserError 2314: (16-17): Expected '(' but got '{'
+// ParserError 7104: (10-15): Builtin function "mload" must be called.

test/libyul/yulSyntaxTests/functional_partial.yul

@@ -2,4 +2,4 @@
     let x := byte
 }
 // ----
-// ParserError 2314: (20-21): Expected '(' but got '}'
+// ParserError 7104: (15-19): Builtin function "byte" must be called.

test/libyul/yulSyntaxTests/if_statement_invalid_1.yul

@@ -2,4 +2,4 @@
     if mload {}
 }
 // ----
-// ParserError 2314: (15-16): Expected '(' but got '{'
+// ParserError 7104: (9-14): Builtin function "mload" must be called.

test/libyul/yulSyntaxTests/if_statement_invalid_4.yul

@@ -2,4 +2,4 @@
     if calldatasize {}
 }
 // ----
-// ParserError 2314: (22-23): Expected '(' but got '{'
+// ParserError 7104: (9-21): Builtin function "calldatasize" must be called.

test/libyul/yulSyntaxTests/switch_invalid_expr_2.yul

@@ -4,4 +4,4 @@
     default {}
 }
 // ----
-// ParserError 2314: (23-27): Expected '(' but got reserved keyword 'case'
+// ParserError 7104: (13-18): Builtin function "mload" must be called.