TypeGraph: Remove NodeTracker from the TypeGraph class

The TypeGraph class should only be responsible for storing Type nodes.
Traversing the graph and tracking which nodes have been visited should
not be included there.

Passes now take a NodeTrackerHolder as an input parameter, which
provides access to a zeroed-out NodeTracker.

oi/type_graph/AddChildren.cpp

@@ -32,7 +32,7 @@ using ref = std::reference_wrapper<T>;
 namespace oi::detail::type_graph {
 
 Pass AddChildren::createPass(DrgnParser& drgnParser, SymbolService& symbols) {
-  auto fn = [&drgnParser, &symbols](TypeGraph& typeGraph) {
+  auto fn = [&drgnParser, &symbols](TypeGraph& typeGraph, NodeTracker&) {
     AddChildren pass(typeGraph, drgnParser);
     pass.enumerateChildClasses(symbols);
     for (auto& type : typeGraph.rootTypes()) {

oi/type_graph/AddPadding.cpp

@@ -25,7 +25,7 @@ using ref = std::reference_wrapper<T>;
 namespace oi::detail::type_graph {
 
 Pass AddPadding::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker&) {
     AddPadding pass(typeGraph);
     for (auto& type : typeGraph.rootTypes()) {
       pass.accept(type);

oi/type_graph/AlignmentCalc.cpp

@@ -25,7 +25,7 @@ using ref = std::reference_wrapper<T>;
 namespace oi::detail::type_graph {
 
 Pass AlignmentCalc::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker&) {
     AlignmentCalc alignmentCalc;
     alignmentCalc.calculateAlignments(typeGraph.rootTypes());
   };

oi/type_graph/EnforceCompatibility.cpp

@@ -27,8 +27,8 @@
 namespace oi::detail::type_graph {
 
 Pass EnforceCompatibility::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker& tracker) {
-    EnforceCompatibility pass{typeGraph.resetTracker()};
+    EnforceCompatibility pass{tracker};
     for (auto& type : typeGraph.rootTypes()) {
       pass.accept(type);
     }

oi/type_graph/Flattener.cpp

@@ -21,8 +21,8 @@
 namespace oi::detail::type_graph {
 
 Pass Flattener::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker& tracker) {
-    Flattener flattener{typeGraph.resetTracker()};
+    Flattener flattener{tracker};
     for (auto& type : typeGraph.rootTypes()) {
       flattener.accept(type);
     }

oi/type_graph/NameGen.cpp

@@ -23,7 +23,7 @@ using ref = std::reference_wrapper<T>;
 namespace oi::detail::type_graph {
 
 Pass NameGen::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker&) {
     NameGen nameGen;
     nameGen.generateNames(typeGraph.rootTypes());
   };

oi/type_graph/NodeTracker.h

@@ -59,6 +59,12 @@ class NodeTracker {
     std::fill(visited_.begin(), visited_.end(), false);
   }
 
+  /*
+   * resize
+   *
+   * Resizes the underlying vector to the requested size, with the same
+   * semantics as std::vector::resize().
+   */
   void resize(size_t size) {
     visited_.resize(size);
   }
@@ -67,4 +73,39 @@ class NodeTracker {
   std::vector<bool> visited_;
 };
 
+/*
+ * NodeTrackerHolder
+ *
+ * Wrapper which ensures that the contained NodeTracker has been reset before
+ * allowing access to it.
+ */
+class NodeTrackerHolder {
+ public:
+  /*
+   * Implicit ctor from NodeTracker
+   */
+  NodeTrackerHolder(NodeTracker& tracker) : tracker_(tracker) {
+  }
+
+  /*
+   * get
+   *
+   * Returns a reference to a NodeTracker which has been reset, i.e. one in
+   * which all nodes are marked "not visited".
+   */
+  NodeTracker& get() {
+    tracker_.reset();
+    return tracker_;
+  }
+
+  NodeTracker& get(size_t size) {
+    tracker_.reset();
+    tracker_.resize(size);
+    return tracker_;
+  }
+
+ private:
+  NodeTracker& tracker_;
+};
+
 }  // namespace oi::detail::type_graph

oi/type_graph/PassManager.cpp

@@ -20,6 +20,7 @@
 #include <iostream>
 #include <sstream>
 
+#include "NodeTracker.h"
 #include "Printer.h"
 #include "TypeGraph.h"
 
@@ -28,8 +29,8 @@ using ref = std::reference_wrapper<T>;
 
 namespace oi::detail::type_graph {
 
-void Pass::run(TypeGraph& typeGraph) {
+void Pass::run(TypeGraph& typeGraph, NodeTrackerHolder tracker) {
-  fn_(typeGraph);
+  fn_(typeGraph, tracker.get(typeGraph.size()));
 }
 
 void PassManager::addPass(Pass p) {
@@ -37,11 +38,11 @@ void PassManager::addPass(Pass p) {
 }
 
 namespace {
-void print(const TypeGraph& typeGraph) {
+void print(const TypeGraph& typeGraph, NodeTrackerHolder tracker) {
   if (!VLOG_IS_ON(1))
     return;
   std::stringstream out;
-  Printer printer{out, typeGraph.resetTracker(), typeGraph.size()};
+  Printer printer{out, tracker.get(typeGraph.size()), typeGraph.size()};
   for (const auto& type : typeGraph.rootTypes()) {
     printer.print(type);
   }
@@ -54,19 +55,21 @@ void print(const TypeGraph& typeGraph) {
 const std::string separator = "----------------";
 
 void PassManager::run(TypeGraph& typeGraph) {
+  NodeTracker tracker;
+
   VLOG(1) << separator;
   VLOG(1) << "Parsed Type Graph:";
   VLOG(1) << separator;
-  print(typeGraph);
+  print(typeGraph, tracker);
   VLOG(1) << separator;
 
   for (size_t i = 0; i < passes_.size(); i++) {
     auto& pass = passes_[i];
     LOG(INFO) << "Running pass (" << i + 1 << "/" << passes_.size()
               << "): " << pass.name();
-    pass.run(typeGraph);
+    pass.run(typeGraph, tracker);
     VLOG(1) << separator;
-    print(typeGraph);
+    print(typeGraph, tracker);
     VLOG(1) << separator;
   }
 }

oi/type_graph/PassManager.h

@@ -21,6 +21,8 @@
 
 namespace oi::detail::type_graph {
 
+class NodeTrackerHolder;
+class NodeTracker;
 class TypeGraph;
 class Type;
 
@@ -30,12 +32,13 @@ class Type;
  * TODO
  */
 class Pass {
-  using PassFn = std::function<void(TypeGraph& typeGraph)>;
+  using PassFn =
+      std::function<void(TypeGraph& typeGraph, NodeTracker& tracker)>;
 
  public:
   Pass(std::string name, PassFn fn) : name_(std::move(name)), fn_(fn) {
   }
-  void run(TypeGraph& typeGraph);
+  void run(TypeGraph& typeGraph, NodeTrackerHolder tracker);
   std::string& name() {
     return name_;
   };

oi/type_graph/Prune.cpp

@@ -21,8 +21,8 @@
 namespace oi::detail::type_graph {
 
 Pass Prune::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker& tracker) {
-    Prune pass{typeGraph.resetTracker()};
+    Prune pass{tracker};
     for (auto& type : typeGraph.rootTypes()) {
       pass.accept(type);
     }

oi/type_graph/RemoveMembers.cpp

@@ -22,7 +22,7 @@ namespace oi::detail::type_graph {
 
 Pass RemoveMembers::createPass(
     const std::vector<std::pair<std::string, std::string>>& membersToIgnore) {
-  auto fn = [&membersToIgnore](TypeGraph& typeGraph) {
+  auto fn = [&membersToIgnore](TypeGraph& typeGraph, NodeTracker&) {
     RemoveMembers removeMembers{membersToIgnore};
     for (auto& type : typeGraph.rootTypes()) {
       removeMembers.accept(type);

oi/type_graph/RemoveTopLevelPointer.cpp

@@ -20,7 +20,7 @@
 namespace oi::detail::type_graph {
 
 Pass RemoveTopLevelPointer::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker&) {
     RemoveTopLevelPointer pass;
     pass.removeTopLevelPointers(typeGraph.rootTypes());
   };

oi/type_graph/TopoSorter.cpp

@@ -23,7 +23,7 @@ using ref = std::reference_wrapper<T>;
 namespace oi::detail::type_graph {
 
 Pass TopoSorter::createPass() {
-  auto fn = [](TypeGraph& typeGraph) {
+  auto fn = [](TypeGraph& typeGraph, NodeTracker&) {
     TopoSorter sorter;
     sorter.sort(typeGraph.rootTypes());
     typeGraph.finalTypes = std::move(sorter.sortedTypes());

oi/type_graph/TypeGraph.cpp

@@ -17,12 +17,6 @@
 
 namespace oi::detail::type_graph {
 
-NodeTracker& TypeGraph::resetTracker() const noexcept {
-  tracker_.reset();
-  tracker_.resize(size());
-  return tracker_;
-}
-
 template <>
 Primitive& TypeGraph::makeType<Primitive>(Primitive::Kind kind) {
   switch (kind) {

oi/type_graph/TypeGraph.h

@@ -19,7 +19,6 @@
 #include <memory>
 #include <vector>
 
-#include "NodeTracker.h"
 #include "Types.h"
 
 namespace oi::detail::type_graph {
@@ -48,8 +47,6 @@ class TypeGraph {
     rootTypes_.push_back(type);
   }
 
-  NodeTracker& resetTracker() const noexcept;
-
   // Override of the generic makeType function that returns singleton Primitive
   // objects
   template <typename T>
@@ -88,7 +85,6 @@ class TypeGraph {
   std::vector<std::reference_wrapper<Type>> rootTypes_;
   // Store all type objects in vectors for ownership. Order is not significant.
   std::vector<std::unique_ptr<Type>> types_;
-  mutable NodeTracker tracker_;
   NodeId next_id_ = 0;
 };
 

oi/type_graph/TypeIdentifier.cpp

@@ -22,9 +22,8 @@ namespace oi::detail::type_graph {
 
 Pass TypeIdentifier::createPass(
     const std::vector<ContainerInfo>& passThroughTypes) {
-  auto fn = [&passThroughTypes](TypeGraph& typeGraph) {
+  auto fn = [&passThroughTypes](TypeGraph& typeGraph, NodeTracker& tracker) {
-    TypeIdentifier typeId{typeGraph.resetTracker(), typeGraph,
+    TypeIdentifier typeId{tracker, typeGraph, passThroughTypes};
-                          passThroughTypes};
     for (auto& type : typeGraph.rootTypes()) {
       typeId.accept(type);
     }

test/test_add_children.cpp

@@ -2,6 +2,7 @@
 
 #include "oi/SymbolService.h"
 #include "oi/type_graph/AddChildren.h"
+#include "oi/type_graph/NodeTracker.h"
 #include "oi/type_graph/Printer.h"
 #include "oi/type_graph/TypeGraph.h"
 #include "test_drgn_parser.h"
@@ -22,12 +23,13 @@ std::string AddChildrenTest::run(std::string_view function,
 
   Type& type = drgnParser.parse(drgnRoot);
   typeGraph.addRoot(type);
+  NodeTracker tracker;
 
   auto pass = AddChildren::createPass(drgnParser, *symbols_);
-  pass.run(typeGraph);
+  pass.run(typeGraph, tracker);
 
   std::stringstream out;
-  Printer printer{out, typeGraph.resetTracker(), typeGraph.size()};
+  Printer printer{out, tracker, typeGraph.size()};
   printer.print(type);
 
   return out.str();

test/test_drgn_parser.cpp

@@ -6,6 +6,7 @@
 // TODO needed?:
 #include "oi/ContainerInfo.h"
 #include "oi/OIParser.h"
+#include "oi/type_graph/NodeTracker.h"
 #include "oi/type_graph/Printer.h"
 #include "oi/type_graph/TypeGraph.h"
 #include "oi/type_graph/Types.h"
@@ -55,7 +56,8 @@ std::string DrgnParserTest::run(std::string_view function,
   Type& type = drgnParser.parse(drgnRoot);
 
   std::stringstream out;
-  Printer printer{out, typeGraph.resetTracker(), typeGraph.size()};
+  NodeTracker tracker;
+  Printer printer{out, tracker, typeGraph.size()};
   printer.print(type);
 
   return out.str();

test/test_node_tracker.cpp

@@ -82,3 +82,33 @@ TEST(NodeTrackerTest, LargeIds) {
   EXPECT_TRUE(tracker.visit(myclass1));
   EXPECT_TRUE(tracker.visit(myclass2));
 }
+
+TEST(NodeTrackerTest, NodeTrackerHolder) {
+  Class myclass{0, Class::Kind::Class, "myclass", 0};
+  Array myarray{1, myclass, 3};
+
+  NodeTracker baseTracker_doNotUse;
+  NodeTrackerHolder holder{baseTracker_doNotUse};
+
+  {
+    auto& tracker = holder.get();
+    // First visit
+    EXPECT_FALSE(tracker.visit(myarray));
+    EXPECT_FALSE(tracker.visit(myclass));
+
+    // Second visit
+    EXPECT_TRUE(tracker.visit(myarray));
+    EXPECT_TRUE(tracker.visit(myclass));
+  }
+
+  {
+    auto& tracker = holder.get();
+    // First visit, fresh tracker
+    EXPECT_FALSE(tracker.visit(myarray));
+    EXPECT_FALSE(tracker.visit(myclass));
+
+    // Second visit, fresh tracker
+    EXPECT_TRUE(tracker.visit(myarray));
+    EXPECT_TRUE(tracker.visit(myclass));
+  }
+}

test/type_graph_utils.cpp

@@ -3,6 +3,7 @@
 #include <gtest/gtest.h>
 
 #include "oi/ContainerInfo.h"
+#include "oi/type_graph/NodeTracker.h"
 #include "oi/type_graph/PassManager.h"
 #include "oi/type_graph/Printer.h"
 #include "oi/type_graph/TypeGraph.h"
@@ -21,7 +22,8 @@ void check(const TypeGraph& typeGraph,
            std::string_view expected,
            std::string_view comment) {
   std::stringstream out;
-  type_graph::Printer printer(out, typeGraph.resetTracker(), typeGraph.size());
+  NodeTracker tracker;
+  type_graph::Printer printer(out, tracker, typeGraph.size());
 
   for (const auto& type : typeGraph.rootTypes()) {
     printer.print(type);
@@ -47,7 +49,8 @@ void test(type_graph::Pass pass,
   // Validate input formatting
   check(typeGraph, input, "parsing input graph");
 
-  pass.run(typeGraph);
+  NodeTracker tracker;
+  pass.run(typeGraph, tracker);
 
   check(typeGraph, expectedAfter, "after running pass");
 }