More logic about control flow with continue and about finalize. Remove BlockScope.

libyul/optimiser/RedundantAssignEliminator.cpp

@@ -97,31 +97,30 @@ void RedundantAssignEliminator::operator()(FunctionDefinition const& _functionDe
 {
 	std::set<YulString> outerDeclaredVariables;
 	TrackedAssignments outerAssignments;
+	ForLoopInfo forLoopInfo;
 	swap(m_declaredVariables, outerDeclaredVariables);
 	swap(m_assignments, outerAssignments);
+	swap(m_forLoopInfo, forLoopInfo);
 
 	(*this)(_functionDefinition.body);
 
 	for (auto const& param: _functionDefinition.parameters)
-	{
-		changeUndecidedTo(param.name, State::Unused);
-		finalize(param.name);
-	}
+		finalize(param.name, State::Unused);
 	for (auto const& retParam: _functionDefinition.returnVariables)
-	{
-		changeUndecidedTo(retParam.name, State::Used);
-		finalize(retParam.name);
-	}
+		finalize(retParam.name, State::Used);
 
 	swap(m_declaredVariables, outerDeclaredVariables);
 	swap(m_assignments, outerAssignments);
+	swap(m_forLoopInfo, forLoopInfo);
 }
 
 void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 {
-	// This will set all variables that are declared in this
-	// block to "unused" when it is destroyed.
-	BlockScope scope(*this);
+	ForLoopInfo outerForLoopInfo;
+	swap(outerForLoopInfo, m_forLoopInfo);
+
+	set<YulString> outerDeclaredVariables;
+	swap(m_declaredVariables, outerDeclaredVariables);
 
 	// We need to visit the statements directly because of the
 	// scoping rules.
@@ -133,9 +132,6 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 
 	visit(*_forLoop.condition);
 
-	ForLoopInfo outerForLoopInfo;
-	swap(outerForLoopInfo, m_forLoopInfo);
-
 	TrackedAssignments zeroRuns{m_assignments};
 
 	(*this)(_forLoop.body);
@@ -150,6 +146,7 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	(*this)(_forLoop.body);
 
 	merge(m_assignments, move(m_forLoopInfo.pendingContinueStmts));
+	m_forLoopInfo.pendingContinueStmts.clear();
 	(*this)(_forLoop.post);
 
 	visit(*_forLoop.condition);
@@ -158,28 +155,39 @@ void RedundantAssignEliminator::operator()(ForLoop const& _forLoop)
 	merge(m_assignments, move(oneRun));
 	merge(m_assignments, move(zeroRuns));
 	merge(m_assignments, move(m_forLoopInfo.pendingBreakStmts));
+	m_forLoopInfo.pendingBreakStmts.clear();
 
-	// Oestore potential outer for-loop states.
+	for (auto const& var: m_declaredVariables)
+		finalize(var, State::Unused);
+	swap(m_declaredVariables, outerDeclaredVariables);
+
+	// Restore potential outer for-loop states.
 	swap(m_forLoopInfo, outerForLoopInfo);
 }
 
 void RedundantAssignEliminator::operator()(Break const&)
 {
-	m_forLoopInfo.pendingBreakStmts.push_back(m_assignments);
+	m_forLoopInfo.pendingBreakStmts.emplace_back(move(m_assignments));
+	m_assignments.clear();
 }
 
 void RedundantAssignEliminator::operator()(Continue const&)
 {
-	m_forLoopInfo.pendingContinueStmts.push_back(m_assignments);
+	m_forLoopInfo.pendingContinueStmts.emplace_back(move(m_assignments));
+	m_assignments.clear();
 }
 
 void RedundantAssignEliminator::operator()(Block const& _block)
 {
-	// This will set all variables that are declared in this
-	// block to "unused" when it is destroyed.
-	BlockScope scope(*this);
+	set<YulString> outerDeclaredVariables;
+	swap(m_declaredVariables, outerDeclaredVariables);
 
 	ASTWalker::operator()(_block);
+
+	for (auto const& var: m_declaredVariables)
+		finalize(var, State::Unused);
+
+	swap(m_declaredVariables, outerDeclaredVariables);
 }
 
 void RedundantAssignEliminator::run(Dialect const& _dialect, Block& _ast)
@@ -239,21 +247,35 @@ void RedundantAssignEliminator::merge(TrackedAssignments& _target, vector<Tracke
 void RedundantAssignEliminator::changeUndecidedTo(YulString _variable, RedundantAssignEliminator::State _newState)
 {
 	for (auto& assignment: m_assignments[_variable])
-		if (assignment.second == State{State::Undecided})
+		if (assignment.second == State::Undecided)
 			assignment.second = _newState;
 }
 
-void RedundantAssignEliminator::finalize(YulString _variable)
+void RedundantAssignEliminator::finalize(YulString _variable, RedundantAssignEliminator::State _finalState)
 {
-	for (auto& assignment: m_assignments[_variable])
+	finalize(m_assignments, _variable, _finalState);
+	for (auto& assignments: m_forLoopInfo.pendingBreakStmts)
+		finalize(assignments, _variable, _finalState);
+	for (auto& assignments: m_forLoopInfo.pendingContinueStmts)
+		finalize(assignments, _variable, _finalState);
+}
+
+void RedundantAssignEliminator::finalize(
+	TrackedAssignments& _assignments,
+	YulString _variable,
+	RedundantAssignEliminator::State _finalState
+)
+{
+	for (auto const& assignment: _assignments[_variable])
 	{
-		assertThrow(assignment.second != State::Undecided, OptimizerException, "");
-		if (assignment.second == State{State::Unused} && MovableChecker{*m_dialect, *assignment.first->value}.movable())
+		State const state = assignment.second == State::Undecided ? _finalState : assignment.second;
+
+		if (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_pendingRemovals.insert(assignment.first);
 	}
-	m_assignments.erase(_variable);
+	_assignments.erase(_variable);
 }
 
 void AssignmentRemover::operator()(Block& _block)

libyul/optimiser/RedundantAssignEliminator.h

@@ -137,33 +137,6 @@ private:
 		Value m_value = Undecided;
 	};
 
-	/**
-	 * Takes care about storing the list of declared variables and
-	 * sets them to "unused" when it is destroyed.
-	 */
-	class BlockScope
-	{
-	public:
-		explicit BlockScope(RedundantAssignEliminator& _rae): m_rae(_rae)
-		{
-			swap(m_rae.m_declaredVariables, m_outerDeclaredVariables);
-		}
-		~BlockScope()
-		{
-			// This should actually store all declared variables
-			// into a different mapping
-			for (auto const& var: m_rae.m_declaredVariables)
-				m_rae.changeUndecidedTo(var, State::Unused);
-			for (auto const& var: m_rae.m_declaredVariables)
-				m_rae.finalize(var);
-			swap(m_rae.m_declaredVariables, m_outerDeclaredVariables);
-		}
-
-	private:
-		RedundantAssignEliminator& m_rae;
-		std::set<YulString> m_outerDeclaredVariables;
-	};
-
 	// 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>>;
@@ -174,7 +147,12 @@ private:
 	static void merge(TrackedAssignments& _target, TrackedAssignments&& _source);
 	static void merge(TrackedAssignments& _target, std::vector<TrackedAssignments>&& _source);
 	void changeUndecidedTo(YulString _variable, State _newState);
-	void finalize(YulString _variable);
+	/// Called when a variable goes out of scope. Sets the state of all still undecided
+	/// assignments to the final state. In this case, this also applies to pending
+	/// break and continue TrackedAssignments.
+	void finalize(YulString _variable, State _finalState);
+	/// Helper function for the above.
+	void finalize(TrackedAssignments& _assignments, YulString _variable, State _finalState);
 
 	Dialect const* m_dialect;
 	std::set<YulString> m_declaredVariables;