From 99d831d7d150fc03941cbd0482d51dc54d228fae Mon Sep 17 00:00:00 2001
From: Daniel Kirchner <daniel@ekpyron.org>
Date: Wed, 15 Jul 2020 19:50:19 +0200
Subject: [PATCH] Extract recursive function detection from Semantics into the
 CallGraph.

---
 libyul/optimiser/CallGraphGenerator.cpp | 41 +++++++++++++++++++++++++
 libyul/optimiser/CallGraphGenerator.h   |  4 +++
 libyul/optimiser/Semantics.cpp          | 22 +------------
 3 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/libyul/optimiser/CallGraphGenerator.cpp b/libyul/optimiser/CallGraphGenerator.cpp
index d3805e7f7..565518af0 100644
--- a/libyul/optimiser/CallGraphGenerator.cpp
+++ b/libyul/optimiser/CallGraphGenerator.cpp
@@ -30,6 +30,47 @@ using namespace solidity;
 using namespace solidity::yul;
 using namespace solidity::util;
 
+namespace
+{
+// TODO: This algorithm is non-optimal.
+struct CallGraphCycleFinder
+{
+	CallGraph const& callGraph;
+	set<YulString> containedInCycle{};
+	set<YulString> visited{};
+	vector<YulString> currentPath{};
+
+	void visit(YulString _function)
+	{
+		if (visited.count(_function))
+			return;
+		if (
+			auto it = find(currentPath.begin(), currentPath.end(), _function);
+			it != currentPath.end()
+		)
+			containedInCycle.insert(it, currentPath.end());
+		else
+		{
+			currentPath.emplace_back(_function);
+			if (callGraph.functionCalls.count(_function))
+				for (auto const& child: callGraph.functionCalls.at(_function))
+					visit(child);
+			currentPath.pop_back();
+			visited.insert(_function);
+		}
+	}
+};
+}
+
+set<YulString> CallGraph::recursiveFunctions() const
+{
+	CallGraphCycleFinder cycleFinder{*this};
+	// Visiting the root only is not enough, since there may be disconnected recursive functions.
+	for (auto const& call: functionCalls)
+		cycleFinder.visit(call.first);
+	return cycleFinder.containedInCycle;
+}
+
 CallGraph CallGraphGenerator::callGraph(Block const& _ast)
 {
 	CallGraphGenerator gen;
diff --git a/libyul/optimiser/CallGraphGenerator.h b/libyul/optimiser/CallGraphGenerator.h
index 2a05c7564..2b57f8592 100644
--- a/libyul/optimiser/CallGraphGenerator.h
+++ b/libyul/optimiser/CallGraphGenerator.h
@@ -35,6 +35,10 @@ struct CallGraph
 {
 	std::map<YulString, std::set<YulString>> functionCalls;
 	std::set<YulString> functionsWithLoops;
+	/// @returns the set of functions contained in cycles in the call graph, i.e.
+	/// functions that are part of a (mutual) recursion.
+	/// Note that this does not include functions that merely call recursive functions.
+	std::set<YulString> recursiveFunctions() const;
 };
 
 /**
diff --git a/libyul/optimiser/Semantics.cpp b/libyul/optimiser/Semantics.cpp
index f9d831b7a..6100b1cdc 100644
--- a/libyul/optimiser/Semantics.cpp
+++ b/libyul/optimiser/Semantics.cpp
@@ -102,33 +102,13 @@ map<YulString, SideEffects> SideEffectsPropagator::sideEffects(
 	// is actually a bit different from "not movable".
 
 	map<YulString, SideEffects> ret;
-	for (auto const& function: _directCallGraph.functionsWithLoops)
+	for (auto const& function: _directCallGraph.functionsWithLoops + _directCallGraph.recursiveFunctions())
 	{
 		ret[function].movable = false;
 		ret[function].sideEffectFree = false;
 		ret[function].sideEffectFreeIfNoMSize = false;
 	}
 
-	// Detect recursive functions.
-	for (auto const& call: _directCallGraph.functionCalls)
-	{
-		// TODO we could shortcut the search as soon as we find a
-		// function that has as bad side-effects as we can
-		// ever achieve via recursion.
-		auto search = [&](YulString const& _functionName, util::CycleDetector<YulString>& _cycleDetector, size_t) {
-			for (auto const& callee: _directCallGraph.functionCalls.at(_functionName))
-				if (!_dialect.builtin(callee))
-					if (_cycleDetector.run(callee))
-						return;
-		};
-		if (util::CycleDetector<YulString>(search).run(call.first))
-		{
-			ret[call.first].movable = false;
-			ret[call.first].sideEffectFree = false;
-			ret[call.first].sideEffectFreeIfNoMSize = false;
-		}
-	}
-
 	for (auto const& call: _directCallGraph.functionCalls)
 	{
 		YulString funName = call.first;