tbv2: fix pointer codegen

A previous change enabled running OIL tests with specific features enabled. This highlighted that pointer code generation under TreeBuilder-v2 was very broken. This change updates pointer code generation to work and enables the skipped tests. All enabled tests need `expected_json_v2` added to them due to formatting differences. Reformatted and rewrote the basic type handler that handles primitives and pointers. Removed the reliance on `features` to decide whether to generate for TreeBuilder-v2 as the intermediate features have been removed. Pointers are treated as containers with a capacity of 1 and a length of 0 if null/a cycle and 1 if followed. This holds for void pointers where, although they aren't followed, the length is still set. There were a couple of other changes needed to enable these tests on TBv2 that aren't worth their own issues and PRs, I sneaked them in here. Extra changes: - Added `Pointer` and `Reference` to TopoSorter so they generate `NameProvider` instances. It might be worth visiting the graph differently for `NameProvider` as it requires so many instances that others generators do not. Will consider that in the future. - Follow typedefs when calculating exclusive size for a type. Closes #458. Test plan: - CI - Enabled previously disabled tests.
2024-11-09 21:24:14 +00:00 · 2024-01-16 19:10:16 +00:00 · 2024-01-16 19:10:16 +00:00 · 4d2eecf6fa
commit 4d2eecf6fa
parent 40af807d8b
9 changed files with 386 additions and 134 deletions
--- a/oi/CodeGen.cpp
+++ b/oi/CodeGen.cpp
@ -195,7 +195,12 @@ void genDecls(const TypeGraph& typeGraph, std::string& code) {
 namespace {

 size_t calculateExclusiveSize(const Type& t) {
-  if (const auto* c = dynamic_cast<const Class*>(&t)) {
+  const Type* finalType = &t;
+  while (const auto* td = dynamic_cast<const Typedef*>(finalType)) {
+    finalType = &td->underlyingType();
+  }
+
+  if (const auto* c = dynamic_cast<const Class*>(finalType)) {
    return std::accumulate(
        c->members.cbegin(), c->members.cend(), 0, [](size_t a, const auto& m) {
          if (m.name.starts_with(AddPadding::MemberPrefix))
@ -203,7 +208,7 @@ size_t calculateExclusiveSize(const Type& t) {
          return a;
        });
  }
-  return t.size();
+  return finalType->size();
 }

 }  // namespace
@ -211,7 +216,7 @@ size_t calculateExclusiveSize(const Type& t) {
 void genNames(const TypeGraph& typeGraph, std::string& code) {
  code += R"(
 template <typename T>
-struct NameProvider {};
+struct NameProvider;
 )";

  // TODO: stop types being duplicated at this point and remove this check
@ -239,6 +244,9 @@ struct ExclusiveSizeProvider {
 )";

  for (const Type& t : typeGraph.finalTypes) {
+    if (dynamic_cast<const Typedef*>(&t))
+      continue;
+
    size_t exclusiveSize = calculateExclusiveSize(t);
    if (exclusiveSize != t.size()) {
      code += "template <> struct ExclusiveSizeProvider<";
@ -1120,23 +1128,6 @@ void addStandardTypeHandlers(TypeGraph& typeGraph,
    }
 )";

-  if (features[Feature::TreeBuilderV2]) {
-    code += R"(
-template <typename Ctx, typename T>
-constexpr inst::Field make_field(std::string_view name) {
-  return inst::Field{
-    sizeof(T),
-    ExclusiveSizeProvider<T>::size,
-    name,
-    NameProvider<T>::names,
-    TypeHandler<Ctx, T>::fields,
-    TypeHandler<Ctx, T>::processors,
-    std::is_fundamental_v<T>
-  };
-}
-)";
-  }
-
  // TODO: bit of a hack - making ContainerInfo a node in the type graph and
  // traversing for it would remove the need for this set altogether.
  std::unordered_set<const ContainerInfo*> used{};
@ -1318,9 +1309,6 @@ void CodeGen::generate(TypeGraph& typeGraph,
    code += "using namespace oi::detail;\n";
    code += "using oi::exporters::ParsedData;\n";
    code += "using namespace oi::exporters;\n";
-    code += "namespace OIInternal {\nnamespace {\n";
-    FuncGen::DefineBasicTypeHandlers(code, config_.features);
-    code += "} // namespace\n} // namespace OIInternal\n";
  }

  if (config_.features[Feature::CaptureThriftIsset]) {
@ -1360,6 +1348,7 @@ void CodeGen::generate(TypeGraph& typeGraph,
  }

  if (config_.features[Feature::TreeBuilderV2]) {
+    FuncGen::DefineBasicTypeHandlers(code);
    addStandardTypeHandlers(typeGraph, config_.features, code);
    addTypeHandlers(typeGraph, code);
  } else {
--- a/oi/FuncGen.cpp
+++ b/oi/FuncGen.cpp
@ -614,81 +614,102 @@ class BackInserter {
 * pointer's value always, then the value of the pointer if it is unique. void
 * is of type Unit and always stores nothing.
 */
-void FuncGen::DefineBasicTypeHandlers(std::string& code, FeatureSet features) {
+void FuncGen::DefineBasicTypeHandlers(std::string& code) {
  code += R"(
-    template <typename Ctx, typename T>
-    struct TypeHandler {
+template <typename Ctx, typename T>
+struct TypeHandler;
+)";
+
+  code += R"(
+template <typename Ctx, typename T>
+constexpr inst::Field make_field(std::string_view name) {
+  return inst::Field{
+      sizeof(T),
+      ExclusiveSizeProvider<T>::size,
+      name,
+      NameProvider<T>::names,
+      TypeHandler<Ctx, T>::fields,
+      TypeHandler<Ctx, T>::processors,
+      std::is_fundamental_v<T>,
+  };
+}
+)";
+  code += R"(
+template <typename Ctx, typename T>
+struct TypeHandler {
  using DB = typename Ctx::DataBuffer;
+
 private:
+  static void process_pointer(result::Element& el,
+                              std::function<void(inst::Inst)> stack_ins,
+                              ParsedData d) {
+    el.pointer = std::get<ParsedData::VarInt>(d.val).value;
+  }
+  static void process_pointer_content(result::Element& el,
+                                      std::function<void(inst::Inst)> stack_ins,
+                                      ParsedData d) {
+    using U = std::decay_t<std::remove_pointer_t<T>>;
+    const ParsedData::Sum& sum = std::get<ParsedData::Sum>(d.val);
+
+    el.container_stats.emplace(result::Element::ContainerStats{ .capacity = 1, .length = 0 });
+    if (sum.index == 0)
+      return;
+    el.container_stats->length = 1;
+
+    if constexpr (oi_is_complete<U>) {
+      static constexpr auto childField = make_field<Ctx, U>("*");
+      stack_ins(childField);
+    }
+  }
+
  static auto choose_type() {
-            if constexpr(std::is_pointer_v<T>) {
-                return std::type_identity<types::st::Pair<DB,
+    if constexpr (std::is_pointer_v<T>) {
+      return std::type_identity<types::st::Pair<
+          DB,
          types::st::VarInt<DB>,
-                  types::st::Sum<DB, types::st::Unit<DB>, typename TypeHandler<Ctx, std::remove_pointer_t<T>>::type>
-                >>();
+          types::st::Sum<
+              DB,
+              types::st::Unit<DB>,
+              typename TypeHandler<Ctx, std::remove_pointer_t<T>>::type>>>();
    } else {
      return std::type_identity<types::st::Unit<DB>>();
    }
  }
-
-      public:
-        using type = typename decltype(choose_type())::type;
-)";
-  if (features[Feature::TreeBuilderV2]) {
-    code += R"(private:
-        static void process_pointer(result::Element& el, std::function<void(inst::Inst)> stack_ins, ParsedData d) {
-          el.pointer = std::get<ParsedData::VarInt>(d.val).value;
-        }
-        static void process_pointer_content(result::Element& el, std::function<void(inst::Inst)> stack_ins, ParsedData d) {
-          static constexpr std::array<std::string_view, 1> names{"TODO"};
-          static constexpr auto childField = make_field<Ctx, T>("*");
-
-          const ParsedData::Sum& sum = std::get<ParsedData::Sum>(d.val);
-
-          el.container_stats.emplace(result::Element::ContainerStats{ .capacity = 1 });
-
-          if (sum.index == 0)
-            return;
-
-          el.container_stats->length = 1;
-          stack_ins(childField);
-        }
-
-        static constexpr auto choose_fields() {
-          if constexpr(std::is_pointer_v<T>) {
-            return std::array<exporters::inst::Field, 0>{};
-          } else {
-            return std::array<exporters::inst::Field, 0>{};
-          }
-        }
  static constexpr auto choose_processors() {
-          if constexpr(std::is_pointer_v<T>) {
+    if constexpr (std::is_pointer_v<T>) {
      return std::array<inst::ProcessorInst, 2>{
-              {types::st::VarInt<DB>::describe, &process_pointer},
-              {types::st::Sum<DB, types::st::Unit<DB>, typename TypeHandler<Ctx, std::remove_pointer_t<T>>::type>::describe, &process_pointer_content},
+          exporters::inst::ProcessorInst{types::st::VarInt<DB>::describe,
+                                         &process_pointer},
+          exporters::inst::ProcessorInst{
+              types::st::Sum<
+                  DB,
+                  types::st::Unit<DB>,
+                  typename TypeHandler<Ctx, std::remove_pointer_t<T>>::type>::
+                  describe,
+              &process_pointer_content},
      };
    } else {
      return std::array<inst::ProcessorInst, 0>{};
    }
  }
+
 public:
-        static constexpr auto fields = choose_fields();
+  using type = typename decltype(choose_type())::type;
+
+  static constexpr std::array<exporters::inst::Field, 0> fields{};
  static constexpr auto processors = choose_processors();
-)";
-  }
-  code += R"(
+
  static types::st::Unit<DB> getSizeType(
-          Ctx& ctx,
-          const T& t,
-          typename TypeHandler<Ctx, T>::type returnArg) {
-              if constexpr(std::is_pointer_v<T>) {
+      Ctx& ctx, const T& t, typename TypeHandler<Ctx, T>::type returnArg) {
+    if constexpr (std::is_pointer_v<T>) {
      JLOG("ptr val @");
      JLOGPTR(t);
      auto r0 = returnArg.write((uintptr_t)t);
      if (t && ctx.pointers.add((uintptr_t)t)) {
-                  return r0.template delegate<1>([&t](auto ret) {
-                    if constexpr (!std::is_void<std::remove_pointer_t<T>>::value) {
-                      return TypeHandler<Ctx, std::remove_pointer_t<T>>::getSizeType(*t, ret);
+        return r0.template delegate<1>([&ctx, &t](auto ret) {
+          using U = std::decay_t<std::remove_pointer_t<T>>;
+          if constexpr (oi_is_complete<U>) {
+            return TypeHandler<Ctx, U>::getSizeType(ctx, *t, ret);
          } else {
            return ret;
          }
@ -700,24 +721,20 @@ void FuncGen::DefineBasicTypeHandlers(std::string& code, FeatureSet features) {
      return returnArg;
    }
  }
-    };
-  )";
+};
+)";

  code += R"(
-    template <typename Ctx>
-    class TypeHandler<Ctx, void> {
+template <typename Ctx>
+class TypeHandler<Ctx, void> {
  using DB = typename Ctx::DataBuffer;
+
 public:
  using type = types::st::Unit<DB>;
+  static constexpr std::array<exporters::inst::Field, 0> fields{};
+  static constexpr std::array<exporters::inst::ProcessorInst, 0> processors{};
+};
 )";
-  if (features[Feature::TreeBuilderV2]) {
-    code +=
-        "static constexpr std::array<exporters::inst::Field, 0> fields{};\n";
-    code +=
-        "static constexpr std::array<exporters::inst::ProcessorInst, 0> "
-        "processors{};\n";
-  }
-  code += "};\n";
 }

 ContainerInfo FuncGen::GetOiArrayContainerInfo() {
--- a/oi/FuncGen.h
+++ b/oi/FuncGen.h
@ -76,7 +76,7 @@ class FuncGen {

  static void DefineDataSegmentDataBuffer(std::string& testCode);
  static void DefineBackInserterDataBuffer(std::string& code);
-  static void DefineBasicTypeHandlers(std::string& code, FeatureSet features);
+  static void DefineBasicTypeHandlers(std::string& code);

  static ContainerInfo GetOiArrayContainerInfo();
 };
--- a/oi/type_graph/TopoSorter.cpp
+++ b/oi/type_graph/TopoSorter.cpp
@ -126,12 +126,27 @@ void TopoSorter::visit(Pointer& p) {
    // Typedefs can not be forward declared, so we must sort them before
    // pointers which reference them
    accept(p.pointeeType());
-    return;
-  }
-
+  } else {
    // Pointers do not create a dependency, but we do still care about the types
    // they point to, so delay them until the end.
    acceptAfter(p.pointeeType());
+  }
+
+  sortedTypes_.push_back(p);
+}
+
+void TopoSorter::visit(Reference& r) {
+  if (dynamic_cast<Typedef*>(&r.pointeeType())) {
+    // Typedefs can not be forward declared, so we must sort them before
+    // pointers which reference them
+    accept(r.pointeeType());
+  } else {
+    // Pointers do not create a dependency, but we do still care about the types
+    // they point to, so delay them until the end.
+    acceptAfter(r.pointeeType());
+  }
+
+  sortedTypes_.push_back(r);
 }

 void TopoSorter::visit(CaptureKeys& c) {
--- a/oi/type_graph/TopoSorter.h
+++ b/oi/type_graph/TopoSorter.h
@ -46,6 +46,7 @@ class TopoSorter : public RecursiveVisitor {
  void visit(Enum& e) override;
  void visit(Typedef& td) override;
  void visit(Pointer& p) override;
+  void visit(Reference& r) override;
  void visit(Primitive& p) override;
  void visit(CaptureKeys& p) override;
  void visit(Incomplete& i) override;
--- a/test/integration/anonymous.toml
+++ b/test/integration/anonymous.toml
@ -97,7 +97,6 @@ definitions = '''
      ]}]'''

  [cases.anon_struct]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const AnonStructContainer&"]
    setup = '''
      return AnonStructContainer{
@ -133,6 +132,40 @@ definitions = '''
        }]
      }]
    }]'''
+    expect_json_v2 = '''[{
+      "name":"a0",
+      "typeNames":["ns_anonymous::AnonStructContainer"],
+      "staticSize":8,
+      "exclusiveSize":0,
+      "size":20,
+      "members":[{
+        "name":"anon",
+        "typeNames":["__oi_anon_1"],
+        "staticSize":8,
+        "exclusiveSize":0,
+        "size":20,
+        "members":[{
+          "name":"node",
+          "typeNames":["ns_anonymous::Node*"],
+          "staticSize":8,
+          "exclusiveSize":8,
+          "size":20,
+          "length":1,
+          "capacity":1,
+          "members":[{
+            "name":"*",
+            "typeNames":["ns_anonymous::Node"],
+            "staticSize":12,
+            "exclusiveSize":0,
+            "size":12,
+            "members":[
+              { "name": "a", "staticSize": 4, "exclusiveSize": 4, "size": 4 },
+              { "name": "b", "staticSize": 4, "exclusiveSize": 4, "size": 4 },
+              { "name": "c", "staticSize": 4, "exclusiveSize": 4, "size": 4 }
+            ]
+        }]
+      }]}
+    ]}]'''

  [cases.anon_struct_ptr]
    skip = "We don't support pointer to anon-structs yet" # https://github.com/facebookexperimental/object-introspection/issues/20
@ -146,7 +179,6 @@ definitions = '''
    features = ["chase-raw-pointers"]

  [cases.anon_typedef]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const AnonTypedefContainer&"]
    setup = '''
      return AnonTypedefContainer{
@ -192,6 +224,34 @@ definitions = '''
        }]
      }]
    }]'''
+    expect_json_v2 = '''[{
+      "staticSize": 8,
+      "exclusiveSize": 0,
+      "size": 20,
+      "members": [{
+        "typeNames":["AnonStruct", "__oi_anon_2"],
+        "staticSize": 8,
+        "exclusiveSize": 0,
+        "size": 20,
+        "members": [{
+          "typeNames": ["ns_anonymous::Node*"],
+          "staticSize": 8,
+          "exclusiveSize": 8,
+          "size": 20,
+          "members": [{
+            "typeNames": ["ns_anonymous::Node"],
+            "staticSize": 12,
+            "exclusiveSize": 0,
+            "size": 12,
+            "members": [
+              { "name": "a", "staticSize": 4, "exclusiveSize": 4, "size": 4 },
+              { "name": "b", "staticSize": 4, "exclusiveSize": 4, "size": 4 },
+              { "name": "c", "staticSize": 4, "exclusiveSize": 4, "size": 4 }
+            ]
+          }]
+        }]
+      }]
+    }]'''

  [cases.anon_union]
    param_types = ["const AnonUnionContainer&"]
@ -217,7 +277,6 @@ definitions = '''
    }]'''

  [cases.nested_anon_struct]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const NestedAnonContainer&"]
    features = ["chase-raw-pointers"]
    setup = 'return NestedAnonContainer{.m = { .v = {.as = {new Node{1, 2, 3}}}}};'
@ -272,6 +331,64 @@ definitions = '''
        "dynamicSize": 0
      }]
    }]'''
+    expect_json_v2 = '''[{
+      "staticSize":80,
+      "exclusiveSize":0,
+      "size":92,
+      "members":[
+        {
+          "name":"m",
+          "typeNames": ["__oi_anon_1"],
+          "staticSize": 48,
+          "exclusiveSize":0,
+          "size": 60,
+          "members":[
+            {"name":"__oi_anon_0", "typeNames":["__oi_anon_2"], "staticSize":16, "exclusiveSize":16, "size":16},
+            {
+              "name":"v",
+              "typeNames":["__oi_anon_3"],
+              "staticSize":32,
+              "exclusiveSize":4,
+              "size":44,
+              "members":[
+                {"name":"a", "typeNames":["int32_t"], "staticSize":4, "exclusiveSize":4, "size":4},
+                {"name":"b", "typeNames":["int32_t"], "staticSize":4, "exclusiveSize":4, "size":4},
+                {"name":"c", "typeNames":["int32_t"], "staticSize":4, "exclusiveSize":4, "size":4},
+                {"name":"__oi_anon_4", "typeNames":["__oi_anon_4"], "staticSize":8, "exclusiveSize":8, "size":8},
+                {
+                  "name":"as",
+                  "typeNames":["AnonStruct", "__oi_anon_6"],
+                  "staticSize":8,
+                  "exclusiveSize":0,
+                  "size":20,
+                  "members":[{
+                    "name":"node",
+                    "typeNames":["ns_anonymous::Node*"],
+                    "staticSize":8,
+                    "exclusiveSize":8,
+                    "size":20,
+                    "members":[{
+                      "name":"*",
+                      "typeNames":["ns_anonymous::Node"],
+                      "staticSize":12,
+                      "exclusiveSize":0,
+                      "size":12,
+                      "members":[
+                        { "name": "a", "staticSize": 4, "exclusiveSize": 4, "size": 4 },
+                        { "name": "b", "staticSize": 4, "exclusiveSize": 4, "size": 4 },
+                        { "name": "c", "staticSize": 4, "exclusiveSize": 4, "size": 4 }
+                      ]
+                    }]
+                  }]
+                }
+              ]
+            }
+          ]
+        },
+        {"name":"__oi_anon_1", "typeNames": ["__oi_anon_8"], "staticSize":24, "exclusiveSize":24, "size":24},
+        {"name":"__oi_anon_2", "typeNames": ["__oi_anon_9"], "staticSize":8, "exclusiveSize":8, "size":8}
+      ]
+    }]'''

  # This test is disabled due to GCC not supporting it
  # [cases.anon_array]
--- a/test/integration/pointers.toml
+++ b/test/integration/pointers.toml
@ -134,7 +134,6 @@ definitions = '''


  [cases.struct_primitive_ptrs]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const PrimitivePtrs&"]
    setup = "return PrimitivePtrs{0, new int(0), new int(0)};"
    features = ["chase-raw-pointers"]
