Skip to content

Commit cff8937

Browse files
cblichmanncopybara-github
authored andcommitted
clang_generator: Use a single function to output type names to the final header
This aligns with the v1 header generator behavior and also prevents us from emitting long namespaced names while in the respective namespaces. Fixes some instances of nested namespaces in headers as well (rare). Drive-by: - Mark fields in `EmitterBase` as `protected` - Do not call deprecated functions (Clang API) PiperOrigin-RevId: 785428374 Change-Id: I1279d7943175aab96872285073ffa78a09b744bb
1 parent 547ba1c commit cff8937

6 files changed

Lines changed: 62 additions & 43 deletions

File tree

sandboxed_api/tools/clang_generator/emitter.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,9 @@ namespace sapi {
4242

4343
// Common header description with auto-generation notice.
4444
constexpr absl::string_view kHeaderDescription =
45-
R"(// AUTO-GENERATED by the Sandboxed API generator.
46-
// Edits will be discarded when regenerating this file.)";
45+
R"(// AUTO-GENERATED by the Sandboxed API Clang tool.
46+
// Edits will be discarded when regenerating this file.
47+
)";
4748

4849
// Common header file prolog with auto-generation notice.
4950
// Note: The includes will be adjusted by Copybara when converting to/from
@@ -121,7 +122,7 @@ absl::StatusOr<std::string> PrintFunctionPrototypeComment(
121122
const TypeMapper& type_mapper, const clang::FunctionDecl* decl) {
122123
std::string out = absl::StrCat(
123124
type_mapper.MapQualTypeParameterForCxx(decl->getDeclaredReturnType()),
124-
" ", decl->getQualifiedNameAsString(), "(");
125+
" ", ToStringView(decl->getName()), "(");
125126

