diff --git a/sandboxed_api/tools/clang_generator/emitter.cc b/sandboxed_api/tools/clang_generator/emitter.cc index 9db09d4..7d2351d 100644 --- a/sandboxed_api/tools/clang_generator/emitter.cc +++ b/sandboxed_api/tools/clang_generator/emitter.cc @@ -25,6 +25,7 @@ #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" #include "absl/strings/strip.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Type.h" @@ -433,16 +434,8 @@ absl::StatusOr EmitHeader( return out; } -void Emitter::EmitType(clang::QualType qual) { - clang::TypeDecl* decl = nullptr; - if (const auto* typedef_type = qual->getAs()) { - decl = typedef_type->getDecl(); - } else if (const auto* enum_type = qual->getAs()) { - decl = enum_type->getDecl(); - } else if (const auto* record_type = qual->getAs()) { - decl = record_type->getDecl(); - } - if (!decl) { +void Emitter::EmitType(clang::TypeDecl* type_decl) { + if (!type_decl) { return; } @@ -450,18 +443,17 @@ void Emitter::EmitType(clang::QualType qual) { // TODO(cblichmann): Instead of this and the hard-coded entities below, we // should map add the correct (system) headers to the // generated output. - if (decl->getASTContext().getSourceManager().isInSystemHeader( - decl->getBeginLoc())) { + if (type_decl->getASTContext().getSourceManager().isInSystemHeader( + type_decl->getBeginLoc())) { return; } - const std::vector ns_path = GetNamespacePath(decl); + const std::vector ns_path = GetNamespacePath(type_decl); std::string ns_name; if (!ns_path.empty()) { const auto& ns_root = ns_path.front(); - // Filter out any and all declarations from the C++ standard library, - // from SAPI itself and from other well-known namespaces. This avoids - // re-declaring things like standard integer types, for example. + // Filter out declarations from the C++ standard library, from SAPI itself + // and from other well-known namespaces. if (ns_root == "std" || ns_root == "__gnu_cxx" || ns_root == "sapi") { return; } @@ -472,7 +464,7 @@ void Emitter::EmitType(clang::QualType qual) { } // Skip types from Abseil that will already be included in the generated // header. - if (auto name = ToStringView(decl->getName()); + if (auto name = ToStringView(type_decl->getName()); name == "StatusCode" || name == "StatusToStringMode" || name == "CordMemoryAccounting" || name == "string_view" || name == "LogSeverity" || name == "LogEntry" || name == "Span" || @@ -483,16 +475,17 @@ void Emitter::EmitType(clang::QualType qual) { ns_name = absl::StrJoin(ns_path, "::"); } - std::string spelling = GetSpelling(decl); + std::string spelling = GetSpelling(type_decl); if (const auto& [it, inserted] = rendered_types_.emplace(ns_name, spelling); inserted) { rendered_types_ordered_.push_back(&*it); } } -void Emitter::AddTypesFiltered(const QualTypeSet& types) { - for (clang::QualType qual : types) { - EmitType(qual); +void Emitter::AddTypeDeclarations( + const std::vector& type_decls) { + for (clang::TypeDecl* type_decl : type_decls) { + EmitType(type_decl); } } diff --git a/sandboxed_api/tools/clang_generator/emitter.h b/sandboxed_api/tools/clang_generator/emitter.h index bacd65f..62556d5 100644 --- a/sandboxed_api/tools/clang_generator/emitter.h +++ b/sandboxed_api/tools/clang_generator/emitter.h @@ -60,13 +60,13 @@ class RenderedType { // Sandboxed API header. class Emitter { public: - // Adds the set of previously collected types to the emitter, recording the - // spelling of each one. Types that are not supported by the current - // generator settings or that are unwanted/unnecessary are skipped. Filtered - // types include C++ constructs or well-known standard library elements. The - // latter can be replaced by including the correct headers in the emitted - // header. - void AddTypesFiltered(const QualTypeSet& types); + // Adds the declarations of previously collected types to the emitter, + // recording the spelling of each one. Types/declarations that are not + // supported by the current generator settings or that are unwanted or + // unnecessary are skipped. Other filtered types include C++ constructs or + // well-known standard library elements. The latter can be replaced by + // including the correct headers in the emitted header. + void AddTypeDeclarations(const std::vector& type_decls); absl::Status AddFunction(clang::FunctionDecl* decl); @@ -74,7 +74,7 @@ class Emitter { absl::StatusOr EmitHeader(const GeneratorOptions& options); private: - void EmitType(clang::QualType qual); + void EmitType(clang::TypeDecl* type_decl); protected: // Stores namespaces and a list of spellings for types. Keeps track of types diff --git a/sandboxed_api/tools/clang_generator/emitter_test.cc b/sandboxed_api/tools/clang_generator/emitter_test.cc index 6d15913..9f66faf 100644 --- a/sandboxed_api/tools/clang_generator/emitter_test.cc +++ b/sandboxed_api/tools/clang_generator/emitter_test.cc @@ -65,7 +65,6 @@ TEST_F(EmitterTest, BasicFunctionality) { RunFrontendAction(R"(extern "C" void ExposedFunction() {})", std::make_unique(emitter, options)), IsOk()); - EXPECT_THAT(emitter.GetRenderedFunctions(), SizeIs(1)); absl::StatusOr header = emitter.EmitHeader(options); @@ -92,6 +91,7 @@ TEST_F(EmitterTest, RelatedTypes) { extern "C" void Structize(MyStruct* s);)", std::make_unique(emitter, GeneratorOptions())), IsOk()); + EXPECT_THAT(emitter.GetRenderedFunctions(), SizeIs(2)); // Types from "std" should be skipped EXPECT_THAT(emitter.SpellingsForNS("std"), IsEmpty()); @@ -105,6 +105,25 @@ TEST_F(EmitterTest, RelatedTypes) { "typedef struct { int member; } MyStruct")); } +TEST_F(EmitterTest, TypedefNames) { + EmitterForTesting emitter; + ASSERT_THAT( + RunFrontendAction( + R"(typedef enum { kNone, kSome } E; + struct A { E member; }; + typedef struct { int member; } B; + typedef struct tagC { int member; } C; + extern "C" void Colorize(A*, B*, C*);)", + std::make_unique(emitter, GeneratorOptions())), + IsOk()); + + EXPECT_THAT( + UglifyAll(emitter.SpellingsForNS("")), + ElementsAre("typedef enum { kNone, kSome } E", "struct A { E member; }", + "typedef struct { int member; } B", + "struct tagC { int member; }", "typedef struct tagC C")); +} + TEST_F(EmitterTest, NestedStruct) { EmitterForTesting emitter; ASSERT_THAT( @@ -209,6 +228,34 @@ TEST_F(EmitterTest, TypedefStructByValueSkipsFunction) { EXPECT_THAT(emitter.GetRenderedFunctions(), IsEmpty()); } +TEST_F(EmitterTest, TypedefTypeDependencies) { + EmitterForTesting emitter; + EXPECT_THAT( + RunFrontendAction( + R"(typedef bool some_other_unused; + using size_t = long long int; + typedef struct _Image Image; + typedef size_t (*StreamHandler)(const Image*, const void*, + const size_t); + enum unrelated_unused { NONE, SOME }; + struct _Image { + StreamHandler stream; + int size; + }; + extern "C" void Process(StreamHandler handler);)", + std::make_unique(emitter, GeneratorOptions())), + IsOk()); + EXPECT_THAT(emitter.GetRenderedFunctions(), SizeIs(1)); + + EXPECT_THAT(UglifyAll(emitter.SpellingsForNS("")), + ElementsAre("using size_t = long long", "struct _Image", + "typedef size_t (*StreamHandler)(const Image *, " + "const void *, const size_t)", + "struct _Image {" + " StreamHandler stream;" + " int size; }")); +} + TEST(IncludeGuard, CreatesRandomizedGuardForEmptyFilename) { // Copybara will transform the string. This is intentional. constexpr absl::string_view kGeneratedHeaderPrefix = diff --git a/sandboxed_api/tools/clang_generator/generator.cc b/sandboxed_api/tools/clang_generator/generator.cc index 8095653..de0052a 100644 --- a/sandboxed_api/tools/clang_generator/generator.cc +++ b/sandboxed_api/tools/clang_generator/generator.cc @@ -18,7 +18,9 @@ #include #include "absl/status/status.h" +#include "clang/AST/Decl.h" #include "clang/AST/Type.h" +#include "clang/Basic/SourceManager.h" #include "clang/Lex/PreprocessorOptions.h" #include "sandboxed_api/tools/clang_generator/diagnostics.h" #include "sandboxed_api/tools/clang_generator/emitter.h" @@ -43,6 +45,11 @@ std::string GetOutputFilename(absl::string_view source_file) { return ReplaceFileExtension(source_file, ".sapi.h"); } +bool GeneratorASTVisitor::VisitTypeDecl(clang::TypeDecl* decl) { + collector_.RecordOrderedDecl(decl); + return true; +} + bool GeneratorASTVisitor::VisitFunctionDecl(clang::FunctionDecl* decl) { if (decl->isCXXClassMember() || // Skip classes !decl->isExternC() || // Skip non external functions @@ -55,14 +62,16 @@ bool GeneratorASTVisitor::VisitFunctionDecl(clang::FunctionDecl* decl) { if (bool all_functions = options_.function_names.empty(); all_functions || options_.function_names.count(ToStringView(decl->getName())) > 0) { + clang::SourceManager& source_manager = + decl->getASTContext().getSourceManager(); + // Skip functions from system headers when all functions are requested. // This allows to still specify standard library functions explicitly. - if (all_functions && - decl->getASTContext().getSourceManager().isInSystemHeader( - decl->getBeginLoc())) { + if (all_functions && source_manager.isInSystemHeader(decl->getBeginLoc())) { return true; } + // TODO(cblichmann): Skip functions to implement limit_scan_depth feature. functions_.push_back(decl); collector_.CollectRelatedTypes(decl->getDeclaredReturnType()); @@ -81,7 +90,7 @@ void GeneratorASTConsumer::HandleTranslationUnit(clang::ASTContext& context) { return; } - emitter_.AddTypesFiltered(visitor_.collector_.collected()); + emitter_.AddTypeDeclarations(visitor_.collector_.GetTypeDeclarations()); for (clang::FunctionDecl* func : visitor_.functions_) { absl::Status status = emitter_.AddFunction(func); diff --git a/sandboxed_api/tools/clang_generator/generator.h b/sandboxed_api/tools/clang_generator/generator.h index 3e855f7..9e4f9a4 100644 --- a/sandboxed_api/tools/clang_generator/generator.h +++ b/sandboxed_api/tools/clang_generator/generator.h @@ -21,6 +21,7 @@ #include "absl/container/flat_hash_set.h" #include "absl/strings/string_view.h" #include "clang/AST/ASTConsumer.h" +#include "clang/AST/Decl.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" @@ -60,6 +61,7 @@ class GeneratorASTVisitor explicit GeneratorASTVisitor(const GeneratorOptions& options) : options_(options) {} + bool VisitTypeDecl(clang::TypeDecl* decl); bool VisitFunctionDecl(clang::FunctionDecl* decl); private: diff --git a/sandboxed_api/tools/clang_generator/types.cc b/sandboxed_api/tools/clang_generator/types.cc index 8acabe6..1c6e96d 100644 --- a/sandboxed_api/tools/clang_generator/types.cc +++ b/sandboxed_api/tools/clang_generator/types.cc @@ -14,8 +14,10 @@ #include "sandboxed_api/tools/clang_generator/types.h" +#include "absl/container/flat_hash_set.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" +#include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/QualTypeNames.h" #include "clang/AST/Type.h" @@ -34,6 +36,11 @@ bool IsFunctionReferenceType(clang::QualType qual) { } // namespace +void TypeCollector::RecordOrderedDecl(clang::TypeDecl* type_decl) { + // This implicitly assigns a number (its source order) to each declaration. + ordered_decls_.push_back(type_decl); +} + void TypeCollector::CollectRelatedTypes(clang::QualType qual) { if (!seen_.insert(qual)) { return; @@ -106,6 +113,44 @@ void TypeCollector::CollectRelatedTypes(clang::QualType qual) { } } +std::vector TypeCollector::GetTypeDeclarations() { + if (ordered_decls_.empty()) { + return {}; + } + + // The AST context is the same for all declarations in this translation unit, + // so use a reference here. + clang::ASTContext& context = ordered_decls_.front()->getASTContext(); + + absl::flat_hash_set collected_names; + for (clang::QualType qual : collected_) { + collected_names.insert(clang::TypeName::getFullyQualifiedName( + qual, context, context.getPrintingPolicy())); + } + + std::vector result; + for (clang::TypeDecl* type_decl : ordered_decls_) { + // Ideally, collected_.contains() on the underlying QualType of the TypeDecl + // would work here. However, QualTypes obtained from a TypeDecl contain + // different Type pointers, even when referring to one of the same types + // from the set and thus will not be found. Instead, work around the issue + // by always using the fully qualified name of the type. + if (!collected_names.contains(type_decl->getQualifiedNameAsString())) { + continue; + } + if (auto* tag_decl = llvm::dyn_cast(type_decl); + tag_decl && tag_decl->getTypedefNameForAnonDecl()) { + // Skip anonymous declarations that are typedef-ed. For example skip + // things like "typedef enum { A } SomeName". In this case, the enum is + // unnamed and the emitter will instead work with the complete typedef, so + // nothing is lost. + continue; + } + result.push_back(type_decl); + } + return result; +} + namespace { // Removes "const" from a qualified type if it denotes a pointer or reference diff --git a/sandboxed_api/tools/clang_generator/types.h b/sandboxed_api/tools/clang_generator/types.h index 3af27ae..cbc9607 100644 --- a/sandboxed_api/tools/clang_generator/types.h +++ b/sandboxed_api/tools/clang_generator/types.h @@ -18,7 +18,9 @@ #include #include +#include "absl/container/flat_hash_set.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" #include "clang/AST/Type.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallPtrSet.h" @@ -42,6 +44,12 @@ inline bool IsPointerOrReference(clang::QualType qual) { class TypeCollector { public: + // Records the source order of the given type in the current translation unit. + // This is different from collecting related types, as the emitter also needs + // to know in which order to emit typedefs vs forward decls, etc. and + // QualTypes only refer to complete definitions. + void RecordOrderedDecl(clang::TypeDecl* type_decl); + // Computes the transitive closure of all types that a type depends on. Those // are types that need to be declared before a declaration of the type denoted // by the qual parameter is valid. For example, given @@ -53,13 +61,15 @@ class TypeCollector { // // calling this function on the type "AggregateStruct" yields these types: // int - // SubStruct // bool + // SubStruct void CollectRelatedTypes(clang::QualType qual); - QualTypeSet& collected() { return collected_; } + // Returns the declarations for the collected types in source order. + std::vector GetTypeDeclarations(); private: + std::vector ordered_decls_; QualTypeSet collected_; QualTypeSet seen_; };