aeubanks updated this revision to Diff 465926.
aeubanks added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134378/new/

https://reviews.llvm.org/D134378

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
  lldb/test/API/lang/cpp/unique-types2/Makefile
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -0,0 +1,22 @@
+template <class T> class Foo {
+  T t;
+};
+
+template <class T, class... Ss> class FooPack {
+  T t;
+};
+
+int main() {
+  Foo<char> t1;
+  Foo<int> t2;
+  Foo<Foo<int>> t3;
+
+  FooPack<char> p1;
+  FooPack<int> p2;
+  FooPack<Foo<int>> p3;
+  FooPack<char, int> t4;
+  FooPack<char, float> t5;
+  FooPack<int, int> t6;
+  FooPack<int, int, int> t7;
+  // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
@@ -0,0 +1,40 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase(TestBase):
+    def do_test(self, debug_flags):
+        """Test that we only display the requested Foo instantiation, not all Foo instantiations."""
+        self.build(dictionary=debug_flags)
+        lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+        self.expect("image lookup -A -t 'Foo<char>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'Foo<int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'Foo<Foo<int> >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'Foo<float>'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+        self.expect("image lookup -A -t 'FooPack<char>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<Foo<int> >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<char, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<char, float>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<int, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<int, int, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+        self.expect("image lookup -A -t 'FooPack<float>'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+        self.expect("image lookup -A -t 'FooPack<float, int>'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+    @skipIf(compiler=no_match("clang"))
+    @skipIf(compiler_version=["<", "15.0"])
+    def test_simple_template_name(self):
+        self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+    @skipIf(compiler=no_match("clang"))
+    @skipIf(compiler_version=["<", "15.0"])
+    def test_no_simple_template_name(self):
+        self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
+
Index: lldb/test/API/lang/cpp/unique-types2/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===================================================================
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -51,7 +51,7 @@
         # Record types without a defining declaration are not complete.
         self.assertPointeeIncomplete("FwdClass *", "fwd_class")
         self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-        self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+        self.assertPointeeIncomplete("FwdTemplateClass<int> *", "fwd_template_class")
 
         # A pointer type is complete even when it points to an incomplete type.
         fwd_class_ptr = self.expect_expr("fwd_class", result_type="FwdClass *")
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9325,7 +9325,9 @@
     const TypeSystemClang::TemplateParameterInfos &template_param_infos) {
   if (template_param_infos.IsValid()) {
     std::string template_basename(parent_name);
-    template_basename.erase(template_basename.find('<'));
+    // With -gsimple-template-names we may omit template parameters in the name.
+    if (auto i = template_basename.find('<'); i != std::string::npos)
+      template_basename.erase(i);
 
     return CreateClassTemplateDecl(decl_ctx, owning_module, access_type,
                                    template_basename.c_str(), tag_decl_kind,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2492,6 +2492,45 @@
     return types.GetSize() < max_matches;
   });
 
+  // If we've come up empty and the name contains '<' meaning it contains
+  // templates parameters, try again with the template parameters stripped since
+  // with -gsimple-template-names the DT_name may only contain the base name.
+  if (types.Empty()) {
+    const llvm::StringRef nameRef = name.GetStringRef();
+    auto it = nameRef.find('<');
+    if (it != llvm::StringRef::npos) {
+      const llvm::StringRef nameNoTemplateParams = nameRef.slice(0, it);
+      const llvm::StringRef templateParams = nameRef.slice(it, nameRef.size());
+      m_index->GetTypes(ConstString(nameNoTemplateParams), [&](DWARFDIE die) {
+        if (!DIEInDeclContext(parent_decl_ctx, die))
+          return true; // The containing decl contexts don't match
+
+        TypeSP Ty = GetTypeForDIE(die);
+        llvm::StringRef qualName = Ty->GetQualifiedName().AsCString();
+        auto it = qualName.find('<');
+        // If the candidate qualified name doesn't have '<', it doesn't have
+        // template params to compare.
+        if (it == llvm::StringRef::npos)
+          return true;
+
+        // Filter out non-matching instantiations by comparing template params.
+        llvm::StringRef qualNameTemplateParams =
+            qualName.slice(it, qualName.size());
+        if (templateParams != qualNameTemplateParams)
+          return true;
+
+        Type *matching_type = ResolveType(die, true, true);
+        if (!matching_type)
+          return true;
+
+        // We found a type pointer, now find the shared pointer form our type
+        // list.
+        types.InsertUnique(matching_type->shared_from_this());
+        return types.GetSize() < max_matches;
+      });
+    }
+  }
+
   // Next search through the reachable Clang modules. This only applies for
   // DWARF objects compiled with -gmodules that haven't been processed by
   // dsymutil.
@@ -2960,8 +2999,6 @@
 
         if (!try_resolving_type) {
           if (log) {
-            std::string qualified_name;
-            type_die.GetQualifiedName(qualified_name);
             GetObjectFile()->GetModule()->LogMessage(
                 log,
                 "SymbolFileDWARF::"
@@ -2969,7 +3006,7 @@
                 "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;
         }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -99,11 +99,6 @@
 
   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,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -798,66 +798,6 @@
   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);
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -30,8 +30,6 @@
 
   const char *GetPubname() const;
 
-  const char *GetQualifiedName(std::string &storage) const;
-
   using DWARFBaseDIE::GetName;
   void GetName(lldb_private::Stream &s) const;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -210,13 +210,6 @@
     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
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -121,11 +121,18 @@
   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);
 
+  /// Get the template parameters of a die as a string if the die name does not
+  /// already contain them. This happens with -gsimple-template-names.
+  std::string GetTemplateParametersString(const DWARFDIE &die);
+
+  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,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1523,6 +1523,95 @@
   return type_sp;
 }
 
+std::string
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE &die) {
+  if (llvm::StringRef(die.GetName()).contains("<"))
+    return std::string();
+  TypeSystemClang::TemplateParameterInfos template_param_infos;
+  if (ParseTemplateParameterInfos(die, template_param_infos)) {
+    std::string all_template_names;
+    llvm::SmallVector<clang::TemplateArgument, 2> args =
+        template_param_infos.args;
+    if (template_param_infos.hasParameterPack()) {
+      args.append(template_param_infos.packed_args->args);
+    }
+    for (auto &arg : args) {
+      std::string template_name;
+      llvm::raw_string_ostream os(template_name);
+      arg.dump(os);
+      if (!template_name.empty()) {
+        if (all_template_names.empty()) {
+          all_template_names.append("<");
+        } else {
+          all_template_names.append(", ");
+        }
+        all_template_names.append(template_name);
+      }
+    }
+    if (!all_template_names.empty()) {
+      all_template_names.append(">");
+      // does the spacing here matter?
+    }
+    return all_template_names;
+  }
+  return std::string();
+}
+
+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) {
+    // The name may not contain template parameters due to simplified template
+    // names; we must reconstruct the full name from child template parameter
+    // dies via GetTemplateParametersString().
+    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,
+                              GetTemplateParametersString(parent_decl_ctx_die));
+        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);
+  qualified_name.append(GetTemplateParametersString(die));
+
+  return qualified_name;
+}
+
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
                                            const DWARFDIE &die,
