Merge pull request #10489 from ethereum/develop

Merge develop into breaking.

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),
-	// literal,
-	// identifier (might turn into label or functional assignment)
-	ElementaryOperation elementary(parseElementaryOperation());
+	// Expression/FunctionCall
+	// Assignment
+	variant<Literal, Identifier> elementary(parseLiteralOrIdentifier());
 
 	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))
-	{
-		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 {};
-	}
+	yulAssert(false, "");
+	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();
-	if (holds_alternative<FunctionCall>(operation) || currentToken() == Token::LParen)
-		return parseCall(std::move(operation));
-	else if (holds_alternative<Identifier>(operation))
-		return std::get<Identifier>(operation);
-	else
-	{
-		yulAssert(holds_alternative<Literal>(operation), "");
-		return std::get<Literal>(operation);
-	}
+	variant<Literal, Identifier> operation = parseLiteralOrIdentifier();
+	return visit(GenericVisitor{
+		[&](Identifier& _identifier) -> Expression
+		{
+			if (currentToken() == Token::LParen)
+				return parseCall(std::move(operation));
+			if (m_dialect.builtin(_identifier.name))
+				fatalParserError(
+					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()};
-		if (m_dialect.builtin(literal))
-		{
-			Identifier identifier{currentLocation(), literal};
-			advance();
-			expectToken(Token::LParen, false);
-			return FunctionCall{identifier.location, identifier, {}};
-		}
-		else
-			ret = Identifier{currentLocation(), literal};
+		Identifier identifier{currentLocation(), YulString{currentLiteral()}};
 		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);
-		break;
+		return literal;
 	}
 	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))
-	{
-		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
+	if (!holds_alternative<Identifier>(_initialOp))
 		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();
 

libyul/optimiser/NameDispenser.cpp

@@ -35,8 +35,9 @@ using namespace solidity::yul;
 using namespace solidity::util;
 
 NameDispenser::NameDispenser(Dialect const& _dialect, Block const& _ast, set<YulString> _reservedNames):
-	NameDispenser(_dialect, NameCollector(_ast).names() + std::move(_reservedNames))
+	NameDispenser(_dialect, NameCollector(_ast).names() + _reservedNames)
 {
+	m_reservedNames = move(_reservedNames);
 }
 
 NameDispenser::NameDispenser(Dialect const& _dialect, set<YulString> _usedNames):
@@ -61,3 +62,9 @@ bool NameDispenser::illegalName(YulString _name)
 {
 	return isRestrictedIdentifier(m_dialect, _name) || m_usedNames.count(_name);
 }
+
+void NameDispenser::reset(Block const& _ast)
+{
+	m_usedNames = NameCollector(_ast).names() + m_reservedNames;
+	m_counter = 0;
+}

libyul/optimiser/NameDispenser.h

@@ -51,11 +51,19 @@ public:
 	/// return it.
 	void markUsed(YulString _name) { m_usedNames.insert(_name); }
 
-private:
+	std::set<YulString> const& usedNames() { return m_usedNames; }
+
+	/// Returns true if `_name` is either used or is a restricted identifier.
 	bool illegalName(YulString _name);
 
+	/// Resets `m_usedNames` with *only* the names that are used in the AST. Also resets value of
+	/// `m_counter` to zero.
+	void reset(Block const& _ast);
+
+private:
 	Dialect const& m_dialect;
 	std::set<YulString> m_usedNames;
+	std::set<YulString> m_reservedNames;
 	size_t m_counter = 0;
 };
 

libyul/optimiser/NameSimplifier.cpp

@@ -19,6 +19,8 @@
 #include <libyul/optimiser/NameCollector.h>
 #include <libyul/AST.h>
 #include <libyul/Dialect.h>
+#include <libyul/YulString.h>
+#include <libyul/optimiser/NameDispenser.h>
 #include <libyul/optimiser/OptimizerUtilities.h>
 
 #include <libsolutil/CommonData.h>
@@ -28,19 +30,13 @@
 using namespace solidity::yul;
 using namespace std;
 
-NameSimplifier::NameSimplifier(
-	OptimiserStepContext& _context,
-	Block const& _ast
-):
-	m_context(_context),
-	m_usedNames(_context.reservedIdentifiers)
+NameSimplifier::NameSimplifier(OptimiserStepContext& _context, Block const& _ast):
+	m_context(_context)
 {
-	for (YulString name: m_usedNames)
+	for (YulString name: _context.reservedIdentifiers)
 		m_translations[name] = name;
 
-	set<YulString> allNames = NameCollector(_ast).names();
-	m_usedNames += allNames;
-	for (YulString name: allNames)
+	for (YulString const& name: NameCollector(_ast).names())
 		findSimplification(name);
 }
 
