Merge pull request #6313 from ethereum/yul-for-continue-tests-prerequisites

[Yul] Refactors RedundantAssignEliminator for future changes wrt. break/continue stmts.

libyul/optimiser/RedundantAssignEliminator.cpp

@@ -61,41 +61,44 @@ void RedundantAssignEliminator::operator()(If const& _if)
 {
 	visit(*_if.condition);
 
-	RedundantAssignEliminator branch{*this};
+	TrackedAssignments skipBranch{m_assignments};
-	branch(_if.body);
+	(*this)(_if.body);
 
-	join(branch);
+	merge(m_assignments, move(skipBranch));
 }
 
 void RedundantAssignEliminator::operator()(Switch const& _switch)
 {
 	visit(*_switch.expression);
 
+	TrackedAssignments const preState{m_assignments};
+
 	bool hasDefault = false;
-	vector<RedundantAssignEliminator> branches;
+	vector<TrackedAssignments> branches;
 	for (auto const& c: _switch.cases)
 	{
 		if (!c.value)
 			hasDefault = true;
-		branches.emplace_back(*this);
+		(*this)(c.body);
-		branches.back()(c.body);
+		branches.emplace_back(move(m_assignments));
+		m_assignments = preState;
 	}
 
 	if (hasDefault)
 	{
-		*this = std::move(branches.back());
+		m_assignments = move(branches.back());
 		branches.pop_back();
 	}
 	for (auto& branch: branches)
-		join(branch);
+		merge(m_assignments, move(branch));
 }
 
 void RedundantAssignEliminator::operator()(FunctionDefinition const& _functionDefinition)
 {
-	std::set<YulString> declaredVariables;
+	std::set<YulString> outerDeclaredVariables;
-	std::map<YulString, std::map<Assignment const*, State>> assignments;
+	TrackedAssignments outerAssignments;
-	swap(m_declaredVariables, declaredVariables);
+	swap(m_declaredVariables, outerDeclaredVariables);
-	swap(m_assignments, assignments);
+	swap(m_assignments, outerAssignments);
 
 	(*this)(_functionDefinition.body);
 
@@ -110,8 +113,8 @@ void RedundantAssignEliminator::operator()(FunctionDefinition const& _functionDe
 		finalize(retParam.name);
 	}
 
-	swap(m_declaredVariables, declaredVariables);
+	swap(m_declaredVariables, outerDeclaredVariables);
-	swap(m_assignments, assignments);
+	swap(m_assignments, outerAssignments);
 }
 
 void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
@@ -130,14 +133,14 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 
 	visit(*_forLoop.condition);
 
-	RedundantAssignEliminator zeroRuns{*this};
+	TrackedAssignments zeroRuns{m_assignments};
 
 	(*this)(_forLoop.body);
 	(*this)(_forLoop.post);
 
 	visit(*_forLoop.condition);
 
-	RedundantAssignEliminator oneRun{*this};
+	TrackedAssignments oneRun{m_assignments};
 
 	(*this)(_forLoop.body);
 	(*this)(_forLoop.post);
@@ -145,8 +148,8 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	visit(*_forLoop.condition);
 
 	// Order does not matter because "max" is commutative and associative.
-	join(oneRun);
+	merge(m_assignments, move(oneRun));
-	join(zeroRuns);
+	merge(m_assignments, move(zeroRuns));
 }
 
 void RedundantAssignEliminator::operator()(Break const&)
@@ -173,7 +176,7 @@ void RedundantAssignEliminator::run(Dialect const& _dialect, Block& _ast)
 	RedundantAssignEliminator rae{_dialect};
 	rae(_ast);
 
-	AssignmentRemover remover{rae.m_assignmentsToRemove};
+	AssignmentRemover remover{rae.m_pendingRemovals};
 	remover(_ast);
 }
 
@@ -204,16 +207,14 @@ void joinMap(std::map<K, V>& _a, std::map<K, V>&& _b, F _conflictSolver)
 	}
 }
 
-void RedundantAssignEliminator::join(RedundantAssignEliminator& _other)
+void RedundantAssignEliminator::merge(TrackedAssignments& _target, TrackedAssignments&& _other)
 {
-	m_assignmentsToRemove.insert(begin(_other.m_assignmentsToRemove), end(_other.m_assignmentsToRemove));
+	joinMap(_target, move(_other), [](
-
-	joinMap(m_assignments, std::move(_other.m_assignments), [](
 		map<Assignment const*, State>& _assignmentHere,
 		map<Assignment const*, State>&& _assignmentThere
 	)
 	{
-		return joinMap(_assignmentHere, std::move(_assignmentThere), State::join);
+		return joinMap(_assignmentHere, move(_assignmentThere), State::join);
 	});
 }
 
@@ -232,7 +233,7 @@ void RedundantAssignEliminator::finalize(YulString _variable)
 		if (assignment.second == State{State::Unused} && MovableChecker{*m_dialect, *assignment.first->value}.movable())
 			// TODO the only point where we actually need this
 			// to be a set is for the for loop
-			m_assignmentsToRemove.insert(assignment.first);
+			m_pendingRemovals.insert(assignment.first);
 	}
 	m_assignments.erase(_variable);
 }

libyul/optimiser/RedundantAssignEliminator.h

@@ -100,8 +100,9 @@ class RedundantAssignEliminator: public ASTWalker
 {
 public:
 	explicit RedundantAssignEliminator(Dialect const& _dialect): m_dialect(&_dialect) {}
-	RedundantAssignEliminator(RedundantAssignEliminator const&) = default;
+	RedundantAssignEliminator() = delete;
-	RedundantAssignEliminator& operator=(RedundantAssignEliminator const&) = default;
+	RedundantAssignEliminator(RedundantAssignEliminator const&) = delete;
+	RedundantAssignEliminator& operator=(RedundantAssignEliminator const&) = delete;
 	RedundantAssignEliminator(RedundantAssignEliminator&&) = default;
 	RedundantAssignEliminator& operator=(RedundantAssignEliminator&&) = default;
 
@@ -119,8 +120,6 @@ public:
 	static void run(Dialect const& _dialect, Block& _ast);
 
 private:
-	RedundantAssignEliminator() = default;
-
 	class State
 	{
 	public:
@@ -164,19 +163,21 @@ private:
 		std::set<YulString> m_outerDeclaredVariables;
 	};
 
-	/// Joins the assignment mapping with @a _other according to the rules laid out
+	// TODO check that this does not cause nondeterminism!
+	// This could also be a pseudo-map from state to assignment.
+	using TrackedAssignments = std::map<YulString, std::map<Assignment const*, State>>;
+
+	/// Joins the assignment mapping of @a _source into @a _target according to the rules laid out
 	/// above.
-	/// Will destroy @a _other.
+	/// Will destroy @a _source.
-	void join(RedundantAssignEliminator& _other);
+	static void merge(TrackedAssignments& _target, TrackedAssignments&& _source);
 	void changeUndecidedTo(YulString _variable, State _newState);
 	void finalize(YulString _variable);
 
 	Dialect const* m_dialect;
 	std::set<YulString> m_declaredVariables;
-	// TODO check that this does not cause nondeterminism!
+	std::set<Assignment const*> m_pendingRemovals;
-	// This could also be a pseudo-map from state to assignment.
+	TrackedAssignments m_assignments;
-	std::map<YulString, std::map<Assignment const*, State>> m_assignments;
-	std::set<Assignment const*> m_assignmentsToRemove;
 };
 
 class AssignmentRemover: public ASTModifier