Rename RemoveIgnored -> RemoveMembers

Also reshuffle CodeGen's passes to fix an alignment bug with removed
members.

Change RemoveMembers to actually remove members instead of replacing
them with padding. AddPadding must be run afterwards to fill in the
gaps.

oi/CodeGen.cpp

@@ -31,7 +31,7 @@
 #include "type_graph/DrgnParser.h"
 #include "type_graph/Flattener.h"
 #include "type_graph/NameGen.h"
-#include "type_graph/RemoveIgnored.h"
+#include "type_graph/RemoveMembers.h"
 #include "type_graph/RemoveTopLevelPointer.h"
 #include "type_graph/TopoSorter.h"
 #include "type_graph/TypeGraph.h"
@@ -783,28 +783,34 @@ void CodeGen::addDrgnRoot(struct drgn_type* drgnType,
 void CodeGen::transform(type_graph::TypeGraph& typeGraph) {
   type_graph::PassManager pm;
 
-  // 1. Transformation passes
+  // Simplify the type graph first so there is less work for later passes
+  pm.addPass(type_graph::RemoveTopLevelPointer::createPass());
   pm.addPass(type_graph::Flattener::createPass());
   pm.addPass(type_graph::TypeIdentifier::createPass(config_.passThroughTypes));
+
   if (config_.features[Feature::PolymorphicInheritance]) {
+    // Parse new children nodes
     type_graph::DrgnParser drgnParser{
         typeGraph, containerInfos_,
         config_.features[Feature::ChaseRawPointers]};
     pm.addPass(type_graph::AddChildren::createPass(drgnParser, symbols_));
+
     // Re-run passes over newly added children
     pm.addPass(type_graph::Flattener::createPass());
     pm.addPass(
         type_graph::TypeIdentifier::createPass(config_.passThroughTypes));
   }
-  pm.addPass(type_graph::RemoveIgnored::createPass(config_.membersToStub));
-  pm.addPass(type_graph::RemoveTopLevelPointer::createPass());
 
-  // 2. Fixup passes to repair type graph after partial transformations
+  // Calculate alignment before removing members, as those members may have an
+  // influence on the class' overall alignment.
+  pm.addPass(type_graph::AlignmentCalc::createPass());
+  pm.addPass(type_graph::RemoveMembers::createPass(config_.membersToStub));
+
+  // Add padding to fill in the gaps of removed members and ensure their
+  // alignments
   pm.addPass(type_graph::AddPadding::createPass(config_.features));
 
-  // 3. Annotation passes
   pm.addPass(type_graph::NameGen::createPass());
-  pm.addPass(type_graph::AlignmentCalc::createPass());
   pm.addPass(type_graph::TopoSorter::createPass());
 
   pm.run(typeGraph);

oi/type_graph/CMakeLists.txt

@@ -7,7 +7,7 @@ add_library(type_graph
   NameGen.cpp
   PassManager.cpp
   Printer.cpp
-  RemoveIgnored.cpp
+  RemoveMembers.cpp
   RemoveTopLevelPointer.cpp
   TopoSorter.cpp
   TypeGraph.cpp

oi/type_graph/RemoveMembers.cpp

@@ -13,26 +13,26 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "RemoveIgnored.h"
+#include "RemoveMembers.h"
 
 #include "AddPadding.h"
 #include "TypeGraph.h"
 
 namespace type_graph {
 
-Pass RemoveIgnored::createPass(
+Pass RemoveMembers::createPass(
     const std::vector<std::pair<std::string, std::string>>& membersToIgnore) {
   auto fn = [&membersToIgnore](TypeGraph& typeGraph) {
-    RemoveIgnored removeIgnored{typeGraph, membersToIgnore};
+    RemoveMembers removeMembers{membersToIgnore};
     for (auto& type : typeGraph.rootTypes()) {
-      removeIgnored.accept(type);
+      removeMembers.accept(type);
     }
   };
 
-  return Pass("RemoveIgnored", fn);
+  return Pass("RemoveMembers", fn);
 }
 
-void RemoveIgnored::accept(Type& type) {
+void RemoveMembers::accept(Type& type) {
   if (visited_.count(&type) != 0)
     return;
 
@@ -40,7 +40,7 @@ void RemoveIgnored::accept(Type& type) {
   type.accept(*this);
 }
 
-void RemoveIgnored::visit(Class& c) {
+void RemoveMembers::visit(Class& c) {
   for (const auto& param : c.templateParams) {
     accept(param.type());
   }
@@ -54,19 +54,12 @@ void RemoveIgnored::visit(Class& c) {
     accept(child);
   }
 
-  for (size_t i = 0; i < c.members.size(); i++) {
-    if (!ignoreMember(c.name(), c.members[i].name)) {
-      continue;
-    }
-    auto& primitive = typeGraph_.makeType<Primitive>(Primitive::Kind::Int8);
-    auto& paddingArray =
-        typeGraph_.makeType<Array>(primitive, c.members[i].type().size());
-    c.members[i] =
-        Member{paddingArray, AddPadding::MemberPrefix, c.members[i].bitOffset};
-  }
+  std::erase_if(c.members, [this, &c](Member member) {
+    return ignoreMember(c.name(), member.name);
+  });
 }
 
-bool RemoveIgnored::ignoreMember(const std::string& typeName,
+bool RemoveMembers::ignoreMember(const std::string& typeName,
                                  const std::string& memberName) const {
   for (const auto& [ignoredType, ignoredMember] : membersToIgnore_) {
     if (typeName == ignoredType && memberName == ignoredMember) {

oi/type_graph/RemoveMembers.h

@@ -23,23 +23,20 @@
 
 namespace type_graph {
 
-class TypeGraph;
-
 /*
- * RemoveIgnored
+ * RemoveMembers
  *
- * Remove types and members as requested by the [[codegen.ignore]] section in
- * the OI config.
+ * Removes members as requested by the [[codegen.ignore]] section in the OI
+ * config.
  */
-class RemoveIgnored : public RecursiveVisitor {
+class RemoveMembers : public RecursiveVisitor {
  public:
   static Pass createPass(
       const std::vector<std::pair<std::string, std::string>>& membersToIgnore);
 
-  RemoveIgnored(
-      TypeGraph& typeGraph,
+  RemoveMembers(
       const std::vector<std::pair<std::string, std::string>>& membersToIgnore)
-      : typeGraph_(typeGraph), membersToIgnore_(membersToIgnore) {
+      : membersToIgnore_(membersToIgnore) {
   }
 
   using RecursiveVisitor::accept;
@@ -52,7 +49,6 @@ class RemoveIgnored : public RecursiveVisitor {
                     const std::string& memberName) const;
 
   std::unordered_set<Type*> visited_;
-  TypeGraph& typeGraph_;
   const std::vector<std::pair<std::string, std::string>>& membersToIgnore_;
 };
 

test/CMakeLists.txt

@@ -44,7 +44,7 @@ add_executable(test_type_graph
   test_flattener.cpp
   test_name_gen.cpp
   test_node_tracker.cpp
-  test_remove_ignored.cpp
+  test_remove_members.cpp
   test_remove_top_level_pointer.cpp
   test_topo_sorter.cpp
   test_type_identifier.cpp

test/test_codegen.cpp

@@ -5,6 +5,7 @@
 #include <string_view>
 #include <vector>
 
+#include "TypeGraphParser.h"
 #include "mocks.h"
 #include "oi/CodeGen.h"
 #include "oi/type_graph/Printer.h"
@@ -33,6 +34,33 @@ void testTransform(Type& type,
 
   check({type}, expectedAfter, "after transform");
 }
+
+void testTransform(OICodeGen::Config& config,
+                   std::string_view input,
+                   std::string_view expectedAfter) {
+  input.remove_prefix(1);  // Remove initial '\n'
+  TypeGraph typeGraph;
+  TypeGraphParser parser{typeGraph};
+  try {
+    parser.parse(input);
+  } catch (const TypeGraphParserError& err) {
+    FAIL() << "Error parsing input graph: " << err.what();
+  }
+
+  // Validate input formatting
+  check(typeGraph.rootTypes(), input, "parsing input graph");
+
+  MockSymbolService symbols;
+  CodeGen codegen{config, symbols};
+  codegen.transform(typeGraph);
+
+  check(typeGraph.rootTypes(), expectedAfter, "after transform");
+}
+
+void testTransform(std::string_view input, std::string_view expectedAfter) {
+  OICodeGen::Config config;
+  testTransform(config, input, expectedAfter);
+}
 }  // namespace
 
 TEST(CodeGenTest, TransformContainerAllocator) {
@@ -135,3 +163,47 @@ TEST(CodeGenTest, TransformContainerAllocatorParamInParent) {
               Primitive: int32_t
 )");
 }
+
+TEST(CodeGenTest, RemovedMemberAlignment) {
+  OICodeGen::Config config;
+  config.membersToStub = {{"MyClass", "b"}};
+  testTransform(config, R"(
+[0] Class: MyClass (size: 24)
+      Member: a (offset: 0)
+        Primitive: int8_t
+      Member: b (offset: 8)
+        Primitive: int64_t
+      Member: c (offset: 16)
+        Primitive: int8_t
+)",
+                R"(
+[0] Class: MyClass_0 (size: 24, align: 8)
+      Member: a_0 (offset: 0, align: 1)
+        Primitive: int8_t
+      Member: __oi_padding_1 (offset: 1)
+[1]     Array: (length: 15)
+          Primitive: int8_t
+      Member: c_2 (offset: 16, align: 1)
+        Primitive: int8_t
+      Member: __oi_padding_3 (offset: 17)
+[2]     Array: (length: 7)
+          Primitive: int8_t
+)");
+}
+
+TEST(CodeGenTest, UnionMembersAlignment) {
+  testTransform(R"(
+[0] Union: MyUnion (size: 8)
+      Member: a (offset: 0)
+        Primitive: int8_t
+      Member: b (offset: 8)
+        Primitive: int64_t
+)",
+                R"(
+[0] Union: MyUnion_0 (size: 8, align: 8)
+      Member: a_0 (offset: 0, align: 1)
+        Primitive: int8_t
+      Member: b_1 (offset: 8, align: 8)
+        Primitive: int64_t
+)");
+}

test/test_remove_members.cpp

@@ -1,12 +1,12 @@
 #include <gtest/gtest.h>
 
-#include "oi/type_graph/RemoveIgnored.h"
+#include "oi/type_graph/RemoveMembers.h"
 #include "oi/type_graph/Types.h"
 #include "test/type_graph_utils.h"
 
 using namespace type_graph;
 
-TEST(RemoveIgnoredTest, Match) {
+TEST(RemoveMembersTest, Match) {
   auto classB = Class{1, Class::Kind::Class, "ClassB", 4};
 
   auto classA = Class{0, Class::Kind::Class, "ClassA", 12};
@@ -18,7 +18,7 @@ TEST(RemoveIgnoredTest, Match) {
       {"ClassA", "b"},
   };
 
-  test(RemoveIgnored::createPass(membersToIgnore), {classA}, R"(
+  test(RemoveMembers::createPass(membersToIgnore), {classA}, R"(
 [0] Class: ClassA (size: 12)
       Member: a (offset: 0)
 [1]     Class: ClassB (size: 4)
@@ -31,15 +31,12 @@ TEST(RemoveIgnoredTest, Match) {
 [0] Class: ClassA (size: 12)
       Member: a (offset: 0)
 [1]     Class: ClassB (size: 4)
-      Member: __oi_padding (offset: 4)
-[2]     Array: (length: 4)
-          Primitive: int8_t
       Member: c (offset: 8)
         [1]
 )");
 }
 
-TEST(RemoveIgnoredTest, TypeMatchMemberMiss) {
+TEST(RemoveMembersTest, TypeMatchMemberMiss) {
   auto classB = Class{1, Class::Kind::Class, "ClassB", 4};
 
   auto classA = Class{0, Class::Kind::Class, "ClassA", 12};
@@ -51,7 +48,7 @@ TEST(RemoveIgnoredTest, TypeMatchMemberMiss) {
       {"ClassA", "x"},
   };
 
-  test(RemoveIgnored::createPass(membersToIgnore), {classA}, R"(
+  test(RemoveMembers::createPass(membersToIgnore), {classA}, R"(
 [0] Class: ClassA (size: 12)
       Member: a (offset: 0)
 [1]     Class: ClassB (size: 4)
@@ -62,7 +59,7 @@ TEST(RemoveIgnoredTest, TypeMatchMemberMiss) {
 )");
 }
 
-TEST(RemoveIgnoredTest, MemberMatchWrongType) {
+TEST(RemoveMembersTest, MemberMatchWrongType) {
   auto classB = Class{1, Class::Kind::Class, "ClassB", 4};
 
   auto classA = Class{0, Class::Kind::Class, "ClassA", 12};
@@ -74,7 +71,7 @@ TEST(RemoveIgnoredTest, MemberMatchWrongType) {
       {"ClassB", "b"},
   };
 
-  test(RemoveIgnored::createPass(membersToIgnore), {classA}, R"(
+  test(RemoveMembers::createPass(membersToIgnore), {classA}, R"(
 [0] Class: ClassA (size: 12)
       Member: a (offset: 0)
 [1]     Class: ClassB (size: 4)
@@ -85,11 +82,11 @@ TEST(RemoveIgnoredTest, MemberMatchWrongType) {
 )");
 }
 
-TEST(RemoveIgnoredTest, RecurseClassParam) {
+TEST(RemoveMembersTest, RecurseClassParam) {
   const std::vector<std::pair<std::string, std::string>>& membersToIgnore = {
       {"ClassA", "b"},
   };
-  test(RemoveIgnored::createPass(membersToIgnore), R"(
+  test(RemoveMembers::createPass(membersToIgnore), R"(
 [0] Class: MyClass (size: 0)
       Param
 [1]     Class: ClassA (size: 12)
@@ -106,19 +103,16 @@ TEST(RemoveIgnoredTest, RecurseClassParam) {
 [1]     Class: ClassA (size: 12)
           Member: a (offset: 0)
             Primitive: int32_t
-          Member: __oi_padding (offset: 4)
-[2]         Array: (length: 4)
-              Primitive: int8_t
           Member: c (offset: 8)
             Primitive: int32_t
 )");
 }
 
-TEST(RemoveIgnoredTest, RecurseClassParent) {
+TEST(RemoveMembersTest, RecurseClassParent) {
   const std::vector<std::pair<std::string, std::string>>& membersToIgnore = {
       {"ClassA", "b"},
   };
-  test(RemoveIgnored::createPass(membersToIgnore), R"(
+  test(RemoveMembers::createPass(membersToIgnore), R"(
 [0] Class: MyClass (size: 0)
       Parent (offset: 0)
 [1]     Class: ClassA (size: 12)
@@ -135,19 +129,16 @@ TEST(RemoveIgnoredTest, RecurseClassParent) {
 [1]     Class: ClassA (size: 12)
           Member: a (offset: 0)
             Primitive: int32_t
-          Member: __oi_padding (offset: 4)
-[2]         Array: (length: 4)
-              Primitive: int8_t
           Member: c (offset: 8)
             Primitive: int32_t
 )");
 }
 
-TEST(RemoveIgnoredTest, RecurseClassMember) {
+TEST(RemoveMembersTest, RecurseClassMember) {
   const std::vector<std::pair<std::string, std::string>>& membersToIgnore = {
       {"ClassA", "b"},
   };
-  test(RemoveIgnored::createPass(membersToIgnore), R"(
+  test(RemoveMembers::createPass(membersToIgnore), R"(
 [0] Class: MyClass (size: 0)
       Member: xxx (offset: 0)
 [1]     Class: ClassA (size: 12)
@@ -164,19 +155,16 @@ TEST(RemoveIgnoredTest, RecurseClassMember) {
 [1]     Class: ClassA (size: 12)
           Member: a (offset: 0)
             Primitive: int32_t
-          Member: __oi_padding (offset: 4)
-[2]         Array: (length: 4)
-              Primitive: int8_t
           Member: c (offset: 8)
             Primitive: int32_t
 )");
 }
 
-TEST(RemoveIgnoredTest, RecurseClassChild) {
+TEST(RemoveMembersTest, RecurseClassChild) {
   const std::vector<std::pair<std::string, std::string>>& membersToIgnore = {
       {"ClassA", "b"},
   };
-  test(RemoveIgnored::createPass(membersToIgnore), R"(
+  test(RemoveMembers::createPass(membersToIgnore), R"(
 [0] Class: MyClass (size: 0)
       Child
 [1]     Class: ClassA (size: 12)
@@ -193,9 +181,6 @@ TEST(RemoveIgnoredTest, RecurseClassChild) {
 [1]     Class: ClassA (size: 12)
           Member: a (offset: 0)
             Primitive: int32_t
-          Member: __oi_padding (offset: 4)
-[2]         Array: (length: 4)
-              Primitive: int8_t
           Member: c (offset: 8)
             Primitive: int32_t
 )");