@@ -77,7 +73,7 @@ void NameSimplifier::operator()(FunctionCall& _funCall)
 	ASTModifier::operator()(_funCall);
 }
 
-void NameSimplifier::findSimplification(YulString _name)
+void NameSimplifier::findSimplification(YulString const& _name)
 {
 	if (m_translations.count(_name))
 		return;
@@ -98,19 +94,19 @@ void NameSimplifier::findSimplification(YulString _name)
 		{regex("index_access_t_array"), "index_access"},
 		{regex("[0-9]*_$"), ""}
 	};
+
 	for (auto const& [pattern, substitute]: replacements)
 	{
 		string candidate = regex_replace(name, pattern, substitute);
-		if (
-			!isRestrictedIdentifier(m_context.dialect, YulString(candidate)) &&
-			!m_usedNames.count(YulString(candidate))
-		)
+		if (!m_context.dispenser.illegalName(YulString(candidate)))
 			name = candidate;
 	}
+
 	if (name != _name.str())
 	{
-		m_usedNames.insert(YulString(name));
-		m_translations[_name] = YulString(name);
+		YulString newName{name};
+		m_context.dispenser.markUsed(newName);
+		m_translations[_name] = move(newName);
 	}
 }
 

libyul/optimiser/NameSimplifier.h

@@ -57,19 +57,15 @@ public:
 	void operator()(FunctionDefinition& _funDef) override;
 
 private:
-	NameSimplifier(
-		OptimiserStepContext& _context,
-		Block const& _ast
-	);
+	NameSimplifier(OptimiserStepContext& _context, Block const& _ast);
 
 	/// Tries to rename a list of variables.
 	void renameVariables(std::vector<TypedName>& _variables);
 
-	void findSimplification(YulString _name);
+	void findSimplification(YulString const& _name);
 	void translate(YulString& _name);
 
 	OptimiserStepContext& m_context;
-	std::set<YulString> m_usedNames;
 	std::map<YulString, YulString> m_translations;
 };
 

libyul/optimiser/Suite.cpp

@@ -105,6 +105,7 @@ void OptimiserSuite::run(
 	// ForLoopInitRewriter. Run them first to be able to run arbitrary sequences safely.
 	suite.runSequence("hfgo", ast);
 
+	NameSimplifier::run(suite.m_context, ast);
 	// Now the user-supplied part
 	suite.runSequence(_optimisationSequence, ast);
 
@@ -140,6 +141,9 @@ void OptimiserSuite::run(
 		if (ast.statements.size() > 1 && std::get<Block>(ast.statements.front()).statements.empty())
 			ast.statements.erase(ast.statements.begin());
 	}
+
+	suite.m_dispenser.reset(ast);
+	NameSimplifier::run(suite.m_context, ast);
 	VarNameCleaner::run(suite.m_context, ast);
 
 	*_object.analysisInfo = AsmAnalyzer::analyzeStrictAssertCorrect(_dialect, _object);
@@ -191,7 +195,6 @@ map<string, unique_ptr<OptimiserStep>> const& OptimiserSuite::allSteps()
 			LiteralRematerialiser,
 			LoadResolver,
 			LoopInvariantCodeMotion,
-			NameSimplifier,
 			RedundantAssignEliminator,
 			ReasoningBasedSimplifier,
 			Rematerialiser,
@@ -203,6 +206,7 @@ map<string, unique_ptr<OptimiserStep>> const& OptimiserSuite::allSteps()
 			VarDeclInitializer
 		>();
 	// Does not include VarNameCleaner because it destroys the property of unique names.
+	// Does not include NameSimplifier.
 	return instance;
 }
 
@@ -230,7 +234,6 @@ map<string, char> const& OptimiserSuite::stepNameToAbbreviationMap()
 		{LiteralRematerialiser::name,         'T'},
 		{LoadResolver::name,                  'L'},
 		{LoopInvariantCodeMotion::name,       'M'},
-		{NameSimplifier::name,                'N'},
 		{ReasoningBasedSimplifier::name,      'R'},
 		{RedundantAssignEliminator::name,     'r'},
 		{Rematerialiser::name,                'm'},