From fa9e6e8a5cbf836303bd6b108051181d77b74275 Mon Sep 17 00:00:00 2001 From: Christian Blichmann Date: Tue, 8 Mar 2022 07:16:17 -0800 Subject: [PATCH] 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 --- .../tools/clang_generator/emitter.cc | 40 +++++++++++++------ .../tools/clang_generator/emitter_test.cc | 40 +++++++++++++++++++ .../frontend_action_test_util.cc | 8 ++++ .../frontend_action_test_util.h | 6 +++ sandboxed_api/tools/clang_generator/types.cc | 9 ++++- 5 files changed, 90 insertions(+), 13 deletions(-) diff --git a/sandboxed_api/tools/clang_generator/emitter.cc b/sandboxed_api/tools/clang_generator/emitter.cc index 79484a2..b01e20f 100644 --- a/sandboxed_api/tools/clang_generator/emitter.cc +++ b/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(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(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(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) { diff --git a/sandboxed_api/tools/clang_generator/emitter_test.cc b/sandboxed_api/tools/clang_generator/emitter_test.cc index 47ef627..42739c7 100644 --- a/sandboxed_api/tools/clang_generator/emitter_test.cc +++ b/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(emitter, GeneratorOptions())); + + // Types from "std" should be skipped + EXPECT_THAT(emitter.rendered_types_["std"], IsEmpty()); + + std::vector 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 = diff --git a/sandboxed_api/tools/clang_generator/frontend_action_test_util.cc b/sandboxed_api/tools/clang_generator/frontend_action_test_util.cc index 490ed65..2a62f95 100644 --- a/sandboxed_api/tools/clang_generator/frontend_action_test_util.cc +++ b/sandboxed_api/tools/clang_generator/frontend_action_test_util.cc @@ -17,6 +17,8 @@ #include #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 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 diff --git a/sandboxed_api/tools/clang_generator/frontend_action_test_util.h b/sandboxed_api/tools/clang_generator/frontend_action_test_util.h index b0730ef..a958fbc 100644 --- a/sandboxed_api/tools/clang_generator/frontend_action_test_util.h +++ b/sandboxed_api/tools/clang_generator/frontend_action_test_util.h @@ -78,6 +78,12 @@ class FrontendActionTest : public ::testing::Test { absl::flat_hash_map 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_ diff --git a/sandboxed_api/tools/clang_generator/types.cc b/sandboxed_api/tools/clang_generator/types.cc index a8878fe..a05f9f8 100644 --- a/sandboxed_api/tools/clang_generator/types.cc +++ b/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()) { - 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; }