[lldb][NFCish] Move DWARFDebugInfoEntry::GetQualifiedName() into DWARFASTParserClang

[aeubanks]

Summary

In D134378, we'll need the clang AST to be able to construct the qualified in some cases.

This makes logging in one place slightly less informative.

[aeubanks]

update

[dblaikie]

Looked like this was a fairly uncontroversial part of the other patch, so I feel confident approving it - but welcome to wait for a second opinion from more lldb-affiliated developers.

[Michael137]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1602

Overall LGTM. Correct me if I missed something, but the main functional change is that previously when GetQualifiedName returned nullptr we didn't re-assign it to unique_typename. However, now we would unconditionally assign. Does that matter? Should we only re-assign if the string is non-empty? Or should we use some Optional type?

[aeubanks]

check if string is empty before assigning to unique_typename

[shafik]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1534

So it looks like you can do SymbolFileDWARF::GetDWARFDeclContext(...) and DWARFDeclContext has GetQualifiedName() which I think is doing what you are doing below and maybe handles more cases.

[aeubanks]

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1534

DWARFDeclContext doesn't have access to the clang AST, which in D134378 we will use to reconstruct template parameters (I hope I'm understanding your suggestion correctly, still new to lldb)

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

protected:
 
   clang::BlockDecl *ResolveBlockDIE(const DWARFDIE &die);
 
   clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE &die);
 
   bool ParseTemplateDIE(const DWARFDIE &die,
                         lldb_private::TypeSystemClang::TemplateParameterInfos
                             &template_param_infos);
+
   bool ParseTemplateParameterInfos(
       const DWARFDIE &parent_die,
       lldb_private::TypeSystemClang::TemplateParameterInfos
           &template_param_infos);
 
+  std::string GetCPlusPlusQualifiedName(const DWARFDIE &die);
+
   bool ParseChildMembers(
       const DWARFDIE &die, lldb_private::CompilerType &class_compiler_type,
       std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> &base_classes,
       std::vector<DWARFDIE> &member_function_dies,
       DelayedPropertyList &delayed_properties,
       const lldb::AccessType default_accessibility,
       lldb_private::ClangASTImporter::LayoutInfo &layout_info);
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType(
 
   if (symbol_context_scope != nullptr)
     type_sp->SetSymbolContextScope(symbol_context_scope);
 
   dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
   return type_sp;
 }
 
