[llvm-branch-commits] [lldb] 722247c - [lldb] Unify the two CreateTypedef implementations in TypeSystemClang

Raphael Isemann via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Dec 17 01:54:21 PST 2020


Author: Raphael Isemann
Date: 2020-12-17T10:49:26+01:00
New Revision: 722247c8124a6b840686757ae128b16cea248130

URL: https://github.com/llvm/llvm-project/commit/722247c8124a6b840686757ae128b16cea248130
DIFF: https://github.com/llvm/llvm-project/commit/722247c8124a6b840686757ae128b16cea248130.diff

LOG: [lldb] Unify the two CreateTypedef implementations in TypeSystemClang

To get LLDB one step closer to fulfil the software redundancy requirements of
modern aircrafts, we apparently decided to have two separately maintained
implementations of `CreateTypedef` in TypeSystemClang. Let's pass on the idea of
an LLDB-powered jetliner and deleted one implementation.

On a more serious note: This function got duplicated a long time ago when the
idea of CompilerType with a backing TypeSystemClang subclass happened
(56939cb31061d24ae3d1fc62da38b57e78bb2556). One implementation was supposed to
be called from CompilerType::CreateTypedef and the other has just always been
around to create typedefs. By accident one of the implementations is only used
by the PDB parser while the CompilerType::CreateTypedef backend is used by the
rest of LLDB.

We also had some patches over the year that only fixed one of the two functions
(D18099 for example only fixed up the CompilerType::CreateTypedef
implementation). D51162 and D86140 both fixed the same missing `addDecl` call
for one of the two implementations.

This patch:
* deletes the `CreateTypedefType` function as its only used by the PDB parser
  and the `CreateTypedef` implementation is anyway needed as it's the backend
  implementation of CompilerType.
* replaces the calls in the PDB parser by just calling the CompilerType wrapper.
* moves the documentation to the remaining function.
* moves the check for empty typedef names that was only in the deleted
  implementation to the other (I don't think this fixes anything as I believe
  all callers are already doing the same check).

I'll fix up the usual stuff (not using StringRef, not doing early exit) in a NFC
follow-up.

This patch is not NFC as the PDB parser now calls the function that has the fix
from D18099.

Reviewed By: labath, JDevlieghere

Differential Revision: https://reviews.llvm.org/D93382

Added: 
    

