[lldb] Fix std::unordered_* synthetic children when typedefs are used.

[clayborg]

There was a bug in both the GNU and libc++ library synthetic child providers when a typedef was used in the type of the variable. Previous code was looking at the top level typename to try and determine if std::unordered_ was a map or set and this failed when typedefs were being used. This patch fixes both C++ library synthetic child providers with updated tests.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

Changes

There was a bug in both the GNU and libc++ library synthetic child providers when a typedef was used in the type of the variable. Previous code was looking at the top level typename to try and determine if std::unordered_ was a map or set and this failed when typedefs were being used. This patch fixes both C++ library synthetic child providers with updated tests.

---

Full diff: https://github.com/llvm/llvm-project/pull/123125.diff

4 Files Affected:

• (modified) lldb/examples/synthetic/gnu_libstdcpp.py (+2-10)
• (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp (+2-1)
• (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py (+6-6)
• (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp (+18-6)

diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py
index a6605a7a7eb5b3..20b9488af55977 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -61,19 +61,11 @@ class StdUnorderedMapSynthProvider:
     def __init__(self, valobj, dict):
         self.valobj = valobj
         self.count = None
-        self.kind = self.get_object_kind(valobj)
-
-    def get_object_kind(self, valobj):
-        type_name = valobj.GetTypeName()
-        return "set" if "set" in type_name else "map"
 
     def extract_type(self):
         type = self.valobj.GetType()
-        # type of std::pair<key, value> is the first template
-        # argument type of the 4th template argument to std::map and
-        # 3rd template argument for std::set. That's why
-        # we need to know kind of the object
-        template_arg_num = 4 if self.kind == "map" else 3
+        # The last template argument is the allocator type.
+        template_arg_num = type.GetNumberOfTemplateArguments() - 1
         allocator_type = type.GetTemplateArgumentType(template_arg_num)
         data_type = allocator_type.GetTemplateArgumentType(0)
         return data_type
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
index bf91fc42482f3f..be520ee27af06c 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -111,7 +111,8 @@ CompilerType lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::
   // that wraps a std::pair. Peel away the internal wrapper type - whose
   // structure is of no value to users, to expose the std::pair. This
   // matches the structure returned by the std::map synthetic provider.
-  if (isUnorderedMap(m_backend.GetTypeName())) {
+  if (isUnorderedMap(
+          m_backend.GetCompilerType().GetCanonicalType().GetTypeName())) {
     std::string name;
     CompilerType field_type =
         element_type.GetFieldAtIndex(0, name, nullptr, nullptr, nullptr);
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
index 59c24bcead4a4a..c3043b489d951b 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py
@@ -54,7 +54,7 @@ def cleanup():
         self.look_for_content_and_continue(
             "map",
             [
-                "%s::unordered_map" % ns,
+                "UnorderedMap",
                 children_are_key_value,
                 "size=5 {",
                 "hello",
@@ -68,7 +68,7 @@ def cleanup():
         self.look_for_content_and_continue(
             "mmap",
             [
-                "%s::unordered_multimap" % ns,
+                "UnorderedMultiMap",
                 children_are_key_value,
                 "size=6 {",
                 "first = 3",
@@ -81,7 +81,7 @@ def cleanup():
         self.look_for_content_and_continue(
             "iset",
             [
-                "%s::unordered_set" % ns,
+                "IntsUnorderedSet",
                 "size=5 {",
                 "\[\d\] = 5",
                 "\[\d\] = 3",
@@ -92,7 +92,7 @@ def cleanup():
         self.look_for_content_and_continue(
             "sset",
             [
-                "%s::unordered_set" % ns,
+                "StringsUnorderedSet",
                 "size=5 {",
                 '\[\d\] = "is"',
                 '\[\d\] = "world"',
@@ -103,7 +103,7 @@ def cleanup():
         self.look_for_content_and_continue(
             "imset",
             [
-                "%s::unordered_multiset" % ns,
+                "IntsUnorderedMultiSet",
                 "size=6 {",
                 "(\[\d\] = 3(\\n|.)+){3}",
                 "\[\d\] = 2",
@@ -114,7 +114,7 @@ def cleanup():
         self.look_for_content_and_continue(
             "smset",
             [
-                "%s::unordered_multiset" % ns,
+                "StringsUnorderedMultiSet",
                 "size=5 {",
                 '(\[\d\] = "is"(\\n|.)+){2}',
                 '(\[\d\] = "world"(\\n|.)+){2}',
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp
index 00d37dcb4bd040..59a5166c505b35 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unordered/main.cpp
@@ -18,7 +18,9 @@ int main() {
   char buffer[sizeof(std::unordered_map<int, std::string>)] = {0};
   std::unordered_map<int, std::string> &corrupt_map = *(std::unordered_map<int, std::string> *)buffer;
 
-  std::unordered_map<int, std::string> map; // Set break point at this line.
+  // Make a typedef to ensure functionality when typedefs are used.
+  typedef std::unordered_map<int, std::string> UnorderedMap;
+  UnorderedMap map; // Set break point at this line.
   map.emplace(1, "hello");
   map.emplace(2, "world");
   map.emplace(3, "this");
@@ -26,7 +28,9 @@ int main() {
   map.emplace(5, "me");
   thefoo_rw(); // Set break point at this line.
 
-  std::unordered_multimap<int, std::string> mmap;
+  // Make a typedef to ensure functionality when typedefs are used.
+  typedef std::unordered_multimap<int, std::string> UnorderedMultiMap;
+  UnorderedMultiMap mmap;
   mmap.emplace(1, "hello");
   mmap.emplace(2, "hello");
   mmap.emplace(2, "world");
@@ -35,7 +39,9 @@ int main() {
   mmap.emplace(3, "this");
   thefoo_rw(); // Set break point at this line.
 
-  std::unordered_set<int> iset;
+  // Make a typedef to ensure functionality when typedefs are used.
+  typedef std::unordered_set<int> IntsUnorderedSet;
+  IntsUnorderedSet iset;
   iset.emplace(1);
   iset.emplace(2);
   iset.emplace(3);
@@ -43,7 +49,9 @@ int main() {
   iset.emplace(5);
   thefoo_rw(); // Set break point at this line.
 
-  std::unordered_set<std::string> sset;
+  // Make a typedef to ensure functionality when typedefs are used.
+  typedef std::unordered_set<std::string> StringsUnorderedSet;
+  StringsUnorderedSet sset;
   sset.emplace("hello");
   sset.emplace("world");
   sset.emplace("this");
@@ -51,7 +59,9 @@ int main() {
   sset.emplace("me");
   thefoo_rw(); // Set break point at this line.
 
-  std::unordered_multiset<int> imset;
+  // Make a typedef to ensure functionality when typedefs are used.
+  typedef std::unordered_multiset<int> IntsUnorderedMultiSet;
+  IntsUnorderedMultiSet imset;
   imset.emplace(1);
   imset.emplace(2);
   imset.emplace(2);
@@ -60,7 +70,9 @@ int main() {
   imset.emplace(3);
   thefoo_rw(); // Set break point at this line.
 
-  std::unordered_multiset<std::string> smset;
+  // Make a typedef to ensure functionality when typedefs are used.
+  typedef std::unordered_multiset<std::string> StringsUnorderedMultiSet;
+  StringsUnorderedMultiSet smset;
   smset.emplace("hello");
   smset.emplace("world");
   smset.emplace("world");

[Jlalond]

Great to see a quick follow up on this. I left a comment, but I think we should still include some test cases not behind a typedef, and then include your test changes as well to cover all the bases.

[Jlalond]

I think we should re-add at least one test case to ensure we're testing with and without the typedef.

[clayborg]

Going to give others time to give feedback before merging.