+std::string
+DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
+  if (!die.IsValid())
+    return "";
+  const char *name = die.GetName();
+  if (!name)
+    return "";
+  std::string qualified_name;
+  DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+  // TODO: change this to get the correct decl context parent....
+  while (parent_decl_ctx_die) {
+    const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
+    switch (parent_tag) {
+    case DW_TAG_namespace: {
+      if (const char *namespace_name = parent_decl_ctx_die.GetName()) {
+        qualified_name.insert(0, "::");
+        qualified_name.insert(0, namespace_name);
+      } else {
+        qualified_name.insert(0, "(anonymous namespace)::");
+      }
+      parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
+      break;
+    }
+
+    case DW_TAG_class_type:
+    case DW_TAG_structure_type:
+    case DW_TAG_union_type: {
+      if (const char *class_union_struct_name = parent_decl_ctx_die.GetName()) {
+        qualified_name.insert(0, "::");
+        qualified_name.insert(0, class_union_struct_name);
+      }
+      parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
+      break;
+    }
+
+    default:
+      parent_decl_ctx_die.Clear();
+      break;
+    }
+  }
+
+  if (qualified_name.empty())
+    qualified_name.append("::");
+
+  qualified_name.append(name);
+
+  return qualified_name;
+}
+
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
                                            const DWARFDIE &die,
                                            ParsedDWARFTypeAttributes &attrs) {
   TypeSP type_sp;
   CompilerType clang_type;
   const dw_tag_t tag = die.Tag();
   SymbolFileDWARF *dwarf = die.GetDWARF();
DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
   ConstString unique_typename(attrs.name);
   Declaration unique_decl(attrs.decl);
 
   if (attrs.name) {
     if (Language::LanguageIsCPlusPlus(cu_language)) {
       // For C++, we rely solely upon the one definition rule that says
       // only one thing can exist at a given decl context. We ignore the
       // file and line that things are declared on.
-      std::string qualified_name;
-      if (die.GetQualifiedName(qualified_name))
+      std::string qualified_name = GetCPlusPlusQualifiedName(die);
+      if (!qualified_name.empty())
         unique_typename = ConstString(qualified_name);
       unique_decl.Clear();
     }
 
     if (dwarf->GetUniqueDWARFASTTypeMap().Find(
             unique_typename, die, unique_decl, attrs.byte_size.value_or(-1),
             *unique_ast_entry_up)) {
       type_sp = unique_ast_entry_up->m_type_sp;
       if (type_sp) {

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h

public:
 
   // Accessors
 
   // Accessing information about a DIE
   const char *GetMangledName() const;
 
   const char *GetPubname() const;
 
-  const char *GetQualifiedName(std::string &storage) const;
-
   using DWARFBaseDIE::GetName;
   void GetName(lldb_private::Stream &s) const;
 
   void AppendTypeName(lldb_private::Stream &s) const;
 
   lldb_private::Type *ResolveType() const;
 
   // Resolve a type by UID using this DIE's DWARF file

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp

 
 const char *DWARFDIE::GetPubname() const {
   if (IsValid())
     return m_die->GetPubname(m_cu);
   else
     return nullptr;
 }
 
-const char *DWARFDIE::GetQualifiedName(std::string &storage) const {
-  if (IsValid())
-    return m_die->GetQualifiedName(m_cu, storage);
-  else
-    return nullptr;
-}
-
 // GetName
 //
 // Get value of the DW_AT_name attribute and place that value into the supplied
 // stream object. If the DIE is a NULL object "NULL" is placed into the stream,
 // and if no DW_AT_name attribute exists for the DIE then nothing is printed.
 void DWARFDIE::GetName(Stream &s) const {
   if (!IsValid())
     return;

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

public:
 
   const char *GetName(const DWARFUnit *cu) const;
 
   const char *GetMangledName(const DWARFUnit *cu,
                              bool substitute_name_allowed = true) const;
 
   const char *GetPubname(const DWARFUnit *cu) const;
 
-  const char *GetQualifiedName(DWARFUnit *cu, std::string &storage) const;
-
-  const char *GetQualifiedName(DWARFUnit *cu, const DWARFAttributes &attributes,
-                               std::string &storage) const;
-
   bool GetDIENamesAndRanges(
       DWARFUnit *cu, const char *&name, const char *&mangled,
       DWARFRangeList &rangeList, int &decl_file, int &decl_line,
       int &decl_column, int &call_file, int &call_line, int &call_column,
       lldb_private::DWARFExpressionList *frame_base = nullptr) const;
 
   const DWARFAbbreviationDeclaration *
   GetAbbreviationDeclarationPtr(const DWARFUnit *cu) const;

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

if (abs_die) {
         return decl_ctx_die;
     }
 
     die = die.GetParent();
   }
   return DWARFDIE();
 }
 
-const char *DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-                                                  std::string &storage) const {
-  DWARFAttributes attributes;
-  GetAttributes(cu, attributes, Recurse::yes);
-  return GetQualifiedName(cu, attributes, storage);
-}
-
-const char *
-DWARFDebugInfoEntry::GetQualifiedName(DWARFUnit *cu,
-                                      const DWARFAttributes &attributes,
-                                      std::string &storage) const {
-
-  const char *name = GetName(cu);
-
-  if (name) {
-    DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
-    storage.clear();
-    // TODO: change this to get the correct decl context parent....
-    while (parent_decl_ctx_die) {
-      const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
-      switch (parent_tag) {
-      case DW_TAG_namespace: {
-        const char *namespace_name = parent_decl_ctx_die.GetName();
-        if (namespace_name) {
-          storage.insert(0, "::");
-          storage.insert(0, namespace_name);
-        } else {
-          storage.insert(0, "(anonymous namespace)::");
-        }
-        parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-      } break;
-
-      case DW_TAG_class_type:
-      case DW_TAG_structure_type:
-      case DW_TAG_union_type: {
-        const char *class_union_struct_name = parent_decl_ctx_die.GetName();
-
-        if (class_union_struct_name) {
-          storage.insert(0, "::");
-          storage.insert(0, class_union_struct_name);
-        }
-        parent_decl_ctx_die = parent_decl_ctx_die.GetParentDeclContextDIE();
-      } break;
-
-      default:
-        parent_decl_ctx_die.Clear();
-        break;
-      }
-    }
-
-    if (storage.empty())
-      storage.append("::");
-
-    storage.append(name);
-  }
-  if (storage.empty())
-    return nullptr;
-  return storage.c_str();
-}
-
 lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
   return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
 }
 
 const DWARFAbbreviationDeclaration *
 DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(const DWARFUnit *cu) const {
   if (cu) {
     const DWARFAbbreviationDeclarationSet *abbrev_set = cu->GetAbbreviations();

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (type_name) {
             // Tags don't match, don't event try to resolve using this type
             // whose name matches....
             break;
           }
         }
 
         if (!try_resolving_type) {
           if (log) {
-            std::string qualified_name;
-            type_die.GetQualifiedName(qualified_name);
             GetObjectFile()->GetModule()->LogMessage(
                 log,
                 "SymbolFileDWARF::"
                 "FindDefinitionTypeForDWARFDeclContext(tag=%s, "
                 "qualified-name='%s') ignoring die=0x%8.8x (%s)",
                 DW_TAG_value_to_name(dwarf_decl_ctx[0].tag),
                 dwarf_decl_ctx.GetQualifiedName(), type_die.GetOffset(),
-                qualified_name.c_str());
+                type_die.GetName());
           }
           return true;
         }
 
         DWARFDeclContext type_dwarf_decl_ctx = GetDWARFDeclContext(type_die);
 
         if (log) {
           GetObjectFile()->GetModule()->LogMessage(