@ -147,6 +146,16 @@ definitions = '''
        {"name":"b", "staticSize":8, "exclusiveSize":8, "dynamicSize":4},
        {"name":"c", "staticSize":8, "exclusiveSize":8, "dynamicSize":0}
      ]}]'''
+    expect_json_v2 = '''[{
+      "staticSize":24,
+      "exclusiveSize":4,
+      "size":28,
+      "members":[
+        {"name":"a", "typeNames":["int32_t"], "staticSize":4, "exclusiveSize":4, "size":4},
+        {"name":"b", "typeNames":["int32_t*"], "staticSize":8, "exclusiveSize":8, "size":12, "capacity":1, "length":1},
+        {"name":"c", "typeNames":["void*"], "staticSize":8, "exclusiveSize":8, "size":8, "capacity":1, "length":1}
+      ]
+    }]'''
  [cases.struct_primitive_ptrs_no_follow]
    param_types = ["const PrimitivePtrs&"]
    setup = "return PrimitivePtrs{0, new int(0), new int(0)};"
@ -161,7 +170,6 @@ definitions = '''
        {"name":"c", "staticSize":8, "dynamicSize":0, "exclusiveSize":8, "size":8}
      ]}]'''
  [cases.struct_primitive_ptrs_null]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const PrimitivePtrs&"]
    setup = "return PrimitivePtrs{0, nullptr, nullptr};"
    features = ["chase-raw-pointers"]
