Skip to content

[lldb][DataFormatter] Unwrap reference type when formatting std::unordered_map #145872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 26, 2025

Conversation

Michael137
Copy link
Member

Desugar any potential references/typedefs before checking isStdTemplate. Previously, the typename might've been:

const std::unordered_map<...> &

for references. This patch gets the pointee type before grabbing the canonical type. GetNonReferenceType will unwrap typedefs too, so we should always end up with a non-reference before we get to GetCanonicalType.

#145847

…dered_map

Desugar any potential references/typedefs before checking
`isStdTemplate`. Previously, the typename might've been:
```
const std::unordered_map<...> &
```
for references. This patch gets the pointee type before grabbing the
canonical type. `GetNonReferenceType` will unwrap typedefs too, so we
should always end up with a non-reference before we get to
`GetCanonicalType`.

llvm#145847
@Michael137 Michael137 requested a review from labath June 26, 2025 11:28
@Michael137 Michael137 requested a review from JDevlieghere as a code owner June 26, 2025 11:28
@llvmbot llvmbot added the lldb label Jun 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

Desugar any potential references/typedefs before checking isStdTemplate. Previously, the typename might've been:

const std::unordered_map&lt;...&gt; &amp;

for references. This patch gets the pointee type before grabbing the canonical type. GetNonReferenceType will unwrap typedefs too, so we should always end up with a non-reference before we get to GetCanonicalType.

#145847


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

