clang_generator: Do not collect structs/unions if declared in another record

The enclosing type is enough to reconstruct the AST when writing the header and this
change avoids emitting the same struct twice.

PiperOrigin-RevId: 435300029
Change-Id: I34bd660db5ba5c68b64cce73ecf2f026727ac57b

sandboxed_api/tools/clang_generator/emitter.cc

@@ -399,8 +399,8 @@ void Emitter::CollectType(clang::QualType qual) {
     decl = typedef_type->getDecl();
   } else if (const auto* enum_type = qual->getAs<clang::EnumType>()) {
     decl = enum_type->getDecl();
-  } else {
+  } else if (const auto* record_type = qual->getAs<clang::RecordType>()) {
-    decl = qual->getAsRecordDecl();
+    decl = record_type->getDecl();
   }
   if (!decl) {
     return;

sandboxed_api/tools/clang_generator/emitter_test.cc

@@ -74,9 +74,7 @@ TEST_F(EmitterTest, RelatedTypes) {
       size_t width;
       size_t height;
     };
-    struct ByValue {
+    struct ByValue { int value; };
-      int value;
-    };
     extern "C" void Colorize(Channel* chan, ByValue v) {}
 
     typedef struct { int member; } MyStruct;
@@ -87,11 +85,7 @@ TEST_F(EmitterTest, RelatedTypes) {
   // Types from "std" should be skipped
   EXPECT_THAT(emitter.rendered_types_["std"], IsEmpty());
 
-  std::vector<std::string> ugly_types;
+  EXPECT_THAT(UglifyAll(emitter.rendered_types_[""]),
-  for (const auto& type : emitter.rendered_types_[""]) {
-    ugly_types.push_back(Uglify(type));
-  }
-  EXPECT_THAT(ugly_types,
               ElementsAre("typedef enum { kRed, kGreen, kBlue } Color",
                           "struct Channel {"
                           " Color color;"
@@ -106,9 +100,7 @@ TEST_F(EmitterTest, NestedStruct) {
   RunFrontendAction(
       R"(
     struct A {
-      struct B {
+      struct B { int number; };
-         int number;
-      };
       B b;
       int data;
     };
@@ -116,17 +108,11 @@ TEST_F(EmitterTest, NestedStruct) {
         )",
       absl::make_unique<GeneratorAction>(emitter, GeneratorOptions()));
 
-  std::vector<std::string> ugly_types;
+  EXPECT_THAT(UglifyAll(emitter.rendered_types_[""]),
-  for (const auto& type : emitter.rendered_types_[""]) {
+              ElementsAre("struct A {"
-    ugly_types.push_back(Uglify(type));
+                          " struct B { int number; };"
-  }
-  EXPECT_THAT(ugly_types, ElementsAre("struct A {"
-                                      " struct B {"
-                                      " int number;"
-                                      " };"
                           " A::B b;"
-                                      " int data; "
+                          " int data; }"));
-                                      "}"));
 }
 
 TEST_F(EmitterTest, NestedAnonymousStruct) {
@@ -134,25 +120,50 @@ TEST_F(EmitterTest, NestedAnonymousStruct) {
   RunFrontendAction(
       R"(
     struct A {
-      struct {
+      struct { int number; } b;
-         int number;
-      } b;
       int data;
     };
     extern "C" void Structize(A* s);
         )",
       absl::make_unique<GeneratorAction>(emitter, GeneratorOptions()));
 
-  std::vector<std::string> ugly_types;
+  EXPECT_THAT(UglifyAll(emitter.rendered_types_[""]),
-  for (const auto& type : emitter.rendered_types_[""]) {
+              ElementsAre("struct A {"
-    ugly_types.push_back(Uglify(type));
+                          " struct { int number; } b;"
+                          " int data; }"));
 }
-  EXPECT_THAT(ugly_types, ElementsAre("struct A {"
+
-                                      " struct {"
+TEST_F(EmitterTest, ParentNotCollected) {
-                                      " int number;"
+  EmitterForTesting emitter;
-                                      " } b;"
+  RunFrontendAction(
-                                      " int data; "
+      R"(
-                                      "}"));
+    struct A {
+      struct B { int number; };
+      B b;
+      int data;
+    };
+    extern "C" void Structize(A::B* s);
+        )",
+      absl::make_unique<GeneratorAction>(emitter, GeneratorOptions()));
+
+  EXPECT_THAT(UglifyAll(emitter.rendered_types_[""]),
+              ElementsAre("struct A {"
+                          " struct B { int number; };"
+                          " A::B b;"
+                          " int data; }"));
+}
+
+TEST_F(EmitterTest, RemoveQualifiers) {
+  EmitterForTesting emitter;
+  RunFrontendAction(
+      R"(
+    struct A { int data; };
+    extern "C" void Structize(const A* in, A* out);
+        )",
+      absl::make_unique<GeneratorAction>(emitter, GeneratorOptions()));
+
+  EXPECT_THAT(UglifyAll(emitter.rendered_types_[""]),
+              ElementsAre("struct A { int data; }"));
 }
 
 TEST(IncludeGuard, CreatesRandomizedGuardForEmptyFilename) {

sandboxed_api/tools/clang_generator/frontend_action_test_util.cc

@@ -14,6 +14,8 @@
 
 #include "sandboxed_api/tools/clang_generator/frontend_action_test_util.h"
 
+#include <algorithm>
+#include <iterator>
 #include <vector>
 
 #include "absl/status/status.h"
@@ -75,4 +77,12 @@ std::string Uglify(absl::string_view code) {
   return result;
 }
 
+std::vector<std::string> UglifyAll(const std::vector<std::string>& snippets) {
+  std::vector<std::string> result;
+  result.reserve(snippets.size());
+  std::transform(snippets.cbegin(), snippets.cend(), std::back_inserter(result),
+                 Uglify);
+  return result;
+}
+
 }  // namespace sapi

sandboxed_api/tools/clang_generator/frontend_action_test_util.h

@@ -84,6 +84,8 @@ class FrontendActionTest : public ::testing::Test {
 // well.
 std::string Uglify(absl::string_view code);
 
+std::vector<std::string> UglifyAll(const std::vector<std::string>& snippets);
+
 }  // namespace sapi
 
 #endif  // SANDBOXED_API_TOOLS_CLANG_GENERATOR_FRONTEND_ACTION_TEST_UTIL_H_

sandboxed_api/tools/clang_generator/types.cc

@@ -33,10 +33,9 @@ bool IsFunctionReferenceType(clang::QualType qual) {
 }  // namespace
 
 void TypeCollector::CollectRelatedTypes(clang::QualType qual) {
-  if (seen_.count(qual) > 0) {  // contains() is LLVM_VERSION_MAJOR >= 11
+  if (!seen_.insert(qual)) {
     return;
   }
-  seen_.insert(qual);
 
   if (const auto* typedef_type = qual->getAs<clang::TypedefType>()) {
     auto* typedef_decl = typedef_type->getDecl();
@@ -95,7 +94,12 @@ void TypeCollector::CollectRelatedTypes(clang::QualType qual) {
     for (const clang::FieldDecl* field : decl->fields()) {
       CollectRelatedTypes(field->getType());
     }
-    collected_.insert(qual);
+    // Do not collect structs/unions if they are declared within another
+    // record. The enclosing type is enough to reconstruct the AST when
+    // writing the header.
+    const clang::RecordDecl* outer = decl->getOuterLexicalRecordContext();
+    decl = outer ? outer : decl;
+    collected_.insert(clang::QualType(decl->getTypeForDecl(), 0));
     return;
   }
 }