[lldb] Unify the two CreateTypedef implementations in TypeSystemClang

[teemperor]

Summary

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.

[JDevlieghere]

+1 on scrapping the jetliner idea.

LGTM if the Windows bot agrees that this is NFC.

[labath]

I wouldn't dismiss the jetliner idea completely. With all the recent upsets in the aviation industry, there may be an opening for us yet. :P

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

PdbAstBuilder::GetOrCreateTypedefDecl(PdbGlobalSymId id) {
 
   clang::DeclContext *scope = GetParentDeclContext(id);
 
   PdbTypeSymId real_type_id{udt.Type, false};
   clang::QualType qt = GetOrCreateType(real_type_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;
   status.uid = toOpaqueUid(id);
   m_decl_to_status.insert({tnd, status});
   return tnd;
 }
 

lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

case PDB_SymType::Typedef: {
 
     // Check if such a typedef already exists in the current context
     CompilerType ast_typedef =
         m_ast.GetTypeForIdentifier<clang::TypedefNameDecl>(ConstString(name),
                                                            decl_ctx);
     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;
 
       auto typedef_decl = TypeSystemClang::GetAsTypedefDecl(ast_typedef);
       assert(typedef_decl);
       m_uid_to_decl[type.getSymIndexId()] = typedef_decl;
     }
 

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

#endif
   GetMinimumLanguage(lldb::opaque_compiler_type_t type) override;
 
   lldb::TypeClass GetTypeClass(lldb::opaque_compiler_type_t type) override;
 
   unsigned GetTypeQualifiers(lldb::opaque_compiler_type_t type) override;
 
   // 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;
 
   CompilerType GetArrayType(lldb::opaque_compiler_type_t type,
                             uint64_t size) override;
 
   CompilerType GetCanonicalType(lldb::opaque_compiler_type_t type) override;
 
#endif
   CompilerType GetAtomicType(lldb::opaque_compiler_type_t type) override;
 
   CompilerType AddConstModifier(lldb::opaque_compiler_type_t type) override;
 
   CompilerType AddVolatileModifier(lldb::opaque_compiler_type_t type) override;
 
   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,
                              uint32_t opaque_payload) override;
 
   // If the current object represents a typedef type, get the underlying type
   CompilerType GetTypedefedType(lldb::opaque_compiler_type_t type) override;
 

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

 
 CompilerType
 TypeSystemClang::GetNonReferenceType(lldb::opaque_compiler_type_t type) {
   if (type)
     return GetType(GetQualType(type).getNonReferenceType());
   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) {
     clang::QualType qual_type(GetQualType(type));
     return GetType(qual_type.getTypePtr()->getPointeeType());
   }
   return CompilerType();
 }
if (type) {
     return GetType(result);
   }
   return CompilerType();
 }
 
 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));
 
     clang::DeclContext *decl_ctx =
         TypeSystemClang::DeclContextGetAsDeclContext(compiler_decl_ctx);
     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());
 
     clang::TagDecl *tdecl = nullptr;
     if (!qual_type.isNull()) {
       if (const clang::RecordType *rt = qual_type->getAs<clang::RecordType>())
         tdecl = rt->getDecl();
       if (const clang::EnumType *et = qual_type->getAs<clang::EnumType>())

lldb/unittests/Symbol/TestTypeSystemClang.cpp

ClassTemplateSpecializationDecl *spec_decl =
           TTK_Struct, infos);
   ASSERT_NE(spec_decl, nullptr);
   CompilerType type = m_ast->CreateClassTemplateSpecializationType(spec_decl);
   ASSERT_TRUE(type);
   m_ast->StartTagDeclarationDefinition(type);
   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(),
       m_ast->getASTContext()
           .getAutoType(ClangUtil::GetCanonicalQualType(typedef_type),
                        clang::AutoTypeKeyword::Auto, false)
           .getAsOpaquePtr());