@ -175,10 +183,19 @@ definitions = '''
        {"name":"b", "staticSize":8, "dynamicSize":0, "exclusiveSize":8, "size":8},
        {"name":"c", "staticSize":8, "dynamicSize":0, "exclusiveSize":8, "size":8}
      ]}]'''
+    expect_json_v2 = '''[{
+      "staticSize":24,
+      "exclusiveSize":4,
+      "size":24,
+      "members":[
+        {"name":"a", "typeNames":["int32_t"], "staticSize":4, "exclusiveSize":4, "size":4},
+        {"name":"b", "typeNames":["int32_t*"], "staticSize":8, "exclusiveSize":8, "size":8, "capacity":1, "length":0},
+        {"name":"c", "typeNames":["void*"], "staticSize":8, "exclusiveSize":8, "size":8, "capacity":1, "length":0}
+      ]
+    }]'''


  [cases.struct_vector_ptr]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const VectorPtr&"]
    setup = "return VectorPtr{new std::vector<int>{1,2,3}};"
    features = ["chase-raw-pointers"]
@ -188,6 +205,22 @@ definitions = '''
      "members":[
        {"name":"vec", "staticSize":8, "dynamicSize":36}
      ]}]'''
+    expect_json_v2 = '''[{
+      "staticSize":8,
+      "exclusiveSize":0,
+      "size":44,
+      "members": [
+        {
+          "typeNames":["std::vector<int32_t, std::allocator<int32_t>>*"],
+          "staticSize":8,
+          "exclusiveSize":8,
+          "size":44,
+          "length":1,
+          "capacity":1,
+          "members":[{ "staticSize":24, "exclusiveSize":24, "size":36 }]
+        }
+      ]
+    }]'''
  [cases.struct_vector_ptr_no_follow]
    param_types = ["const VectorPtr&"]
    setup = "return VectorPtr{new std::vector<int>{1,2,3}};"