@@ -1548,9 +1637,7 @@
       // 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))
-        unique_typename = ConstString(qualified_name);
+      unique_typename = ConstString(GetCPlusPlusQualifiedName(die));
       unique_decl.Clear();
     }
 
@@ -1723,36 +1810,36 @@
     metadata.SetUserID(die.GetID());
     metadata.SetIsDynamicCXXType(dwarf->ClassOrStructIsVirtual(die));
 
-    if (attrs.name.GetStringRef().contains('<')) {
-      TypeSystemClang::TemplateParameterInfos template_param_infos;
-      if (ParseTemplateParameterInfos(die, template_param_infos)) {
-        clang::ClassTemplateDecl *class_template_decl =
-            m_ast.ParseClassTemplateDecl(
-                decl_ctx, GetOwningClangModule(die), attrs.accessibility,
-                attrs.name.GetCString(), tag_decl_kind, template_param_infos);
-        if (!class_template_decl) {
-          if (log) {
-            dwarf->GetObjectFile()->GetModule()->LogMessage(
-                log,
-                "SymbolFileDWARF(%p) - 0x%8.8x: %s type \"%s\" "
-                "clang::ClassTemplateDecl failed to return a decl.",
-                static_cast<void *>(this), die.GetOffset(),
-                DW_TAG_value_to_name(tag), attrs.name.GetCString());
-          }
-          return TypeSP();
+    TypeSystemClang::TemplateParameterInfos template_param_infos;
+    if (ParseTemplateParameterInfos(die, template_param_infos) &&
+        (!template_param_infos.args.empty() ||
+         template_param_infos.packed_args)) {
+      clang::ClassTemplateDecl *class_template_decl =
+          m_ast.ParseClassTemplateDecl(
+              decl_ctx, GetOwningClangModule(die), attrs.accessibility,
+              attrs.name.GetCString(), tag_decl_kind, template_param_infos);
+      if (!class_template_decl) {
+        if (log) {
+          dwarf->GetObjectFile()->GetModule()->LogMessage(
+              log,
+              "SymbolFileDWARF(%p) - 0x%8.8x: %s type \"%s\" "
+              "clang::ClassTemplateDecl failed to return a decl.",
+              static_cast<void *>(this), die.GetOffset(),
+              DW_TAG_value_to_name(tag), attrs.name.GetCString());
         }
+        return TypeSP();
+      }
 
-        clang::ClassTemplateSpecializationDecl *class_specialization_decl =
-            m_ast.CreateClassTemplateSpecializationDecl(
-                decl_ctx, GetOwningClangModule(die), class_template_decl,
-                tag_decl_kind, template_param_infos);
-        clang_type = m_ast.CreateClassTemplateSpecializationType(
-            class_specialization_decl);
-        clang_type_was_created = true;
+      clang::ClassTemplateSpecializationDecl *class_specialization_decl =
+          m_ast.CreateClassTemplateSpecializationDecl(
+              decl_ctx, GetOwningClangModule(die), class_template_decl,
+              tag_decl_kind, template_param_infos);
+      clang_type = m_ast.CreateClassTemplateSpecializationType(
+          class_specialization_decl);
+      clang_type_was_created = true;
 
-        m_ast.SetMetadata(class_template_decl, metadata);
-        m_ast.SetMetadata(class_specialization_decl, metadata);
-      }
+      m_ast.SetMetadata(class_template_decl, metadata);
+      m_ast.SetMetadata(class_specialization_decl, metadata);
     }
 
     if (!clang_type_was_created) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to