126127
std::string print_separator;
127128
for (int i = 0; i < decl->getNumParams(); ++i) {

sandboxed_api/tools/clang_generator/emitter.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ class Emitter : public EmitterBase {
4343
// Outputs a formatted header for a list of functions and their related types.
4444
absl::StatusOr<std::string> EmitHeader();
4545

46-
protected:
4746
// Rendered function bodies, as a vector to preserve source order. This is
4847
// not strictly necessary, but makes the output look less surprising.
4948
std::vector<std::string> rendered_functions_ordered_;

sandboxed_api/tools/clang_generator/emitter_base.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ std::string PrintDecl(const clang::Decl* decl) {
157157
std::string pretty;
158158
llvm::raw_string_ostream os(pretty);
159159
decl->print(os);
160-
return os.str();
160+
return pretty;
161161
}
162162

163163
// Returns the spelling for a given declaration will be emitted to the final
@@ -308,7 +308,7 @@ std::string EmitInclude(const IncludeInfo& info) {
308308
return out;
309309
}
310310

311-
void EmitterBase::AddIncludes(IncludeInfo* include) {
311+
void EmitterBase::AddIncludes(const IncludeInfo* include) {
312312
std::string include_str = EmitInclude(*include);
313313
if (!include_str.empty()) {
314314
rendered_includes_ordered_.insert(include_str);

sandboxed_api/tools/clang_generator/emitter_base.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,17 @@ class EmitterBase {
9393
virtual absl::Status AddFunction(clang::FunctionDecl* decl) = 0;
9494

9595
// Adds an include to the list of includes to be rendered.
96-
void AddIncludes(IncludeInfo* include);
96+
void AddIncludes(const IncludeInfo* include);
9797

98+
const absl::btree_map<std::string, std::vector<IncludeInfo>>&
99+
collected_includes() const {
100+
return collected_includes_;
101+
}
102+
absl::btree_map<std::string, std::vector<IncludeInfo>>& collected_includes() {
103+
return collected_includes_;
104+
}
105+
106+
protected:
98107
// Stores namespaces and a list of spellings for types. Keeps track of types
99108
// that have been rendered so far. Using a node_hash_set for pointer
100109
// stability.

sandboxed_api/tools/clang_generator/generator.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ void GeneratorASTConsumer::HandleTranslationUnit(clang::ASTContext& context) {
206206
return;
207207
}
208208

209-
for (auto& [parse_ctx, includes] : emitter_.collected_includes_) {
209+
for (auto& [parse_ctx, includes] : emitter_.collected_includes()) {
210210
for (auto& include : includes) {
211211
emitter_.AddIncludes(&include);
212212
}
@@ -237,7 +237,7 @@ bool GeneratorAction::BeginSourceFileAction(clang::CompilerInstance& ci) {
237237
.getFileEntryRefForID(ci.getSourceManager().getMainFileID())
238238
->getName()
239239
.str(),
240-
ci.getSourceManager(), emitter_.collected_includes_));
240+
ci.getSourceManager(), emitter_.collected_includes()));
241241
return true;
242242
}
243243

sandboxed_api/tools/clang_generator/types.cc

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "absl/strings/match.h"
2424
#include "absl/strings/str_cat.h"
2525
#include "absl/strings/str_join.h"
26+
#include "absl/strings/str_replace.h"
2627
#include "absl/strings/string_view.h"
2728
#include "clang/AST/ASTContext.h"
2829
#include "clang/AST/Decl.h"
@@ -57,8 +58,10 @@ bool IsProtoBuf(const clang::RecordDecl* decl) {
5758
//
5859
// This function handles some special cases, such as function pointers and
5960
// enums. In case of enums it preserves the enum keyword in the name.
60-
std::string GetQualTypeName(const clang::ASTContext& context,
61-
clang::QualType qual) {
61+
std::string GetFullyQualifiedName(const clang::ASTContext& context,
62+
clang::QualType qual,
63+
absl::string_view ns_to_strip = "",
64+
bool suppress_enum_keyword = true) {
6265
// Remove any "const", "volatile", etc. except for those added via typedefs.
6366
clang::QualType unqual = qual.getLocalUnqualifiedType();
6467

@@ -68,19 +71,24 @@ std::string GetQualTypeName(const clang::ASTContext& context,
6871
unqual = unqual->getPointeeType();
6972
}
7073

71-
if (unqual->isEnumeralType()) {
72-
auto decl = unqual->getAsTagDecl();
73-
if (decl) {
74-
// Keep the "enum" keyword in the type name.
75-
clang::PrintingPolicy policy = context.getPrintingPolicy();
76-
policy.SuppressTagKeyword = false;
77-
return clang::TypeName::getFullyQualifiedName(unqual, context, policy);
78-
}
74+
clang::PrintingPolicy policy = context.getPrintingPolicy();
75+
if (!suppress_enum_keyword && unqual->isEnumeralType() &&
76+
unqual->getAsTagDecl() != nullptr) {
77+
// Keep the "enum" keyword in the type name.
78+
policy.SuppressTagKeyword = false;
79+
}
80+
81+
// Get the fully qualified name without the "struct" or "class" keyword.
82+
std::string qual_name =
83+
clang::TypeName::getFullyQualifiedName(unqual, context, policy);
84+
85+
if (!ns_to_strip.empty()) {
86+
// Remove the namespace prefix if requested. This is using a textual
87+
// replacement for ease of implementation. A fully generic solution would
88+
// require to implement a custom printer for the QualType.
89+
absl::StrReplaceAll({{absl::StrCat(ns_to_strip, "::"), ""}}, &qual_name);
7990
}
80-
// Return the fully qualified name without the "enum", "struct" or "class"
81-
// keyword.
82-
return clang::TypeName::getFullyQualifiedName(unqual, context,
83-
context.getPrintingPolicy());
91+
return qual_name;
8492
}
8593

8694
// Returns the namespace components of a declaration's qualified name.
@@ -216,7 +224,8 @@ std::vector<NamespacedTypeDecl> TypeCollector::GetTypeDeclarations() {
216224
// only emit type declarations of required types.
217225
absl::flat_hash_set<std::string> collected_names;
218226
for (clang::QualType qual : collected_) {
219-
const std::string qual_name = GetQualTypeName(context, qual);
227+
const std::string qual_name = GetFullyQualifiedName(
228+
context, qual, /*ns_to_strip=*/"", /*suppress_enum_keyword=*/false);
220229
collected_names.insert(qual_name);
221230
}
222231

@@ -245,7 +254,9 @@ std::vector<NamespacedTypeDecl> TypeCollector::GetTypeDeclarations() {
245254
// different Type pointers, even when referring to one of the same types
246255
// from the set and thus will not be found. Instead, work around the issue
247256
// by always using the fully qualified name of the type.
248-
const std::string qual_name = GetQualTypeName(context, type_decl_type);
257+
const std::string qual_name =
258+
GetFullyQualifiedName(context, type_decl_type, /*ns_to_strip=*/"",
259+
/*suppress_enum_keyword=*/false);
249260
if (!collected_names.contains(qual_name)) {
250261
continue;
251262
}
@@ -381,28 +392,28 @@ std::string TypeMapper::MapQualType(clang::QualType qual) const {
381392
}
382393
} else if (const auto* enum_type = qual->getAs<clang::EnumType>()) {
383394
clang::EnumDecl* enum_decl = enum_type->getDecl();
384-
std::string name;
385-
if (auto* typedef_name = enum_decl->getTypedefNameForAnonDecl()) {
386-
name = typedef_name->getQualifiedNameAsString();
387-
} else {
388-
name = enum_decl->getQualifiedNameAsString();
395+
if (auto* typedef_decl = enum_decl->getTypedefNameForAnonDecl()) {
396+
qual = typedef_decl->getUnderlyingType().getDesugaredType(context_);
389397
}
390-
return absl::StrCat("::sapi::v::IntBase<", name, ">");
398+
return absl::StrCat("::sapi::v::IntBase<",
399+
GetFullyQualifiedName(context_, qual, ns_to_strip_),
400+
">");
391401
} else if (IsPointerOrReference(qual)) {
392402
// Remove "const" qualifier from a pointer or reference type's pointee, as
393403
// e.g. const pointers do not work well with SAPI.
394-
return absl::StrCat("::sapi::v::Reg<",
395-
clang::TypeName::getFullyQualifiedName(
396-
MaybeRemoveConst(context_, qual), context_,
397-
context_.getPrintingPolicy()),
398-
">");
404+
return absl::StrCat(
405+
"::sapi::v::Reg<",
406+
GetFullyQualifiedName(context_, MaybeRemoveConst(context_, qual),
407+
ns_to_strip_),
408+
">");
399409
}
410+
400411
// Best-effort mapping to "int", leave a comment.
401-
return absl::StrCat("::sapi::v::Int /* aka '",
402-
clang::TypeName::getFullyQualifiedName(
403-
MaybeRemoveConst(context_, qual), context_,
404-
context_.getPrintingPolicy()),
405-
"' */");
412+
return absl::StrCat(
413+
"::sapi::v::Int /* aka '",
414+
GetFullyQualifiedName(context_, MaybeRemoveConst(context_, qual),
415+
ns_to_strip_),
416+
"' */");
406417
}
407418

408419
std::string TypeMapper::MapQualTypeParameterForCxx(clang::QualType qual) const {
@@ -414,8 +425,7 @@ std::string TypeMapper::MapQualTypeParameterForCxx(clang::QualType qual) const {
414425
// - long long -> uint64_t
415426
// - ...
416427
}
417-
return clang::TypeName::getFullyQualifiedName(qual, context_,
418-
context_.getPrintingPolicy());
428+
return GetFullyQualifiedName(context_, qual, ns_to_strip_);
419429
}
420430

421431
std::string TypeMapper::MapQualTypeParameter(clang::QualType qual) const {

0 commit comments

Comments
 (0)