@ -200,7 +233,6 @@ definitions = '''
        {"name":"vec", "staticSize":8, "dynamicSize":0, "exclusiveSize":8, "size":8}
      ]}]'''
  [cases.struct_vector_ptr_null]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const VectorPtr&"]
    setup = "return VectorPtr{nullptr};"
    features = ["chase-raw-pointers"]
@ -212,10 +244,25 @@ definitions = '''
      "members":[
        {"name":"vec", "staticSize":8, "dynamicSize":0, "exclusiveSize":8, "size":8}
      ]}]'''
+    expect_json_v2 = '''[{
+      "staticSize":8,
+      "exclusiveSize":0,
+      "size":8,
+      "members": [
+        {
+          "typeNames":["std::vector<int32_t, std::allocator<int32_t>>*"],
+          "staticSize":8,
+          "exclusiveSize":8,
+          "size":8,
+          "length":0,
+          "capacity":1,
+          "members":[]
+        }
+      ]
+    }]'''


  [cases.vector_of_pointers]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const std::vector<int*>&"]
    setup = "return {{new int(1), nullptr, new int(3)}};"
    features = ["chase-raw-pointers"]
@ -230,6 +277,18 @@ definitions = '''
        {"staticSize":8, "dynamicSize":0, "pointer":0},
        {"staticSize":8, "dynamicSize":4, "NOT": {"pointer":0}}
      ]}]'''
