From d7f08f7a232b91515f535a81da63e9cf6cac815f Mon Sep 17 00:00:00 2001 From: Ti Liang Date: Fri, 10 Jun 2022 13:12:21 -0400 Subject: [PATCH 1/3] Add weak dependencies analysis for include graph validation Signed-off-by: Ti Liang --- .../include_graph_dependencies.h | 31 ++ src/include_graph_dependencies.cpp | 68 ++++ t/031-validate-include-graph.t.cpp | 302 +++++++++++++++++- t/data/031-validate-include-graph/def7.h | 6 + t/data/031-validate-include-graph/foo.cpp | 3 + t/data/031-validate-include-graph/g.h | 6 + 6 files changed, 400 insertions(+), 16 deletions(-) create mode 100644 t/data/031-validate-include-graph/def7.h create mode 100644 t/data/031-validate-include-graph/g.h diff --git a/include/clangmetatool/include_graph_dependencies.h b/include/clangmetatool/include_graph_dependencies.h index 71bfb6d..c16a5c6 100644 --- a/include/clangmetatool/include_graph_dependencies.h +++ b/include/clangmetatool/include_graph_dependencies.h @@ -5,6 +5,7 @@ namespace clangmetatool { + /** * Collect stateless functions to query and and modify the state of * dependencies of a given `clangmetatool::IncludeGraphData` structure. @@ -46,6 +47,36 @@ struct IncludeGraphDependencies { static std::set liveDependencies(const clangmetatool::collectors::IncludeGraphData *data, const clangmetatool::types::FileUID &headerFUID); + + /* + * A data structure for include graph weak dependencies analysis + * + * Key: all requires headers + * Value: a set of direct included headers that could access the key header + * + * Example: + * { "def1.h": {"a.h", "b.h"}, + * "def2.h": {"a.h", "c.h"} } + * + * So that given the example data above it means current analyzing file: + * - requires definitions from "def1.h", "def2.h" directly + * - includes "a.h", "b.h", "c.h" + * - can access "def1.h" by "a.h" and "b.h" + * - can access "def2.h" by "a.h" and "c.h" + */ + typedef std::map> RequiresMap; + + /** + * Get the 'live' weak dependencies of a header within the given include graph. + * This will return data structure(called RequiresMap) shows weak dependencies of headers + * + * (See docstring of RequiresMap typedef above for more details of the data structure) + */ + static RequiresMap + liveWeakDependencies(const clangmetatool::collectors::IncludeGraphData *data, + const clangmetatool::types::FileUID &headerFUID); + }; // struct IncludeGraphDependencies } // namespace clangmetatool diff --git a/src/include_graph_dependencies.cpp b/src/include_graph_dependencies.cpp index 13d13e6..fe875ae 100644 --- a/src/include_graph_dependencies.cpp +++ b/src/include_graph_dependencies.cpp @@ -138,4 +138,72 @@ std::set IncludeGraphDependencies::liveDependencies( return dependencies; } + +/* + * Traverse the include graph from `fromNode` to all accessible node using BFS + * for `forNode` and update given RequiresMap. + * + * For any node that `usage_reference_count[{fromNode, toNode}] > 0`, add a record + * `{toNode: [fromNode]}` to requiresMap, means that `forNode` needs to access + * resource defined in `toNode` through `fromNode` + * + * Include graph should looks like: + * forNode -> fromNode ... -> toNode + */ +void traverseFor(const types::FileUID &fromNode, const types::FileUID &forNode, + const clangmetatool::collectors::IncludeGraphData *data, + std::set &knownNodes, + IncludeGraphDependencies::RequiresMap& requiresMap){ + std::queue filesToProcess; + + filesToProcess.push(fromNode); + + while (!filesToProcess.empty()) { + auto currentFUID = filesToProcess.front(); + filesToProcess.pop(); + + types::FileGraphEdge currentEdge{forNode, currentFUID}; + auto refCountIt = data->usage_reference_count.find(currentEdge); + // the include graph looks like + // forNode -> fromNode -> ... -> currentFUID + if (refCountIt != data->usage_reference_count.end() && + refCountIt->second > 0) { + requiresMap[currentFUID].emplace(fromNode); + } + + // Find the set of files included by the current file uid + // and set those up for traversal if we haven't seen them already + types::FileGraph::const_iterator rangeBegin, rangeEnd; + std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, currentFUID); + + for (auto edgeIt = rangeBegin; edgeIt != rangeEnd; ++edgeIt) { + types::FileUID nextNode; + std::tie(std::ignore, nextNode) = *edgeIt; + if (knownNodes.find(nextNode) == knownNodes.end()) { + filesToProcess.push(nextNode); + knownNodes.insert(nextNode); + } + } + } + +} + +IncludeGraphDependencies::RequiresMap +IncludeGraphDependencies::liveWeakDependencies( + const clangmetatool::collectors::IncludeGraphData *data, + const clangmetatool::types::FileUID &headerFUID){ + IncludeGraphDependencies::RequiresMap requiresMap; + + types::FileGraph::const_iterator rangeBegin, rangeEnd; + std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, headerFUID); + + for (auto it = rangeBegin; it != rangeEnd; ++it) { + assert(it->first == headerFUID); + std::set knownNodes; + traverseFor(it->second, headerFUID, data, knownNodes, requiresMap); + } + + return requiresMap; +} + } // namespace clangmetatool diff --git a/t/031-validate-include-graph.t.cpp b/t/031-validate-include-graph.t.cpp index 515ff45..66a5680 100644 --- a/t/031-validate-include-graph.t.cpp +++ b/t/031-validate-include-graph.t.cpp @@ -19,13 +19,13 @@ #include #include -class MyTool { +class DependenciesTool { private: clang::CompilerInstance *ci; clangmetatool::collectors::IncludeGraph includeGraph; public: - MyTool(clang::CompilerInstance *ci, clang::ast_matchers::MatchFinder *f) + DependenciesTool(clang::CompilerInstance *ci, clang::ast_matchers::MatchFinder *f) : ci(ci), includeGraph(ci, f) {} void postProcessing( std::map &replacementMap) { @@ -67,6 +67,12 @@ class MyTool { // | ^ // | | // +-> level.h + // | + // | + // +-> g.h + // | | + // | v + // +-> def7.h (G) // // It should be noted that foo.cpp: // * does not depend on a.h, as it provides no symbols used by foo @@ -75,6 +81,7 @@ class MyTool { // * Received 'D1', 'D2' through d.h // * Received 'E' through both e.h, diam.h // * Received 'F' through both f.h, level.h + // * Received 'G' through both g.h, def7.h // foo.cpp: b.h c.h d.h e.h f.h // (a.h provides nothing, diam.h adds no new symbols) @@ -84,13 +91,14 @@ class MyTool { fname2uid["c.h"], fname2uid["d.h"], fname2uid["e.h"], - fname2uid["f.h"] + fname2uid["f.h"], + fname2uid["g.h"] }), clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) ); // Total refcount of def1.h is 0 now, foo no longer depends on b.h - // foo.cpp: c.h d.h e.h f.h + // foo.cpp: c.h d.h e.h f.h g.h clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( data, {mfid, fname2uid["def1.h"]} ); @@ -99,13 +107,15 @@ class MyTool { fname2uid["c.h"], fname2uid["d.h"], fname2uid["e.h"], - fname2uid["f.h"] + fname2uid["f.h"], + fname2uid["g.h"] + }), clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) ); // Total refcount of def2.h is now 0, foo.cpp no longer depends on c.h - // foo.cpp: d.h e.h f.h + // foo.cpp: d.h e.h f.h g.h clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( data, {mfid, fname2uid["def2.h"]} ); @@ -113,7 +123,9 @@ class MyTool { std::set({ fname2uid["d.h"], fname2uid["e.h"], - fname2uid["f.h"] + fname2uid["f.h"], + fname2uid["g.h"] + }), clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) ); @@ -127,39 +139,58 @@ class MyTool { std::set({ fname2uid["d.h"], fname2uid["e.h"], - fname2uid["f.h"] + fname2uid["f.h"], + fname2uid["g.h"] + }), clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) ); // Total refcount for def4.h is now 0, foo.cpp no longer depends on d.h - // foo.cpp: e.h f.h + // foo.cpp: e.h f.h g.h clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( data, {mfid, fname2uid["def4.h"]} ); EXPECT_EQ( std::set({ fname2uid["e.h"], - fname2uid["f.h"] + fname2uid["f.h"], + fname2uid["g.h"] + }), clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) ); + // Total refcount for def5.h is now 0, foo.cpp no longer depends on e.h + // foo.cpp: f.h g.h clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( data, {mfid, fname2uid["def5.h"]} ); - // foo.cpp only depends on f.h now EXPECT_EQ( std::set({ - fname2uid["f.h"] + fname2uid["f.h"], + fname2uid["g.h"] + }), clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) ); - // Total refcount of def6.h is 0 now, foo.cpp now longer depends on f.h clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( data, {mfid, fname2uid["def6.h"]} ); + + // foo.cpp: g.h + EXPECT_EQ( + std::set({ + fname2uid["g.h"] + }), + clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) + ); + + // Total refcount of def6.h is 0 now, foo.cpp now longer depends on f.h + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def7.h"]} + ); // Dependency set of foo.h is empty EXPECT_TRUE( clangmetatool::IncludeGraphDependencies::liveDependencies(data, mfid) @@ -174,6 +205,7 @@ class MyTool { fname2uid["d.h"], fname2uid["e.h"], fname2uid["f.h"], + fname2uid["g.h"], fname2uid["diam.h"], fname2uid["level.h"], fname2uid["mid.h"], @@ -182,7 +214,8 @@ class MyTool { fname2uid["def3.h"], fname2uid["def4.h"], fname2uid["def5.h"], - fname2uid["def6.h"] + fname2uid["def6.h"], + fname2uid["def7.h"] }), clangmetatool::IncludeGraphDependencies::collectAllIncludes(data, fname2uid["foo.cpp"]) ); @@ -196,7 +229,7 @@ class MyTool { } }; -TEST(include_validate_test, basic) { +TEST(include_validate_test, liveDependencies) { llvm::cl::OptionCategory MyToolCategory("my-tool options"); int argc = 4; @@ -212,9 +245,246 @@ TEST(include_validate_test, basic) { clang::tooling::RefactoringTool tool(optionsParser.getCompilations(), optionsParser.getSourcePathList()); - clangmetatool::MetaToolFactory> raf( + clangmetatool::MetaToolFactory> raf( tool.getReplacements()); int r = tool.runAndSave(&raf); ASSERT_EQ(0, r); } + +class WeakDependenciesTool { +private: + clang::CompilerInstance *ci; + clangmetatool::collectors::IncludeGraph includeGraph; + +public: + WeakDependenciesTool(clang::CompilerInstance *ci, + clang::ast_matchers::MatchFinder *f) + : ci(ci), includeGraph(ci, f) {} + void postProcessing( + std::map &replacementMap) { + clangmetatool::collectors::IncludeGraphData *data = includeGraph.getData(); + + std::map fname2uid; + for (auto itr = data->fuid2name.begin(); itr != data->fuid2name.end(); + ++itr) { + fname2uid[itr->second] = itr->first; + } + + // Get main file UID + clang::SourceManager &sm = ci->getSourceManager(); + const clang::FileEntry *mfe = sm.getFileEntryForID(sm.getMainFileID()); + clangmetatool::types::FileUID mfid = mfe->getUID(); + + // clang-format: off + // Symbol counts needed by foo: + // [B: 1, C: 1, D1: 1, D2: 1, E: 1, F: 1, ] + // + // The include graph for foo looks like so, with parentheses containing + // symbols that are provided by that header used by foo.cpp: + // + // +-> a.h () + // | + // +-> b.h -> def1.h (B) + // | + // foo.cpp +-> c.h -> mid.h -> def2.h (C) + // | + // +-> d.h +-> def3.h (D1) + // | | + // | +-> def4.h (D2) + // | + // +-> e.h -----+--> def5.h (E) + // | | + // +-> diam.h --+ + // | + // +-> f.h -> def6.h (F) + // | ^ + // | | + // +-> level.h + // | + // | + // +-> g.h + // | | + // | v + // +-> def7.h (G) + // + // It should be noted that foo.cpp: + // * does not depend on a.h, as it provides no symbols used by foo + // * access def1.h through b.h + // * access def2.h through c.h + // * access def3.h through d.h + // * access def4.h through d.h + // * access def5.h through e.h and diam.h + // * access def6.h through f.h and level.h + // * access def7.h through g.h and def7.h + + EXPECT_EQ( + clangmetatool::IncludeGraphDependencies::RequiresMap( + {std::make_pair( + fname2uid["def1.h"], + std::set{fname2uid["b.h"]}), + std::make_pair( + fname2uid["def2.h"], + std::set{fname2uid["c.h"]}), + std::make_pair( + fname2uid["def3.h"], + std::set{fname2uid["d.h"]}), + std::make_pair( + fname2uid["def4.h"], + std::set{fname2uid["d.h"]}), + std::make_pair(fname2uid["def5.h"], + std::set{ + fname2uid["e.h"], fname2uid["diam.h"]}), + std::make_pair(fname2uid["def6.h"], + std::set{ + fname2uid["f.h"], fname2uid["level.h"]}), + std::make_pair(fname2uid["def7.h"], + std::set{ + fname2uid["g.h"], fname2uid["def7.h"]})}), + clangmetatool::IncludeGraphDependencies::liveWeakDependencies(data, + mfid)); + + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def1.h"]}); + + // def1.h no longer needed + EXPECT_EQ( + clangmetatool::IncludeGraphDependencies::RequiresMap( + {std::make_pair( + fname2uid["def2.h"], + std::set{fname2uid["c.h"]}), + std::make_pair( + fname2uid["def3.h"], + std::set{fname2uid["d.h"]}), + std::make_pair( + fname2uid["def4.h"], + std::set{fname2uid["d.h"]}), + std::make_pair(fname2uid["def5.h"], + std::set{ + fname2uid["e.h"], fname2uid["diam.h"]}), + std::make_pair(fname2uid["def6.h"], + std::set{ + fname2uid["f.h"], fname2uid["level.h"]}), + std::make_pair(fname2uid["def7.h"], + std::set{ + fname2uid["g.h"], fname2uid["def7.h"]})}), + clangmetatool::IncludeGraphDependencies::liveWeakDependencies(data, + mfid)); + + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def2.h"]}); + + // def2.h no longer needed + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + {std::make_pair(fname2uid["def3.h"], + std::set{ + fname2uid["d.h"]}), + std::make_pair(fname2uid["def4.h"], + std::set{ + fname2uid["d.h"]}), + std::make_pair(fname2uid["def5.h"], + std::set{ + fname2uid["e.h"], fname2uid["diam.h"]}), + std::make_pair(fname2uid["def6.h"], + std::set{ + fname2uid["f.h"], fname2uid["level.h"]}), + std::make_pair(fname2uid["def7.h"], + std::set{ + fname2uid["g.h"], fname2uid["def7.h"]})}), + clangmetatool::IncludeGraphDependencies::liveWeakDependencies( + data, mfid)); + + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def3.h"]}); + + // def3.h no longer needed + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + {std::make_pair(fname2uid["def4.h"], + std::set{ + fname2uid["d.h"]}), + std::make_pair(fname2uid["def5.h"], + std::set{ + fname2uid["e.h"], fname2uid["diam.h"]}), + std::make_pair(fname2uid["def6.h"], + std::set{ + fname2uid["f.h"], fname2uid["level.h"]}), + std::make_pair(fname2uid["def7.h"], + std::set{ + fname2uid["g.h"], fname2uid["def7.h"]})}), + clangmetatool::IncludeGraphDependencies::liveWeakDependencies( + data, mfid)); + + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def4.h"]}); + + // def4.h no longer needed + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + {std::make_pair(fname2uid["def5.h"], + std::set{ + fname2uid["e.h"], fname2uid["diam.h"]}), + std::make_pair(fname2uid["def6.h"], + std::set{ + fname2uid["f.h"], fname2uid["level.h"]}), + std::make_pair(fname2uid["def7.h"], + std::set{ + fname2uid["g.h"], fname2uid["def7.h"]})}), + clangmetatool::IncludeGraphDependencies::liveWeakDependencies( + data, mfid)); + + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def5.h"]}); + + // def5.h no longer needed + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + {std::make_pair(fname2uid["def6.h"], + std::set{ + fname2uid["f.h"], fname2uid["level.h"]}), + std::make_pair(fname2uid["def7.h"], + std::set{ + fname2uid["g.h"], fname2uid["def7.h"]})}), + clangmetatool::IncludeGraphDependencies::liveWeakDependencies( + data, mfid)); + + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def6.h"]}); + + // def6.h no longer needed + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + {std::make_pair(fname2uid["def7.h"], + std::set{ + fname2uid["g.h"], fname2uid["def7.h"]})}), + clangmetatool::IncludeGraphDependencies::liveWeakDependencies( + data, mfid)); + + clangmetatool::IncludeGraphDependencies::decrementUsageRefCount( + data, {mfid, fname2uid["def7.h"]}); + + // def7.h no longer needed + EXPECT_TRUE(clangmetatool::IncludeGraphDependencies::liveWeakDependencies( + data, mfid) + .empty()); + } +}; + +TEST(include_validate_test, liveWeakDependencies) { + llvm::cl::OptionCategory MyToolCategory("my-tool options"); + + int argc = 4; + const char *argv[] = { + "foo", CMAKE_SOURCE_DIR "/t/data/031-validate-include-graph/foo.cpp", + "--", "-xc++"}; + + auto result = clang::tooling::CommonOptionsParser::create( + argc, argv, MyToolCategory, llvm::cl::OneOrMore); + ASSERT_TRUE(!!result); + clang::tooling::CommonOptionsParser &optionsParser = result.get(); + + clang::tooling::RefactoringTool tool(optionsParser.getCompilations(), + optionsParser.getSourcePathList()); + + clangmetatool::MetaToolFactory> + raf(tool.getReplacements()); + + int r = tool.runAndSave(&raf); + ASSERT_EQ(0, r); +} diff --git a/t/data/031-validate-include-graph/def7.h b/t/data/031-validate-include-graph/def7.h new file mode 100644 index 0000000..062ab0a --- /dev/null +++ b/t/data/031-validate-include-graph/def7.h @@ -0,0 +1,6 @@ +#ifndef INCLUDED_DEF7_H +#define INCLUDED_DEF7_H + +#define G 7 + +#endif diff --git a/t/data/031-validate-include-graph/foo.cpp b/t/data/031-validate-include-graph/foo.cpp index 04085da..c7310ec 100644 --- a/t/data/031-validate-include-graph/foo.cpp +++ b/t/data/031-validate-include-graph/foo.cpp @@ -6,9 +6,12 @@ #include "diam.h" #include "f.h" #include "level.h" +#include "g.h" +#include "def7.h" int fb() { return B; } int fc() { return C; } int fd() { return D1 + D2; } int fe() { return E;} int ff() { return F;} +int fg() { return G;} diff --git a/t/data/031-validate-include-graph/g.h b/t/data/031-validate-include-graph/g.h new file mode 100644 index 0000000..aee8948 --- /dev/null +++ b/t/data/031-validate-include-graph/g.h @@ -0,0 +1,6 @@ +#ifndef INCLUDED_G_H +#define INCLUDED_G_H + +#include "def7.h" + +#endif From 5030e698906c17b2d681a27cbb5238a402893a84 Mon Sep 17 00:00:00 2001 From: Ti Liang Date: Thu, 16 Jun 2022 13:48:35 -0400 Subject: [PATCH 2/3] Renaming Signed-off-by: Ti Liang --- .../include_graph_dependencies.h | 20 ++--- src/include_graph_dependencies.cpp | 74 +++++++++---------- t/031-validate-include-graph.t.cpp | 14 ++-- 3 files changed, 55 insertions(+), 53 deletions(-) diff --git a/include/clangmetatool/include_graph_dependencies.h b/include/clangmetatool/include_graph_dependencies.h index c16a5c6..b9d2f56 100644 --- a/include/clangmetatool/include_graph_dependencies.h +++ b/include/clangmetatool/include_graph_dependencies.h @@ -46,36 +46,38 @@ struct IncludeGraphDependencies { */ static std::set liveDependencies(const clangmetatool::collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &headerFUID); + const clangmetatool::types::FileUID &fileFUID); /* * A data structure for include graph weak dependencies analysis + * for a specific source file * - * Key: all requires headers - * Value: a set of direct included headers that could access the key header + * Key: all headers the file directly depends on + * Value: a set of direct included headers that could allow the file to + * access the key header * * Example: * { "def1.h": {"a.h", "b.h"}, * "def2.h": {"a.h", "c.h"} } * * So that given the example data above it means current analyzing file: - * - requires definitions from "def1.h", "def2.h" directly + * - depends on definitions from "def1.h", "def2.h" directly * - includes "a.h", "b.h", "c.h" * - can access "def1.h" by "a.h" and "b.h" * - can access "def2.h" by "a.h" and "c.h" */ typedef std::map> RequiresMap; + std::set> DirectDependenciesMap; /** * Get the 'live' weak dependencies of a header within the given include graph. - * This will return data structure(called RequiresMap) shows weak dependencies of headers + * This will return data structure(called DirectDependenciesMap) shows weak dependencies of headers * - * (See docstring of RequiresMap typedef above for more details of the data structure) + * (See docstring of DirectDependenciesMap typedef above for more details of the data structure) */ - static RequiresMap + static DirectDependenciesMap liveWeakDependencies(const clangmetatool::collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &headerFUID); + const clangmetatool::types::FileUID &fileFUID); }; // struct IncludeGraphDependencies } // namespace clangmetatool diff --git a/src/include_graph_dependencies.cpp b/src/include_graph_dependencies.cpp index fe875ae..33add25 100644 --- a/src/include_graph_dependencies.cpp +++ b/src/include_graph_dependencies.cpp @@ -6,11 +6,11 @@ namespace clangmetatool { namespace { -// Returns a range of edges whose source vertex matches the given file uid +// Returns a range of edges starts with given source file uid inline std::pair -edge_range_with_source(const collectors::IncludeGraphData *data, - const types::FileUID &sourceFUID) { +edgeRangeStartsWith(const collectors::IncludeGraphData *data, + const types::FileUID &sourceFUID) { // Exploit an implementation detail of the include graph being an ordered // set of pairs and how operator<(...) on pairs works. // The property in use is that operator<(...) on pairs sorts @@ -32,10 +32,10 @@ edge_range_with_source(const collectors::IncludeGraphData *data, // This function depends on the current view of the graph to check an edge // is important. There are more than one valid solutions to this problem on // a DAG, affected by the order of traversal and the initial state provided. -bool requires(const collectors::IncludeGraphData *data, - const types::FileUID &from, const types::FileUID &to, - std::set &knownEdges, - std::set &knownNodes) { +bool isRequired(const collectors::IncludeGraphData *data, + const types::FileUID &from, const types::FileUID &to, + std::set &knownEdges, + std::set &knownNodes) { std::queue filesToProcess; bool keepEdge = false; @@ -62,7 +62,7 @@ bool requires(const collectors::IncludeGraphData *data, // Find the set of files included by the current file uid // and set those up for traversal if we haven't seen them already types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, currentFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, currentFUID); for (auto edgeIt = rangeBegin; edgeIt != rangeEnd; ++edgeIt) { types::FileUID nextNode; @@ -93,14 +93,14 @@ bool IncludeGraphDependencies::decrementUsageRefCount( std::set IncludeGraphDependencies::collectAllIncludes( const clangmetatool::collectors::IncludeGraphData* data, - const types::FileUID &headerFUID) + const types::FileUID &fileFUID) { types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, headerFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileFUID); std::set visitedNodes; std::queue toVisit; - toVisit.push(headerFUID); + toVisit.push(fileFUID); while (!toVisit.empty()) { auto currentFUID = toVisit.front(); toVisit.pop(); @@ -109,7 +109,7 @@ IncludeGraphDependencies::collectAllIncludes( continue; } types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, currentFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, currentFUID); for (auto it = rangeBegin; it != rangeEnd; ++it) { toVisit.push(it->second); } @@ -120,18 +120,18 @@ IncludeGraphDependencies::collectAllIncludes( std::set IncludeGraphDependencies::liveDependencies( const collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &headerFUID) { + const clangmetatool::types::FileUID &fileFUID) { std::set dependencies; std::set visitedEdges; std::set visitedNodes; types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, headerFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileFUID); for (auto it = rangeBegin; it != rangeEnd; ++it) { - assert(it->first == headerFUID); + assert(it->first == fileFUID); auto &dependency = it->second; - if (requires(data, headerFUID, dependency, visitedEdges, visitedNodes)) { + if (isRequired(data, fileFUID, dependency, visitedEdges, visitedNodes)) { dependencies.insert(dependency); } } @@ -140,41 +140,41 @@ std::set IncludeGraphDependencies::liveDependencies( } /* - * Traverse the include graph from `fromNode` to all accessible node using BFS - * for `forNode` and update given RequiresMap. + * Traverse the include graph for `forNode` start from `rootNode` to all + * accessible node using BFS and update given DirectDependenciesMap. * - * For any node that `usage_reference_count[{fromNode, toNode}] > 0`, add a record - * `{toNode: [fromNode]}` to requiresMap, means that `forNode` needs to access - * resource defined in `toNode` through `fromNode` + * For any node that `usage_reference_count[{rootNode, toNode}] > 0`, add a record + * `{toNode: [rootNode]}` to depsMap, means that `forNode` needs to access + * resource defined in `toNode` through `rootNode` * * Include graph should looks like: - * forNode -> fromNode ... -> toNode + * forNode -> rootNode ... -> toNode */ -void traverseFor(const types::FileUID &fromNode, const types::FileUID &forNode, +void traverseFor(const types::FileUID &forNode, const types::FileUID &rootNode, const clangmetatool::collectors::IncludeGraphData *data, std::set &knownNodes, - IncludeGraphDependencies::RequiresMap& requiresMap){ + IncludeGraphDependencies::DirectDependenciesMap& depsMap){ std::queue filesToProcess; - filesToProcess.push(fromNode); + filesToProcess.push(rootNode); while (!filesToProcess.empty()) { - auto currentFUID = filesToProcess.front(); + auto toNode = filesToProcess.front(); filesToProcess.pop(); - types::FileGraphEdge currentEdge{forNode, currentFUID}; + types::FileGraphEdge currentEdge{forNode, toNode}; auto refCountIt = data->usage_reference_count.find(currentEdge); // the include graph looks like - // forNode -> fromNode -> ... -> currentFUID + // forNode -> rootNode -> ... -> toNode if (refCountIt != data->usage_reference_count.end() && refCountIt->second > 0) { - requiresMap[currentFUID].emplace(fromNode); + depsMap[toNode].emplace(rootNode); } // Find the set of files included by the current file uid // and set those up for traversal if we haven't seen them already types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, currentFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, toNode); for (auto edgeIt = rangeBegin; edgeIt != rangeEnd; ++edgeIt) { types::FileUID nextNode; @@ -188,22 +188,22 @@ void traverseFor(const types::FileUID &fromNode, const types::FileUID &forNode, } -IncludeGraphDependencies::RequiresMap +IncludeGraphDependencies::DirectDependenciesMap IncludeGraphDependencies::liveWeakDependencies( const clangmetatool::collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &headerFUID){ - IncludeGraphDependencies::RequiresMap requiresMap; + const clangmetatool::types::FileUID &fileFUID){ + IncludeGraphDependencies::DirectDependenciesMap depsMap; types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edge_range_with_source(data, headerFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileFUID); for (auto it = rangeBegin; it != rangeEnd; ++it) { - assert(it->first == headerFUID); + assert(it->first == fileFUID); std::set knownNodes; - traverseFor(it->second, headerFUID, data, knownNodes, requiresMap); + traverseFor(fileFUID, it->second, data, knownNodes, depsMap); } - return requiresMap; + return depsMap; } } // namespace clangmetatool diff --git a/t/031-validate-include-graph.t.cpp b/t/031-validate-include-graph.t.cpp index 66a5680..eeb65c1 100644 --- a/t/031-validate-include-graph.t.cpp +++ b/t/031-validate-include-graph.t.cpp @@ -319,7 +319,7 @@ class WeakDependenciesTool { // * access def7.h through g.h and def7.h EXPECT_EQ( - clangmetatool::IncludeGraphDependencies::RequiresMap( + clangmetatool::IncludeGraphDependencies::DirectDependenciesMap( {std::make_pair( fname2uid["def1.h"], std::set{fname2uid["b.h"]}), @@ -349,7 +349,7 @@ class WeakDependenciesTool { // def1.h no longer needed EXPECT_EQ( - clangmetatool::IncludeGraphDependencies::RequiresMap( + clangmetatool::IncludeGraphDependencies::DirectDependenciesMap( {std::make_pair( fname2uid["def2.h"], std::set{fname2uid["c.h"]}), @@ -375,7 +375,7 @@ class WeakDependenciesTool { data, {mfid, fname2uid["def2.h"]}); // def2.h no longer needed - EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::DirectDependenciesMap( {std::make_pair(fname2uid["def3.h"], std::set{ fname2uid["d.h"]}), @@ -398,7 +398,7 @@ class WeakDependenciesTool { data, {mfid, fname2uid["def3.h"]}); // def3.h no longer needed - EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::DirectDependenciesMap( {std::make_pair(fname2uid["def4.h"], std::set{ fname2uid["d.h"]}), @@ -418,7 +418,7 @@ class WeakDependenciesTool { data, {mfid, fname2uid["def4.h"]}); // def4.h no longer needed - EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::DirectDependenciesMap( {std::make_pair(fname2uid["def5.h"], std::set{ fname2uid["e.h"], fname2uid["diam.h"]}), @@ -435,7 +435,7 @@ class WeakDependenciesTool { data, {mfid, fname2uid["def5.h"]}); // def5.h no longer needed - EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::DirectDependenciesMap( {std::make_pair(fname2uid["def6.h"], std::set{ fname2uid["f.h"], fname2uid["level.h"]}), @@ -449,7 +449,7 @@ class WeakDependenciesTool { data, {mfid, fname2uid["def6.h"]}); // def6.h no longer needed - EXPECT_EQ(clangmetatool::IncludeGraphDependencies::RequiresMap( + EXPECT_EQ(clangmetatool::IncludeGraphDependencies::DirectDependenciesMap( {std::make_pair(fname2uid["def7.h"], std::set{ fname2uid["g.h"], fname2uid["def7.h"]})}), From 36c81f66ba17ffb598b50b4438caf2c3c62deff6 Mon Sep 17 00:00:00 2001 From: Ti Liang Date: Fri, 1 Jul 2022 13:45:43 -0400 Subject: [PATCH 3/3] Fix docstring and comments Signed-off-by: Ti Liang --- .../include_graph_dependencies.h | 25 +++++++++++-------- src/include_graph_dependencies.cpp | 22 ++++++++-------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/include/clangmetatool/include_graph_dependencies.h b/include/clangmetatool/include_graph_dependencies.h index b9d2f56..42b2dba 100644 --- a/include/clangmetatool/include_graph_dependencies.h +++ b/include/clangmetatool/include_graph_dependencies.h @@ -30,7 +30,7 @@ struct IncludeGraphDependencies { */ static std::set collectAllIncludes(const clangmetatool::collectors::IncludeGraphData* data, - const clangmetatool::types::FileUID &fileFUID); + const clangmetatool::types::FileUID &fileUID); /** @@ -46,38 +46,41 @@ struct IncludeGraphDependencies { */ static std::set liveDependencies(const clangmetatool::collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &fileFUID); + const clangmetatool::types::FileUID &fileUID); /* * A data structure for include graph weak dependencies analysis * for a specific source file * - * Key: all headers the file directly depends on + * Key: all headers the file indirectly depends on * Value: a set of direct included headers that could allow the file to * access the key header * * Example: + * \code{.unparsed} * { "def1.h": {"a.h", "b.h"}, * "def2.h": {"a.h", "c.h"} } + * \endcode * * So that given the example data above it means current analyzing file: - * - depends on definitions from "def1.h", "def2.h" directly - * - includes "a.h", "b.h", "c.h" - * - can access "def1.h" by "a.h" and "b.h" - * - can access "def2.h" by "a.h" and "c.h" + * - depends on definitions from \c "def1.h", \c "def2.h" + * - includes \c "a.h", \c "b.h", \c "c.h" + * - can access \c "def1.h" by \c "a.h" and \c "b.h" + * - can access \c "def2.h" by \c "a.h" and \c "c.h" */ typedef std::map> DirectDependenciesMap; /** - * Get the 'live' weak dependencies of a header within the given include graph. - * This will return data structure(called DirectDependenciesMap) shows weak dependencies of headers + * Get the live weak dependencies of a header within the given include graph. * - * (See docstring of DirectDependenciesMap typedef above for more details of the data structure) + * Unlike \c "liveDependencies" which returns the first header that leads to a header + * with a needed declaration, the output of this function includes all direct includes + * that have a path to a header with a needed declaration. */ static DirectDependenciesMap liveWeakDependencies(const clangmetatool::collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &fileFUID); + const clangmetatool::types::FileUID &fileUID); }; // struct IncludeGraphDependencies } // namespace clangmetatool diff --git a/src/include_graph_dependencies.cpp b/src/include_graph_dependencies.cpp index 33add25..36fa87f 100644 --- a/src/include_graph_dependencies.cpp +++ b/src/include_graph_dependencies.cpp @@ -93,14 +93,14 @@ bool IncludeGraphDependencies::decrementUsageRefCount( std::set IncludeGraphDependencies::collectAllIncludes( const clangmetatool::collectors::IncludeGraphData* data, - const types::FileUID &fileFUID) + const types::FileUID &fileUID) { types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileUID); std::set visitedNodes; std::queue toVisit; - toVisit.push(fileFUID); + toVisit.push(fileUID); while (!toVisit.empty()) { auto currentFUID = toVisit.front(); toVisit.pop(); @@ -120,18 +120,18 @@ IncludeGraphDependencies::collectAllIncludes( std::set IncludeGraphDependencies::liveDependencies( const collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &fileFUID) { + const clangmetatool::types::FileUID &fileUID) { std::set dependencies; std::set visitedEdges; std::set visitedNodes; types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileUID); for (auto it = rangeBegin; it != rangeEnd; ++it) { - assert(it->first == fileFUID); + assert(it->first == fileUID); auto &dependency = it->second; - if (isRequired(data, fileFUID, dependency, visitedEdges, visitedNodes)) { + if (isRequired(data, fileUID, dependency, visitedEdges, visitedNodes)) { dependencies.insert(dependency); } } @@ -191,16 +191,16 @@ void traverseFor(const types::FileUID &forNode, const types::FileUID &rootNode, IncludeGraphDependencies::DirectDependenciesMap IncludeGraphDependencies::liveWeakDependencies( const clangmetatool::collectors::IncludeGraphData *data, - const clangmetatool::types::FileUID &fileFUID){ + const clangmetatool::types::FileUID &fileUID){ IncludeGraphDependencies::DirectDependenciesMap depsMap; types::FileGraph::const_iterator rangeBegin, rangeEnd; - std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileFUID); + std::tie(rangeBegin, rangeEnd) = edgeRangeStartsWith(data, fileUID); for (auto it = rangeBegin; it != rangeEnd; ++it) { - assert(it->first == fileFUID); + assert(it->first == fileUID); std::set knownNodes; - traverseFor(fileFUID, it->second, data, knownNodes, depsMap); + traverseFor(fileUID, it->second, data, knownNodes, depsMap); } return depsMap;