Avoid sorting FunctionDefinitions by AST ID during codegen

Co-authored-by: Kamil Śliwak <kamil.sliwak@codepoets.it>

libsolidity/codegen/ir/IRGenerationContext.cpp

@@ -31,6 +31,7 @@
 #include <libsolutil/StringUtils.h>
 
 #include <range/v3/view/map.hpp>
+#include <range/v3/algorithm/find.hpp>
 
 using namespace solidity;
 using namespace solidity::util;
@@ -41,7 +42,7 @@ std::string IRGenerationContext::enqueueFunctionForCodeGeneration(FunctionDefini
 	std::string name = IRNames::function(_function);
 
 	if (!m_functions.contains(name))
-		m_functionGenerationQueue.insert(&_function);
+		m_functionGenerationQueue.push_back(&_function);
 
 	return name;
 }
@@ -50,8 +51,8 @@ FunctionDefinition const* IRGenerationContext::dequeueFunctionForCodeGeneration(
 {
 	solAssert(!m_functionGenerationQueue.empty(), "");
 
-	FunctionDefinition const* result = *m_functionGenerationQueue.begin();
-	m_functionGenerationQueue.erase(m_functionGenerationQueue.begin());
+	FunctionDefinition const* result = m_functionGenerationQueue.front();
+	m_functionGenerationQueue.pop_front();
 	return result;
 }
 
@@ -132,7 +133,7 @@ void IRGenerationContext::initializeInternalDispatch(InternalDispatchMap _intern
 {
 	solAssert(internalDispatchClean(), "");
 
-	for (DispatchSet const& functions: _internalDispatch | ranges::views::values)
+	for (DispatchQueue const& functions: _internalDispatch | ranges::views::values)
 		for (auto function: functions)
 			enqueueFunctionForCodeGeneration(*function);
 
@@ -149,16 +150,15 @@ InternalDispatchMap IRGenerationContext::consumeInternalDispatchMap()
 void IRGenerationContext::addToInternalDispatch(FunctionDefinition const& _function)
 {
 	FunctionType const* functionType = TypeProvider::function(_function, FunctionType::Kind::Internal);
-	solAssert(functionType, "");
+	solAssert(functionType);
 
 	YulArity arity = YulArity::fromType(*functionType);
-
-	if (m_internalDispatchMap.count(arity) != 0 && m_internalDispatchMap[arity].count(&_function) != 0)
-		// Note that m_internalDispatchMap[arity] is a set with a custom comparator, which looks at function IDs not definitions
-		solAssert(*m_internalDispatchMap[arity].find(&_function) == &_function, "Different definitions with the same function ID");
-
-	m_internalDispatchMap[arity].insert(&_function);
-	enqueueFunctionForCodeGeneration(_function);
+	DispatchQueue& dispatchQueue = m_internalDispatchMap[arity];
+	if (ranges::find(dispatchQueue, &_function) == ranges::end(dispatchQueue))
+	{
+		dispatchQueue.push_back(&_function);
+		enqueueFunctionForCodeGeneration(_function);
+	}
 }
 
 

libsolidity/codegen/ir/IRGenerationContext.h

@@ -37,7 +37,6 @@
 #include <set>
 #include <string>
 #include <memory>
-#include <vector>
 
 namespace solidity::frontend
 {
@@ -45,20 +44,8 @@ namespace solidity::frontend
 class YulUtilFunctions;
 class ABIFunctions;
 
-struct AscendingFunctionIDCompare
-{
-	bool operator()(FunctionDefinition const* _f1, FunctionDefinition const* _f2) const
-	{
-		// NULLs always first.
-		if (_f1 != nullptr && _f2 != nullptr)
-			return _f1->id() < _f2->id();
-		else
-			return _f1 == nullptr;
-	}
-};
-
-using DispatchSet = std::set<FunctionDefinition const*, AscendingFunctionIDCompare>;
-using InternalDispatchMap = std::map<YulArity, DispatchSet>;
+using DispatchQueue = std::deque<FunctionDefinition const*>;
+using InternalDispatchMap = std::map<YulArity, DispatchQueue>;
 
 /**
  * Class that contains contextual information during IR generation.
@@ -197,10 +184,9 @@ private:
 	/// were discovered by the IR generator during AST traversal.
 	/// Note that the queue gets filled in a lazy way - new definitions can be added while the
 	/// collected ones get removed and traversed.
-	/// The order and duplicates are irrelevant here (hence std::set rather than std::queue) as
-	/// long as the order of Yul functions in the generated code is deterministic and the same on
-	/// all platforms - which is a property guaranteed by MultiUseYulFunctionCollector.
-	DispatchSet m_functionGenerationQueue;
+	/// The order and duplicates are relevant here
+	/// (see: IRGenerationContext::[enqueue|dequeue]FunctionForCodeGeneration)
+	DispatchQueue m_functionGenerationQueue;
 
 	/// Collection of functions that need to be callable via internal dispatch.
 	/// Note that having a key with an empty set of functions is a valid situation. It means that

libsolidity/codegen/ir/IRGenerator.cpp

@@ -279,6 +279,7 @@ InternalDispatchMap IRGenerator::generateInternalDispatchFunctions(ContractDefin
 			templ("out", suffixedVariableNameList("out_", 0, arity.out));
 
 			std::vector<std::map<std::string, std::string>> cases;
+			std::set<int64_t> caseValues;
 			for (FunctionDefinition const* function: internalDispatchMap.at(arity))
 			{
 				solAssert(function, "");
@@ -289,12 +290,14 @@ InternalDispatchMap IRGenerator::generateInternalDispatchFunctions(ContractDefin
 				solAssert(!function->isConstructor(), "");
 				// 0 is reserved for uninitialized function pointers
 				solAssert(function->id() != 0, "Unexpected function ID: 0");
+				solAssert(caseValues.count(function->id()) == 0, "Duplicate function ID");
 				solAssert(m_context.functionCollector().contains(IRNames::function(*function)), "");
 
 				cases.emplace_back(std::map<std::string, std::string>{
 					{"funID", std::to_string(m_context.mostDerivedContract().annotation().internalFunctionIDs.at(function))},
 					{"name", IRNames::function(*function)}
 				});
+				caseValues.insert(function->id());
 			}
 
 			templ("cases", std::move(cases));