+    expect_json_v2 = '''[{
+      "staticSize":24,
+      "exclusiveSize":24,
+      "size":56,
+      "length":3,
+      "capacity":3,
+      "members":[
+        {"staticSize":8, "exclusiveSize":8, "size":12, "NOT": {"pointer":0}},
+        {"staticSize":8, "exclusiveSize":8, "size":8, "pointer":0},
+        {"staticSize":8, "exclusiveSize":8, "size":12, "NOT": {"pointer":0}}
+      ]
+    }]'''
  [cases.vector_of_pointers_no_follow]
    skip = "pointer field is missing from results" # https://github.com/facebookexperimental/object-introspection/issues/21
    param_types = ["const std::vector<int*>&"]
@ -260,7 +319,6 @@ definitions = '''
        {"name":"c", "staticSize":8, "dynamicSize":0, "exclusiveSize":8, "size":8}
      ]}]'''
  [cases.feature_config]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const std::vector<int*>&"]
    setup = "return {{new int(1), nullptr, new int(3)}};"
    config_prefix = 'features = ["chase-raw-pointers"]'
@ -275,3 +333,15 @@ definitions = '''
        {"staticSize":8, "dynamicSize":0, "pointer":0},
        {"staticSize":8, "dynamicSize":4, "NOT": {"pointer":0}}
      ]}]'''
