Address review comments

ethereum · Dec 6, 2023 · b3a7739 · b3a7739
1 parent 47fdd4c
commit b3a7739
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 85 deletions.
diff --git a/libsolidity/CMakeLists.txt b/libsolidity/CMakeLists.txt
@@ -188,8 +188,8 @@ set(sources
 	experimental/analysis/Analysis.h
 	experimental/analysis/DebugWarner.cpp
 	experimental/analysis/DebugWarner.h
-	experimental/analysis/FunctionDependencyGraph.cpp
-	experimental/analysis/FunctionDependencyGraph.h
+	experimental/analysis/FunctionDependencyAnalysis.cpp
+	experimental/analysis/FunctionDependencyAnalysis.h
 	experimental/analysis/TypeClassRegistration.cpp
 	experimental/analysis/TypeClassRegistration.h
 	experimental/analysis/TypeInference.cpp
@@ -198,8 +198,7 @@ set(sources
 	experimental/analysis/TypeRegistration.h
 	experimental/analysis/SyntaxRestrictor.cpp
 	experimental/analysis/SyntaxRestrictor.h
-	experimental/ast/CallGraph.cpp
-	experimental/ast/CallGraph.h
+	experimental/ast/FunctionCallGraph.h
 	experimental/ast/Type.cpp
 	experimental/ast/Type.h
 	experimental/ast/TypeSystem.cpp

diff --git a/libsolidity/experimental/analysis/Analysis.cpp b/libsolidity/experimental/analysis/Analysis.cpp
@@ -17,7 +17,7 @@
 // SPDX-License-Identifier: GPL-3.0
 #include <libsolidity/experimental/analysis/Analysis.h>
 #include <libsolidity/experimental/analysis/DebugWarner.h>
-#include <libsolidity/experimental/analysis/FunctionDependencyGraph.h>
+#include <libsolidity/experimental/analysis/FunctionDependencyAnalysis.h>
 #include <libsolidity/experimental/analysis/SyntaxRestrictor.h>
 #include <libsolidity/experimental/analysis/TypeClassRegistration.h>
 #include <libsolidity/experimental/analysis/TypeInference.h>
@@ -36,7 +36,7 @@ struct Analysis::AnnotationContainer
 
 struct Analysis::GlobalAnnotationContainer
 {
-	FunctionDependencyGraph::GlobalAnnotation functionCallGraphAnnotation;
+	FunctionDependencyAnalysis::GlobalAnnotation functionDependencyGraphAnnotation;
 	TypeClassRegistration::GlobalAnnotation typeClassRegistrationAnnotation;
 	TypeRegistration::GlobalAnnotation typeRegistrationAnnotation;
 	TypeInference::GlobalAnnotation typeInferenceAnnotation;
@@ -67,15 +67,15 @@ TypeClassRegistration::Annotation const& solidity::frontend::experimental::detai
 }
 
 template<>
-FunctionDependencyGraph::GlobalAnnotation const& solidity::frontend::experimental::detail::ConstAnnotationFetcher<FunctionDependencyGraph>::get() const
+FunctionDependencyAnalysis::GlobalAnnotation const& solidity::frontend::experimental::detail::ConstAnnotationFetcher<FunctionDependencyAnalysis>::get() const
 {
-	return analysis.annotationContainer().functionCallGraphAnnotation;
+	return analysis.annotationContainer().functionDependencyGraphAnnotation;
 }
 
 template<>
-FunctionDependencyGraph::GlobalAnnotation& solidity::frontend::experimental::detail::AnnotationFetcher<FunctionDependencyGraph>::get()
+FunctionDependencyAnalysis::GlobalAnnotation& solidity::frontend::experimental::detail::AnnotationFetcher<FunctionDependencyAnalysis>::get()
 {
-	return analysis.annotationContainer().functionCallGraphAnnotation;
+	return analysis.annotationContainer().functionDependencyGraphAnnotation;
 }
 
 template<>
@@ -165,7 +165,7 @@ bool Analysis::check(std::vector<std::shared_ptr<SourceUnit const>> const& _sour
 		TypeClassRegistration,
 		TypeRegistration,
 		// TODO move after step introduced in https://github.com/ethereum/solidity/pull/14578, but before TypeInference
-		FunctionDependencyGraph,
+		FunctionDependencyAnalysis,
 		TypeInference,
 		DebugWarner
 	>;

diff --git a/...ntal/analysis/FunctionDependencyGraph.cpp → ...l/analysis/FunctionDependencyAnalysis.cpp b/...ntal/analysis/FunctionDependencyGraph.cpp → ...l/analysis/FunctionDependencyAnalysis.cpp
@@ -17,39 +17,42 @@
 // SPDX-License-Identifier: GPL-3.0
 
 #include <libsolidity/experimental/analysis/Analysis.h>
-#include <libsolidity/experimental/analysis/FunctionDependencyGraph.h>
+#include <libsolidity/experimental/analysis/FunctionDependencyAnalysis.h>
 
 using namespace solidity::frontend::experimental;
 using namespace solidity::util;
 
-FunctionDependencyGraph::FunctionDependencyGraph(Analysis& _analysis):
+FunctionDependencyAnalysis::FunctionDependencyAnalysis(Analysis& _analysis):
 	m_analysis(_analysis),
 	m_errorReporter(_analysis.errorReporter())
 {
 }
 
-bool FunctionDependencyGraph::analyze(SourceUnit const& _sourceUnit)
+bool FunctionDependencyAnalysis::analyze(SourceUnit const& _sourceUnit)
 {
 	_sourceUnit.accept(*this);
 	return !m_errorReporter.hasErrors();
 }
 
-bool FunctionDependencyGraph::visit(FunctionDefinition const& _functionDefinition)
+bool FunctionDependencyAnalysis::visit(FunctionDefinition const& _functionDefinition)
 {
 	solAssert(!m_currentFunction);
 	m_currentFunction = &_functionDefinition;
+	// Insert a function definition pointer that maps to an empty set; the pointed to set will later be
+	// populated in ``endVisit(Identifier const& _identifier)`` if ``m_currentFunction`` references another.
+	auto [_, inserted] = annotation().functionCallGraph.edges.try_emplace(
+		m_currentFunction, std::set<FunctionDefinition const*, ASTCompareByID<FunctionDefinition>>{}
+	);
+	solAssert(inserted);
 	return true;
 }
 
-void FunctionDependencyGraph::endVisit(FunctionDefinition const&)
+void FunctionDependencyAnalysis::endVisit(FunctionDefinition const&)
 {
-	// If we're done visiting a function declaration without said function referencing/calling
-	// another function in its body - insert it into the graph without child nodes.
-	annotation().functionCallGraph.edges.try_emplace(m_currentFunction, std::set<FunctionDefinition const*, CallGraph::CompareByID>{});
 	m_currentFunction = nullptr;
 }
 
-void FunctionDependencyGraph::endVisit(Identifier const& _identifier)
+void FunctionDependencyAnalysis::endVisit(Identifier const& _identifier)
 {
 	auto const* callee = dynamic_cast<FunctionDefinition const*>(_identifier.annotation().referencedDeclaration);
 	// Check that the identifier is within a function body and is a function, and add it to the graph
@@ -58,12 +61,12 @@ void FunctionDependencyGraph::endVisit(Identifier const& _identifier)
 		addEdge(m_currentFunction, callee);
 }
 
-void FunctionDependencyGraph::addEdge(FunctionDefinition const* _caller, FunctionDefinition const* _callee)
+void FunctionDependencyAnalysis::addEdge(FunctionDefinition const* _caller, FunctionDefinition const* _callee)
 {
 	annotation().functionCallGraph.edges[_caller].insert(_callee);
 }
 
-FunctionDependencyGraph::GlobalAnnotation& FunctionDependencyGraph::annotation()
+FunctionDependencyAnalysis::GlobalAnnotation& FunctionDependencyAnalysis::annotation()
 {
-	return m_analysis.annotation<FunctionDependencyGraph>();
+	return m_analysis.annotation<FunctionDependencyAnalysis>();
 }
diff --git a/...mental/analysis/FunctionDependencyGraph.h → ...tal/analysis/FunctionDependencyAnalysis.h b/...mental/analysis/FunctionDependencyGraph.h → ...tal/analysis/FunctionDependencyAnalysis.h
@@ -21,7 +21,7 @@
 #include <liblangutil/ErrorReporter.h>
 #include <libsolidity/ast/ASTForward.h>
 #include <libsolidity/ast/ASTVisitor.h>
-#include <libsolidity/experimental/ast/CallGraph.h>
+#include <libsolidity/experimental/ast/FunctionCallGraph.h>
 
 #include <memory>
 
@@ -30,16 +30,16 @@ namespace solidity::frontend::experimental
 
 class Analysis;
 
-class FunctionDependencyGraph: private ASTConstVisitor
+class FunctionDependencyAnalysis: private ASTConstVisitor
 {
 public:
-	FunctionDependencyGraph(Analysis& _analysis);
+	FunctionDependencyAnalysis(Analysis& _analysis);
 	bool analyze(SourceUnit const& _sourceUnit);
 
 	struct Annotation {};
 	struct GlobalAnnotation
 	{
-		CallGraph functionCallGraph;
+		FunctionDependencyGraph functionCallGraph;
 	};
 
 private:

diff --git a/libsolidity/experimental/ast/CallGraph.cpp b/libsolidity/experimental/ast/CallGraph.cpp
diff --git a/libsolidity/experimental/ast/CallGraph.h → ...dity/experimental/ast/FunctionCallGraph.h b/libsolidity/experimental/ast/CallGraph.h → ...dity/experimental/ast/FunctionCallGraph.h
@@ -16,7 +16,7 @@
 */
 // SPDX-License-Identifier: GPL-3.0
 
-/// Data structure representing a function call graph.
+/// Data structure representing a function dependency graph.
 
 #pragma once
 
@@ -29,21 +29,12 @@
 namespace solidity::frontend::experimental
 {
 
-struct CallGraph
+struct FunctionDependencyGraph
 {
-	struct CompareByID
-	{
-		using is_transparent = void;
-		bool operator()(FunctionDefinition const* _lhs, FunctionDefinition const* _rhs) const;
-		bool operator()(FunctionDefinition const* _lhs, int64_t _rhs) const;
-		bool operator()(int64_t _lhs, FunctionDefinition const* _rhs) const;
-	};
 	/// Graph edges. Edges are directed and lead from the caller to the callee.
-	/// The map contains a key for every possible caller, even if does not actually perform
+	/// The map contains a key for every function, even if does not actually perform
 	/// any calls.
-	std::map<FunctionDefinition const*, std::set<FunctionDefinition const*, CompareByID>, CompareByID> edges;
+	std::map<FunctionDefinition const*, std::set<FunctionDefinition const*, ASTCompareByID<FunctionDefinition>>, ASTCompareByID<FunctionDefinition>> edges;
 };
 
-std::ostream& operator<<(std::ostream& _out, CallGraph const& _callGraph);
-
 }
diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp
@@ -57,7 +57,7 @@
 #include <libsolidity/parsing/Parser.h>
 
 #include <libsolidity/experimental/analysis/Analysis.h>
-#include <libsolidity/experimental/analysis/FunctionDependencyGraph.h>
+#include <libsolidity/experimental/analysis/FunctionDependencyAnalysis.h>
 #include <libsolidity/experimental/codegen/IRGenerator.h>
 
 #include <libsolidity/codegen/ir/Common.h>

diff --git a/test/InteractiveTests.h b/test/InteractiveTests.h
@@ -22,7 +22,7 @@
 #include <test/libsolidity/ABIJsonTest.h>
 #include <test/libsolidity/ASTJSONTest.h>
 #include <test/libsolidity/ASTPropertyTest.h>
-#include "libsolidity/FunctionDependencyGraphTest.h"
+#include <libsolidity/FunctionDependencyGraphTest.h>
 #include <test/libsolidity/GasTest.h>
 #include <test/libsolidity/MemoryGuardTest.h>
 #include <test/libsolidity/NatspecJSONTest.h>

diff --git a/test/libsolidity/FunctionDependencyGraphTest.cpp b/test/libsolidity/FunctionDependencyGraphTest.cpp
@@ -16,10 +16,10 @@
 */
 // SPDX-License-Identifier: GPL-3.0
 
-#include "FunctionDependencyGraphTest.h"
+#include <test/libsolidity/FunctionDependencyGraphTest.h>
 
 #include <libsolidity/experimental/analysis/Analysis.h>
-#include "libsolidity/experimental/analysis/FunctionDependencyGraph.h"
+#include <libsolidity/experimental/analysis/FunctionDependencyAnalysis.h>
 #include <libyul/backends/evm/EVMDialect.h>
 #include <libyul/optimiser/FunctionCallFinder.h>
 
@@ -46,7 +46,7 @@ TestCase::TestResult FunctionDependencyGraphTest::run(std::ostream& _stream, std
 	}
 
 	m_obtainedResult.clear();
-	for (auto [top, subs]: compiler().experimentalAnalysis().annotation<experimental::FunctionDependencyGraph>().functionCallGraph.edges)
+	for (auto [top, subs]: compiler().experimentalAnalysis().annotation<experimental::FunctionDependencyAnalysis>().functionCallGraph.edges)
 	{
 		std::string topName = top->name().empty() ? "fallback" : top->name();
 		m_obtainedResult += "(" + topName + ") --> {";

diff --git a/test/libsolidity/analysis/FunctionCallGraph.cpp b/test/libsolidity/analysis/FunctionCallGraph.cpp
@@ -16,7 +16,7 @@
 */
 // SPDX-License-Identifier: GPL-3.0
 
-/// Unit tests for libsolidity/analysis/FunctionDependencyGraph.h
+/// Unit tests for libsolidity/analysis/FunctionCallGraph.h
 
 #include <libsolidity/analysis/FunctionCallGraph.h>