From 21a2b69f7442708a2c07253f307cd112b0a33d69 Mon Sep 17 00:00:00 2001
From: cameel <cameel2@gmail.com>
Date: Fri, 24 Jan 2020 23:22:01 +0100
Subject: [PATCH] [yul-phaser] Create OptimiserStepContext on demand instead of
 storing it in Program class

- This also lets us get rid of the static s_externallyUsedIdentifiers.
---
 tools/yulPhaser/Program.cpp | 18 ++++++++++++------
 tools/yulPhaser/Program.h   | 22 +++++++---------------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/tools/yulPhaser/Program.cpp b/tools/yulPhaser/Program.cpp
index 271c047aa..459fca2e6 100644
--- a/tools/yulPhaser/Program.cpp
+++ b/tools/yulPhaser/Program.cpp
@@ -35,6 +35,7 @@
 #include <libyul/optimiser/FunctionGrouper.h>
 #include <libyul/optimiser/FunctionHoister.h>
 #include <libyul/optimiser/Metrics.h>
+#include <libyul/optimiser/OptimiserStep.h>
 #include <libyul/optimiser/Suite.h>
 
 #include <libsolutil/CommonIO.h>
@@ -59,8 +60,6 @@ ostream& operator<<(ostream& _stream, Program const& _program);
 
 }
 
-set<YulString> const Program::s_externallyUsedIdentifiers = {};
-
 Program Program::load(string const& _sourcePath)
 {
 	Dialect const& dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
@@ -82,7 +81,7 @@ Program Program::load(string const& _sourcePath)
 
 void Program::optimise(vector<string> const& _optimisationSteps)
 {
-	applyOptimisationSteps(m_optimiserStepContext, *m_ast, _optimisationSteps);
+	applyOptimisationSteps(m_dialect, m_nameDispenser, *m_ast, _optimisationSteps);
 }
 
 ostream& phaser::operator<<(ostream& _stream, Program const& _program)
@@ -138,19 +137,26 @@ unique_ptr<Block> Program::disambiguateAST(
 	AsmAnalysisInfo const& _analysisInfo
 )
 {
-	Disambiguator disambiguator(_dialect, _analysisInfo, s_externallyUsedIdentifiers);
+	set<YulString> const externallyUsedIdentifiers = {};
+	Disambiguator disambiguator(_dialect, _analysisInfo, externallyUsedIdentifiers);
 
 	return make_unique<Block>(get<Block>(disambiguator(_ast)));
 }
 
 void Program::applyOptimisationSteps(
-	OptimiserStepContext& _context,
+	Dialect const& _dialect,
+	NameDispenser& _nameDispenser,
 	Block& _ast,
 	vector<string> const& _optimisationSteps
 )
 {
+	// An empty set of reserved identifiers. It could be a constructor parameter but I don't
+	// think it would be useful in this tool. Other tools (like yulopti) have it empty too.
+	set<YulString> const externallyUsedIdentifiers = {};
+	OptimiserStepContext context{_dialect, _nameDispenser, externallyUsedIdentifiers};
+
 	for (string const& step: _optimisationSteps)
-		OptimiserSuite::allSteps().at(step)->run(_context, _ast);
+		OptimiserSuite::allSteps().at(step)->run(context, _ast);
 }
 
 size_t Program::computeCodeSize(Block const& _ast)
diff --git a/tools/yulPhaser/Program.h b/tools/yulPhaser/Program.h
index 3648d6e21..bb382b374 100644
--- a/tools/yulPhaser/Program.h
+++ b/tools/yulPhaser/Program.h
@@ -17,7 +17,6 @@
 
 #pragma once
 
-#include <libyul/optimiser/OptimiserStep.h>
 #include <libyul/optimiser/NameDispenser.h>
 #include <libyul/AsmData.h>
 
@@ -41,7 +40,6 @@ namespace solidity::yul
 
 struct AsmAnalysisInfo;
 struct Dialect;
-class YulString;
 
 }
 
@@ -62,9 +60,8 @@ class Program: private boost::noncopyable
 public:
 	Program(Program&& program):
 		m_ast(std::move(program.m_ast)),
-		m_nameDispenser(std::move(program.m_nameDispenser)),
-		// Creating a new instance because a copied one would have a dangling reference to the old dispenser
-		m_optimiserStepContext{program.m_optimiserStepContext.dialect, m_nameDispenser, s_externallyUsedIdentifiers}
+		m_dialect{program.m_dialect},
+		m_nameDispenser(std::move(program.m_nameDispenser))
 	{}
 	Program operator=(Program&& program) = delete;
 
@@ -83,8 +80,8 @@ private:
 		std::unique_ptr<yul::Block> _ast
 	):
 		m_ast(std::move(_ast)),
-		m_nameDispenser(_dialect, *m_ast, s_externallyUsedIdentifiers),
-		m_optimiserStepContext{_dialect, m_nameDispenser, s_externallyUsedIdentifiers}
+		m_dialect{_dialect},
+		m_nameDispenser(_dialect, *m_ast, {})
 	{}
 
 	static langutil::CharStream loadSource(std::string const& _sourcePath);
@@ -102,21 +99,16 @@ private:
 		yul::AsmAnalysisInfo const& _analysisInfo
 	);
 	static void applyOptimisationSteps(
-		yul::OptimiserStepContext& _context,
+		yul::Dialect const& _dialect,
+		yul::NameDispenser& _nameDispenser,
 		yul::Block& _ast,
 		std::vector<std::string> const& _optimisationSteps
 	);
 	static size_t computeCodeSize(yul::Block const& _ast);
 
 	std::unique_ptr<yul::Block> m_ast;
+	yul::Dialect const& m_dialect;
 	yul::NameDispenser m_nameDispenser;
-	yul::OptimiserStepContext m_optimiserStepContext;
-
-	// Always empty set of reserved identifiers. It could be a constructor parameter but I don't
-	// think it would be useful in this tool. Other tools (like yulopti) have it empty too.
-	// We need it to live as long as m_optimiserStepContext because it stores a reference
-	// but since it's empty and read-only we can just have all objects share one static instance.
-	static std::set<yul::YulString> const s_externallyUsedIdentifiers;
 };
 
 }