+    expect_json_v2 = '''[{
+      "staticSize":24,
+      "exclusiveSize":24,
+      "size":56,
+      "length":3,
+      "capacity":3,
+      "members":[
+        {"staticSize":8, "exclusiveSize":8, "size":12, "NOT": {"pointer":0}},
+        {"staticSize":8, "exclusiveSize":8, "size":8, "pointer":0},
+        {"staticSize":8, "exclusiveSize":8, "size":12, "NOT": {"pointer":0}}
+      ]
+    }]'''
--- a/test/integration/pointers_incomplete.toml
+++ b/test/integration/pointers_incomplete.toml
@ -86,7 +86,6 @@ definitions = '''
    expect_json_v2 = '[{"staticSize":16, "exclusiveSize":16}]'

  [cases.containing_struct]
-    oil_skip = "pointers are broken in tbv2" # https://github.com/facebookexperimental/object-introspection/issues/458
    param_types = ["const IncompleteTypeContainer&"]
    setup = "return IncompleteTypeContainer{};"
    features = ["chase-raw-pointers"]
@ -115,6 +114,32 @@ definitions = '''
        }
      ]
    }]'''
+    expect_json_v2 = '''[{
+      "staticSize": 88,
+      "exclusiveSize": 21,
+      "size": 88,
+      "members": [
+        { "name": "ptrundef", "staticSize": 8, "exclusiveSize": 8, "size": 8 },
+        { "name": "__makePad1", "staticSize": 1, "exclusiveSize": 1, "size": 1 },
+        { "name": "shundef", "staticSize": 16, "exclusiveSize": 16, "size": 16 },
+        { "name": "__makePad2", "staticSize": 1, "exclusiveSize": 1, "size": 1 },
+        { "name": "shoptundef",
+          "staticSize": 24,
+          "exclusiveSize": 24,
+          "size": 24,
+          "length": 0,
+          "capacity": 1
+        },
+        { "name": "__makePad3", "staticSize": 1, "exclusiveSize": 1, "size": 1 },
+        { "name": "optundef",
+          "staticSize": 16,
+          "exclusiveSize": 16,
+          "size": 16,
+          "length": 0,
+          "capacity": 1
+        }
+      ]
+    }]'''

  [cases.containing_struct_no_follow]
    param_types = ["const IncompleteTypeContainer&"]
--- a/test/test_topo_sorter.cpp
+++ b/test/test_topo_sorter.cpp
@ -236,6 +236,22 @@ TEST(TopoSorterTest, Pointers) {
  myclass.members.push_back(Member{mypointer, "ptr", 0});

  test({myclass}, R"(
+ClassA*
+MyClass
+ClassA
+)");
+}
+
+TEST(TopoSorterTest, References) {
+  // References do not require pointee types to be defined first
+  auto classA = Class{0, Class::Kind::Class, "ClassA", 69};
+  auto myreference = Reference{1, classA};
+
+  auto myclass = Class{2, Class::Kind::Class, "MyClass", 69};
+  myclass.members.push_back(Member{myreference, "ref", 0});
+
+  test({myclass}, R"(
+ClassA*
 MyClass
 ClassA
 )");
@ -258,6 +274,7 @@ TEST(TopoSorterTest, PointerCycle) {
  // the same sorted order for ClassA and ClassB.
  for (const auto& input : inputs) {
    test(input, R"(
+ClassA*
 ClassB
 ClassA
 )");
@ -276,6 +293,7 @@ TEST(TopoSorterTest, PointerToTypedef) {
  test({myclass}, R"(
 ClassA
 aliasA
+aliasA*
 MyClass
 )");
 }