clang_generator: Correctly emit typedefs with anonymous enums/structs

This change also adds some more basic testing and test utils.

PiperOrigin-RevId: 433203779
Change-Id: I57616af3719ccbc41201dc6d4b0b60ddaf70ebab

sandboxed_api/tools/clang_generator/emitter.cc

@@ -27,6 +27,7 @@
 #include "absl/strings/strip.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Type.h"
 #include "clang/Format/Format.h"
 #include "sandboxed_api/tools/clang_generator/diagnostics.h"
@@ -213,23 +214,38 @@ std::string PrintRecordTemplateArguments(const clang::CXXRecordDecl* record) {
 }
 
 // Serializes the given Clang AST declaration back into compilable source code.
-std::string PrintAstDecl(const clang::Decl* decl) {
-  // TODO(cblichmann): Make types nicer
-  //   - Rewrite typedef to using
-  //   - Rewrite function pointers using std::add_pointer_t<>;
-
-  if (const auto* record = llvm::dyn_cast<clang::CXXRecordDecl>(decl)) {
-    // For C++ classes/structs, only emit a forward declaration.
-    return absl::StrCat(PrintRecordTemplateArguments(record),
-                        record->isClass() ? "class " : "struct ",
-                        std::string(record->getName()));
-  }
+std::string PrintDecl(const clang::Decl* decl) {
   std::string pretty;
   llvm::raw_string_ostream os(pretty);
   decl->print(os);
   return os.str();
 }
 
+// Returns the spelling for a given declaration will be emitted to the final
+// header. This may rewrite declarations (like converting typedefs to using,
+// etc.).
+std::string GetSpelling(const clang::Decl* decl) {
+  // TODO(cblichmann): Make types nicer
+  //   - Rewrite typedef to using
+  //   - Rewrite function pointers using std::add_pointer_t<>;
+
+  if (const auto* typedef_decl = llvm::dyn_cast<clang::TypedefNameDecl>(decl)) {
+    // Special case: anonymous enum/struct
+    if (auto* tag_decl = typedef_decl->getAnonDeclWithTypedefName()) {
+      return absl::StrCat("typedef ", PrintDecl(tag_decl), " ",
+                          ToStringView(typedef_decl->getName()));
+    }
+  }
+
+  if (const auto* record_decl = llvm::dyn_cast<clang::CXXRecordDecl>(decl)) {
+    // For C++ classes/structs, only emit a forward declaration.
+    return absl::StrCat(PrintRecordTemplateArguments(record_decl),
+                        record_decl->isClass() ? "class " : "struct ",
+                        ToStringView(record_decl->getName()));
+  }
+  return PrintDecl(decl);
+}
+
 std::string GetParamName(const clang::ParmVarDecl* decl, int index) {
   if (std::string name = decl->getName().str(); !name.empty()) {
     return absl::StrCat(name, "_");  // Suffix to avoid collisions
@@ -412,7 +428,7 @@ void Emitter::CollectType(clang::QualType qual) {
                            absl::StrJoin(ns_path, "::"));
   }
 
-  rendered_types_[ns_name].push_back(PrintAstDecl(decl));
+  rendered_types_[ns_name].push_back(GetSpelling(decl));
 }
 
 void Emitter::CollectFunction(clang::FunctionDecl* decl) {

sandboxed_api/tools/clang_generator/emitter_test.cc

@@ -29,6 +29,8 @@
 namespace sapi {
 namespace {
 
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
 using ::testing::MatchesRegex;
 using ::testing::SizeIs;
 using ::testing::StrEq;
@@ -37,6 +39,7 @@ using ::testing::StrNe;
 class EmitterForTesting : public Emitter {
  public:
   using Emitter::functions_;
+  using Emitter::rendered_types_;
 };
 
 class EmitterTest : public FrontendActionTest {};
@@ -57,6 +60,43 @@ TEST_F(EmitterTest, BasicFunctionality) {
   EXPECT_THAT(header, IsOk());
 }
 
+TEST_F(EmitterTest, RelatedTypes) {
+  EmitterForTesting emitter;
+  RunFrontendAction(
+      R"(
+    namespace std {
+    using size_t = unsigned long;
+    }  // namespace std
+    using std::size_t;
+    typedef enum { kRed, kGreen, kBlue } Color;
+    struct Channel {
+      Color color;
+      size_t width;
+      size_t height;
+    };
+    struct ByValue {
+      int value;
+    };
+    extern "C" void Colorize(Channel* chan, ByValue v) {}
+
+    typedef struct { int member; } MyStruct;
+    extern "C" void Structize(MyStruct* s);
+  )",
+      absl::make_unique<GeneratorAction>(emitter, GeneratorOptions()));
+
+  // Types from "std" should be skipped
+  EXPECT_THAT(emitter.rendered_types_["std"], IsEmpty());
+
+  std::vector<std::string> ugly_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", "struct ByValue",
+                          "typedef struct { int member; } MyStruct"));
+}
+
 TEST(IncludeGuard, CreatesRandomizedGuardForEmptyFilename) {
   // Copybara will transform the string. This is intentional.
   constexpr absl::string_view kGeneratedHeaderPrefix =

sandboxed_api/tools/clang_generator/frontend_action_test_util.cc

@@ -17,6 +17,8 @@
 #include <vector>
 
 #include "absl/status/status.h"
+#include "absl/strings/ascii.h"
+#include "absl/strings/str_replace.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Frontend/FrontendAction.h"
@@ -67,4 +69,10 @@ std::vector<std::string> FrontendActionTest::GetCommandLineFlagsForTesting(
           "-I.",  "-Wno-error",    std::string(input_file)};
 }
 
+std::string Uglify(absl::string_view code) {
+  std::string result = absl::StrReplaceAll(code, {{"\n", " "}});
+  absl::RemoveExtraAsciiWhitespace(&result);
+  return result;
+}
+
 }  // namespace sapi

sandboxed_api/tools/clang_generator/frontend_action_test_util.h

@@ -78,6 +78,12 @@ class FrontendActionTest : public ::testing::Test {
   absl::flat_hash_map<std::string, std::string> file_contents_;
 };
 
+// Flattens a piece of C++ code into one line and removes consecutive runs of
+// whitespace. This makes it easier to compare code snippets for testing.
+// Note: This is not syntax-aware and will replace characters within strings as
+// well.
+std::string Uglify(absl::string_view code);
+
 }  // namespace sapi
 
 #endif  // SANDBOXED_API_TOOLS_CLANG_GENERATOR_FRONTEND_ACTION_TEST_UTIL_H_

sandboxed_api/tools/clang_generator/types.cc

@@ -15,6 +15,8 @@
 #include "sandboxed_api/tools/clang_generator/types.h"
 
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_format.h"
+#include "clang/AST/Type.h"
 
 namespace sapi {
 namespace {
@@ -37,7 +39,12 @@ void TypeCollector::CollectRelatedTypes(clang::QualType qual) {
   seen_.insert(qual);
 
   if (const auto* typedef_type = qual->getAs<clang::TypedefType>()) {
-    CollectRelatedTypes(typedef_type->getDecl()->getUnderlyingType());
+    auto* typedef_decl = typedef_type->getDecl();
+    if (!typedef_decl->getAnonDeclWithTypedefName()) {
+      // Do not collect anonymous enums/structs as those are handled when
+      // emitting them via their parent typedef/using declaration.
+      CollectRelatedTypes(typedef_decl->getUnderlyingType());
+    }
     collected_.insert(qual);
     return;
   }