Modified: 
    lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
    lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
    lldb/unittests/Symbol/TestTypeSystemClang.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 21f8b13bf07f..5b4ab78ac219 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -881,8 +881,8 @@ PdbAstBuilder::GetOrCreateTypedefDecl(PdbGlobalSymId id) {
 
   std::string uname = std::string(DropNameScope(udt.Name));
 
-  CompilerType ct = m_clang.CreateTypedefType(ToCompilerType(qt), uname.c_str(),
-                                              ToCompilerDeclContext(*scope), 0);
+  CompilerType ct = ToCompilerType(qt).CreateTypedef(
+      uname.c_str(), ToCompilerDeclContext(*scope), 0);
   clang::TypedefNameDecl *tnd = m_clang.GetAsTypedefDecl(ct);
   DeclStatus status;
   status.resolved = true;

diff  --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index 7649e8a90f9a..f9c12e634140 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -550,8 +550,8 @@ lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const PDBSymbol &type) {
     if (!ast_typedef.IsValid()) {
       CompilerType target_ast_type = target_type->GetFullCompilerType();
 
-      ast_typedef = m_ast.CreateTypedefType(
-          target_ast_type, name.c_str(), m_ast.CreateDeclContext(decl_ctx), 0);
+      ast_typedef = target_ast_type.CreateTypedef(
+          name.c_str(), m_ast.CreateDeclContext(decl_ctx), 0);
       if (!ast_typedef)
         return nullptr;
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index f46b145da66c..643ea7e02206 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4402,39 +4402,6 @@ TypeSystemClang::GetNonReferenceType(lldb::opaque_compiler_type_t type) {
   return CompilerType();
 }
 
-CompilerType TypeSystemClang::CreateTypedefType(
-    const CompilerType &type, const char *typedef_name,
-    const CompilerDeclContext &compiler_decl_ctx, uint32_t payload) {
-  if (type && typedef_name && typedef_name[0]) {
-    TypeSystemClang *ast =
-        llvm::dyn_cast<TypeSystemClang>(type.GetTypeSystem());
-    if (!ast)
-      return CompilerType();
-    clang::ASTContext &clang_ast = ast->getASTContext();
-    clang::QualType qual_type(ClangUtil::GetQualType(type));
-
-    clang::DeclContext *decl_ctx =
-        TypeSystemClang::DeclContextGetAsDeclContext(compiler_decl_ctx);
-    if (!decl_ctx)
-      decl_ctx = ast->getASTContext().getTranslationUnitDecl();
-
-    clang::TypedefDecl *decl =
-        clang::TypedefDecl::CreateDeserialized(clang_ast, 0);
-    decl->setDeclContext(decl_ctx);
-    decl->setDeclName(&clang_ast.Idents.get(typedef_name));
-    decl->setTypeSourceInfo(clang_ast.getTrivialTypeSourceInfo(qual_type));
-
-    SetOwningModule(decl, TypePayloadClang(payload).GetOwningModule());
-    decl->setAccess(clang::AS_public); // TODO respect proper access specifier
-
-    decl_ctx->addDecl(decl);
-
-    // Get a uniqued clang::QualType for the typedef decl type
-    return ast->GetType(clang_ast.getTypedefType(decl));
-  }
-  return CompilerType();
-}
-
 CompilerType
 TypeSystemClang::GetPointeeType(lldb::opaque_compiler_type_t type) {
   if (type) {
@@ -4516,7 +4483,7 @@ TypeSystemClang::AddRestrictModifier(lldb::opaque_compiler_type_t type) {
 CompilerType TypeSystemClang::CreateTypedef(
     lldb::opaque_compiler_type_t type, const char *typedef_name,
     const CompilerDeclContext &compiler_decl_ctx, uint32_t payload) {
-  if (type) {
+  if (type && typedef_name && typedef_name[0]) {
     clang::ASTContext &clang_ast = getASTContext();
     clang::QualType qual_type(GetQualType(type));
 
@@ -4525,10 +4492,11 @@ CompilerType TypeSystemClang::CreateTypedef(
     if (!decl_ctx)
       decl_ctx = getASTContext().getTranslationUnitDecl();
 
-    clang::TypedefDecl *decl = clang::TypedefDecl::Create(
-        clang_ast, decl_ctx, clang::SourceLocation(), clang::SourceLocation(),
-        &clang_ast.Idents.get(typedef_name),
-        clang_ast.getTrivialTypeSourceInfo(qual_type));
+    clang::TypedefDecl *decl =
+        clang::TypedefDecl::CreateDeserialized(clang_ast, 0);
+    decl->setDeclContext(decl_ctx);
+    decl->setDeclName(&clang_ast.Idents.get(typedef_name));
+    decl->setTypeSourceInfo(clang_ast.getTrivialTypeSourceInfo(qual_type));
     decl_ctx->addDecl(decl);
     SetOwningModule(decl, TypePayloadClang(payload).GetOwningModule());
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index f17fc622ede4..484c251aa00e 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -665,14 +665,6 @@ class TypeSystemClang : public TypeSystem {
 
   // Creating related types
 
-  /// Using the current type, create a new typedef to that type using
-  /// "typedef_name" as the name and "decl_ctx" as the decl context.
-  /// \param opaque_payload is an opaque TypePayloadClang.
-  static CompilerType
-  CreateTypedefType(const CompilerType &type, const char *typedef_name,
-                    const CompilerDeclContext &compiler_decl_ctx,
-                    uint32_t opaque_payload);
-
   CompilerType GetArrayElementType(lldb::opaque_compiler_type_t type,
                                    ExecutionContextScope *exe_scope) override;
 
@@ -720,6 +712,9 @@ class TypeSystemClang : public TypeSystem {
 
   CompilerType AddRestrictModifier(lldb::opaque_compiler_type_t type) override;
 
+  /// Using the current type, create a new typedef to that type using
+  /// "typedef_name" as the name and "decl_ctx" as the decl context.
+  /// \param opaque_payload is an opaque TypePayloadClang.
   CompilerType CreateTypedef(lldb::opaque_compiler_type_t type,
                              const char *name,
                              const CompilerDeclContext &decl_ctx,

diff  --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 0348e734d240..e6a453bf912e 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -482,9 +482,8 @@ TEST_F(TestTypeSystemClang, TemplateArguments) {
   m_ast->CompleteTagDeclarationDefinition(type);
 
   // typedef foo<int, 47> foo_def;
-  CompilerType typedef_type = m_ast->CreateTypedefType(
-      type, "foo_def",
-      m_ast->CreateDeclContext(m_ast->GetTranslationUnitDecl()), 0);
+  CompilerType typedef_type = type.CreateTypedef(
+      "foo_def", m_ast->CreateDeclContext(m_ast->GetTranslationUnitDecl()), 0);
 
   CompilerType auto_type(
       m_ast.get(),


        


More information about the llvm-branch-commits mailing list