3 Files Affected:

  • (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp (+4-2)
  • (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py (+128)
  • (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/main.cpp (+13)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
index ffc33395830bb..501fd0945b82c 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -113,8 +113,10 @@ CompilerType lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd::
   // 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.GetCompilerType().GetCanonicalType().GetTypeName())) {
+  if (isUnorderedMap(m_backend.GetCompilerType()
+                         .GetNonReferenceType()
+                         .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/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py
index c021a46a17b51..3b412996c6cb4 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py
@@ -64,3 +64,131 @@ def test_iterator_formatters(self):
                 ValueCheck(name="second", summary='"Qux"'),
             ],
         )
+
+        lldbutil.continue_to_breakpoint(process, bkpt)
+
+        # Test references to std::unordered_map
+        self.expect_var_path(
+            "ref1",
+            summary="size=2",
+            type="const StringMapT &",
+            children=[
+                ValueCheck(
+                    name="[0]",
+                    children=[
+                        ValueCheck(name="first", summary='"Baz"'),
+                        ValueCheck(name="second", summary='"Qux"'),
+                    ],
+                ),
+                ValueCheck(
+                    name="[1]",
+                    children=[
+                        ValueCheck(name="first", summary='"Foo"'),
+                        ValueCheck(name="second", summary='"Bar"'),
+                    ],
+                ),
+            ],
+        )
+
+        self.expect_var_path(
+            "ref2",
+            summary="size=2",
+            type="StringMapT &",
+            children=[
+                ValueCheck(
+                    name="[0]",
+                    children=[
+                        ValueCheck(name="first", summary='"Baz"'),
+                        ValueCheck(name="second", summary='"Qux"'),
+                    ],
+                ),
+                ValueCheck(
+                    name="[1]",
+                    children=[
+                        ValueCheck(name="first", summary='"Foo"'),
+                        ValueCheck(name="second", summary='"Bar"'),
+                    ],
+                ),
+            ],
+        )
+
+        self.expect_var_path(
+            "ref3",
+            summary="size=2",
+            type="StringMapTRef",
+            children=[
+                ValueCheck(
+                    name="[0]",
+                    children=[
+                        ValueCheck(name="first", summary='"Baz"'),
+                        ValueCheck(name="second", summary='"Qux"'),
+                    ],
+                ),
+                ValueCheck(
+                    name="[1]",
+                    children=[
+                        ValueCheck(name="first", summary='"Foo"'),
+                        ValueCheck(name="second", summary='"Bar"'),
+                    ],
+                ),
+            ],
+        )
+
+        self.expect_var_path(
+            "ref4",
+            summary="size=2",
+            type="const StringMapT &",
+            children=[
+                ValueCheck(
+                    name="[0]",
+                    children=[
+                        ValueCheck(name="first", summary='"Baz"'),
+                        ValueCheck(name="second", summary='"Qux"'),
+                    ],
+                ),
+                ValueCheck(
+                    name="[1]",
+                    children=[
+                        ValueCheck(name="first", summary='"Foo"'),
+                        ValueCheck(name="second", summary='"Bar"'),
+                    ],
+                ),
+            ],
+        )
+
+        self.expect_var_path(
+            "ref5",
+            summary="size=1",
+            type="const StringMapT &&",
+            children=[
+                ValueCheck(
+                    name="[0]",
+                    children=[
+                        ValueCheck(name="first", summary='"Foo"'),
+                        ValueCheck(name="second", summary='"Bar"'),
+                    ],
+                ),
+            ],
+        )
+
+        self.expect_var_path(
+            "ref6",
+            summary="size=1",
+            type="StringMapT &&",
+            children=[
+                ValueCheck(
+                    name="[0]",
+                    children=[
+                        ValueCheck(name="first", summary='"Baz"'),
+                        ValueCheck(name="second", summary='"Qux"'),
+                    ],
+                ),
+            ],
+        )
+
+        # FIXME: we're getting this wrong.
+        self.expect_var_path(
+            "ref7",
+            summary="size=0",
+            type="const StringMapT *const &",
+        )
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/main.cpp
index adcea69629770..b3bfdfbbdd7f7 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/main.cpp
@@ -3,6 +3,14 @@
 #include <unordered_map>
 
 using StringMapT = std::unordered_map<std::string, std::string>;
+using StringMapTRef = const StringMapT &;
+
+void check_references(const StringMapT &ref1, StringMapT &ref2,
+                      StringMapTRef ref3, StringMapTRef &ref4,
+                      const StringMapT &&ref5, StringMapT &&ref6,
+                      const StringMapT *const &ref7) {
+  std::printf("Break here");
+}
 
 int main() {
   StringMapT string_map;
@@ -21,7 +29,12 @@ int main() {
     StringMapT::const_iterator const_baz = string_map.find("Baz");
     auto bucket_it = string_map.begin(string_map.bucket("Baz"));
     auto const_bucket_it = string_map.cbegin(string_map.bucket("Baz"));
+
     std::printf("Break here");
+
+    check_references(string_map, string_map, string_map, string_map,
+                     StringMapT{{"Foo", "Bar"}}, StringMapT{{"Baz", "Qux"}},
+                     &string_map);
   }
 
   return 0;

lldbutil.continue_to_breakpoint(process, bkpt)

# Test references to std::unordered_map
self.expect_var_path(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is just for testing reference resolution, I'd consider making all of the maps have the same content (or even be references to the same object) so that this can be checked with a for loop. (The type will be different, but maybe we don't have to check that since that doesn't come from the data formatter?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. Simplified this in the latest commit. I just passed the "type" as a parameter to a helper function

],
)

# FIXME: we're getting this wrong.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha!

@Michael137 Michael137 merged commit aeea062 into llvm:main Jun 26, 2025
7 checks passed
@Michael137 Michael137 deleted the lldb/const-unordered_map-formatter branch June 26, 2025 16:10
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
…dered_map (llvm#145872)

Desugar any potential references/typedefs before checking
`isStdTemplate`. Previously, the typename might've been:
```
const std::unordered_map<...> &
```
for references. This patch gets the pointee type before grabbing the
canonical type. `GetNonReferenceType` will unwrap typedefs too, so we
should always end up with a non-reference before we get to
`GetCanonicalType`.

llvm#145847
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
…dered_map (llvm#145872)

Desugar any potential references/typedefs before checking
`isStdTemplate`. Previously, the typename might've been:
```
const std::unordered_map<...> &
```
for references. This patch gets the pointee type before grabbing the
canonical type. `GetNonReferenceType` will unwrap typedefs too, so we
should always end up with a non-reference before we get to
`GetCanonicalType`.

llvm#145847
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants