[lldb][Formatter] Consolidate libstdc++ and libc++ unique_ptr formatter tests into generic test #147031
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…er tests into generic test The libc++ test was a subset of the tests in libstdc++. This test moves the libc++ test into `generic` and somne additional test-cases from `libstdc++` (specifically the recursive unique_ptr case). It turns out the libstdc++ formatter supports dereferencing using the "object" or "obj" names. We could either drop those from the tests or support the same for libc++. I took the latter approach but don't have strong opinions on this. Split out from llvm#146740
Member
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThe libc++ test was a subset of the tests in libstdc++. This test moves the libc++ test into Split out from #146740 Full diff: https://github.com/llvm/llvm-project/pull/147031.diff 9 Files Affected:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 009586f525282..51e43b2f62a1e 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -399,7 +399,7 @@ lldb_private::formatters::LibcxxUniquePtrSyntheticFrontEnd::
return 0;
if (name == "deleter")
return 1;
- if (name == "$$dereference$$")
+ if (name == "obj" || name == "object" || name == "$$dereference$$")
return 2;
return llvm::createStringError("Type has no child named '%s'",
name.AsCString());
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/Makefile
similarity index 92%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/Makefile
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/Makefile
index c1c8b4a2a0a53..ece665a0fd5b7 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/Makefile
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/Makefile
@@ -1,7 +1,5 @@
CXX_SOURCES := main.cpp
-USE_LIBCPP := 1
-
# We need debug info tuning for lldb in order to emit the preferred name for
# std::string. See https://reviews.llvm.org/D145803.
CXXFLAGS_EXTRAS := -std=c++14 -glldb
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/TestDataFormatterStdUniquePtr.py
similarity index 64%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/TestDataFormatterStdUniquePtr.py
index 25a1cd82a4baa..57e1ea9f234f8 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/TestDataFormatterLibcxxUniquePtr.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/TestDataFormatterStdUniquePtr.py
@@ -1,8 +1,7 @@
"""
-Test lldb data formatter for libc++ std::unique_ptr.
+Test lldb data formatter for std::unique_ptr.
"""
-
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -32,10 +31,8 @@ def make_expected_basic_string_ptr(self) -> str:
"std::default_delete<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > >"
)
- @add_test_categories(["libc++"])
- def test_unique_ptr_variables(self):
+ def do_test(self):
"""Test `frame variable` output for `std::unique_ptr` types."""
- self.build()
lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp")
@@ -121,3 +118,67 @@ def test_unique_ptr_variables(self):
self.expect_var_path("ptr_node->next->value", value="2")
self.expect_var_path("(*ptr_node).value", value="1")
self.expect_var_path("(*(*ptr_node).next).value", value="2")
+
+ @add_test_categories(["libstdcxx"])
+ def test_libstdcxx(self):
+ self.build(dictionary={"USE_LIBSTDCPP": 1})
+ self.do_test()
+
+ @add_test_categories(["libc++"])
+ def test_libcxx(self):
+ self.build(dictionary={"USE_LIBCPP": 1})
+ self.do_test()
+
+ def do_test_recursive_unique_ptr(self):
+ # Tests that LLDB can handle when we have a loop in the unique_ptr
+ # reference chain and that it correctly handles the different options
+ # for the frame variable command in this case.
+ self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+ lldbutil.run_break_set_by_source_regexp(self, "Set break point at this line.")
+ self.runCmd("run", RUN_SUCCEEDED)
+ self.expect(
+ "thread list",
+ STOPPED_DUE_TO_BREAKPOINT,
+ substrs=["stopped", "stop reason = breakpoint"],
+ )
+
+ self.expect("frame variable f1->next", substrs=["next = NodeU @"])
+ self.expect(
+ "frame variable --ptr-depth=1 f1->next",
+ substrs=["next = NodeU @", "value = 2"],
+ )
+ self.expect(
+ "frame variable --ptr-depth=2 f1->next",
+ substrs=["next = NodeU @", "value = 1", "value = 2"],
+ )
+
+ frame = self.frame()
+ self.assertTrue(frame.IsValid())
+ self.assertEqual(
+ 2,
+ frame.GetValueForVariablePath("f1->next.object.value").GetValueAsUnsigned(),
+ )
+ self.assertEqual(
+ 2, frame.GetValueForVariablePath("f1->next->value").GetValueAsUnsigned()
+ )
+ self.assertEqual(
+ 1,
+ frame.GetValueForVariablePath(
+ "f1->next.object.next.obj.value"
+ ).GetValueAsUnsigned(),
+ )
+ self.assertEqual(
+ 1,
+ frame.GetValueForVariablePath("f1->next->next->value").GetValueAsUnsigned(),
+ )
+
+ @add_test_categories(["libstdcxx"])
+ def test_recursive_unique_ptr_libstdcxx(self):
+ self.build(dictionary={"USE_LIBSTDCPP": 1})
+ self.do_test_recursive_unique_ptr()
+
+ @add_test_categories(["libc++"])
+ def test_recursive_unique_ptr_libcxx(self):
+ self.build(dictionary={"USE_LIBCPP": 1})
+ self.do_test_recursive_unique_ptr()
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/main.cpp
similarity index 74%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/main.cpp
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/main.cpp
index afdddf0bbaf16..15e9f70dbd6aa 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unique_ptr/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/main.cpp
@@ -15,11 +15,20 @@ struct NodeU {
// representation when the type of the second element is an empty class. So
// we need a deleter class with a dummy data member to trigger the other path.
struct NonEmptyIntDeleter {
- void operator()(int* ptr) { delete ptr; }
+ void operator()(int *ptr) { delete ptr; }
int dummy_ = 9999;
};
+static void recursive() {
+ // Set up a structure where we have a loop in the unique_ptr chain.
+ NodeU *f1 = new NodeU{nullptr, 1};
+ NodeU *f2 = new NodeU{nullptr, 2};
+ f1->next.reset(f2);
+ f2->next.reset(f1);
+ std::puts("Set break point at this line.");
+}
+
int main() {
std::unique_ptr<int> up_empty;
std::unique_ptr<int> up_int = std::make_unique<int>(10);
@@ -33,5 +42,9 @@ int main() {
std::unique_ptr<NodeU>(new NodeU{nullptr, 2});
ptr_node = std::unique_ptr<NodeU>(new NodeU{std::move(ptr_node), 1});
- return 0; // break here
+ std::puts("// break here");
+
+ recursive();
+
+ return 0;
}
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/Makefile
deleted file mode 100644
index bf8e6b8703f36..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/Makefile
+++ /dev/null
@@ -1,5 +0,0 @@
-CXX_SOURCES := main.cpp
-
-USE_LIBSTDCPP := 1
-
-include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
deleted file mode 100644
index 8f57dc88f3187..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
+++ /dev/null
@@ -1,134 +0,0 @@
-"""
-Test lldb data formatter subsystem.
-"""
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class StdUniquePtrDataFormatterTestCase(TestBase):
- @add_test_categories(["libstdcxx"])
- @expectedFailureAll(bugnumber="llvm.org/pr50861", compiler="gcc")
- @skipIf(oslist=["linux"], archs=["arm$", "aarch64"])
- def test_with_run_command(self):
- self.build()
- self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
-
- lldbutil.run_break_set_by_source_regexp(self, "Set break point at this line.")
- self.runCmd("run", RUN_SUCCEEDED)
-
- # The stop reason of the thread should be breakpoint.
- self.expect(
- "thread list",
- STOPPED_DUE_TO_BREAKPOINT,
- substrs=["stopped", "stop reason = breakpoint"],
- )
-
- frame = self.frame()
- self.assertTrue(frame.IsValid())
-
- self.expect("frame variable nup", substrs=["nup = nullptr"])
- self.expect("frame variable iup", substrs=["iup = 123"])
- self.expect("frame variable sup", substrs=['sup = "foobar"'])
-
- self.expect("frame variable ndp", substrs=["ndp = nullptr"])
- self.expect(
- "frame variable idp", substrs=["idp = 456", "deleter = ", "a = 1", "b = 2"]
- )
- self.expect(
- "frame variable sdp",
- substrs=['sdp = "baz"', "deleter = ", "a = 3", "b = 4"],
- )
-
- self.assertEqual(
- 123, frame.GetValueForVariablePath("iup.object").GetValueAsUnsigned()
- )
- self.assertEqual(
- 123, frame.GetValueForVariablePath("*iup").GetValueAsUnsigned()
- )
- self.assertFalse(frame.GetValueForVariablePath("iup.deleter").IsValid())
-
- self.assertEqual(
- '"foobar"', frame.GetValueForVariablePath("sup.object").GetSummary()
- )
- self.assertEqual('"foobar"', frame.GetValueForVariablePath("*sup").GetSummary())
- self.assertFalse(frame.GetValueForVariablePath("sup.deleter").IsValid())
-
- self.assertEqual(
- 456, frame.GetValueForVariablePath("idp.object").GetValueAsUnsigned()
- )
- self.assertEqual(
- 456, frame.GetValueForVariablePath("*idp").GetValueAsUnsigned()
- )
- self.assertEqual(
- '"baz"', frame.GetValueForVariablePath("sdp.object").GetSummary()
- )
- self.assertEqual('"baz"', frame.GetValueForVariablePath("*sdp").GetSummary())
-
- idp_deleter = frame.GetValueForVariablePath("idp.deleter")
- self.assertTrue(idp_deleter.IsValid())
- self.assertEqual(
- 1, idp_deleter.GetChildMemberWithName("a").GetValueAsUnsigned()
- )
- self.assertEqual(
- 2, idp_deleter.GetChildMemberWithName("b").GetValueAsUnsigned()
- )
-
- sdp_deleter = frame.GetValueForVariablePath("sdp.deleter")
- self.assertTrue(sdp_deleter.IsValid())
- self.assertEqual(
- 3, sdp_deleter.GetChildMemberWithName("a").GetValueAsUnsigned()
- )
- self.assertEqual(
- 4, sdp_deleter.GetChildMemberWithName("b").GetValueAsUnsigned()
- )
-
- @skipIfFreeBSD
- @skipIfWindows # libstdcpp not ported to Windows
- @skipIfDarwin # doesn't compile on Darwin
- @skipIfwatchOS # libstdcpp not ported to watchos
- @skipIf(oslist=["linux"], archs=["arm$", "aarch64"])
- @add_test_categories(["libstdcxx"])
- def test_recursive_unique_ptr(self):
- # Tests that LLDB can handle when we have a loop in the unique_ptr
- # reference chain and that it correctly handles the different options
- # for the frame variable command in this case.
- self.build()
- self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
-
- lldbutil.run_break_set_by_source_regexp(self, "Set break point at this line.")
- self.runCmd("run", RUN_SUCCEEDED)
- self.expect(
- "thread list",
- STOPPED_DUE_TO_BREAKPOINT,
- substrs=["stopped", "stop reason = breakpoint"],
- )
-
- self.expect("frame variable f1->fp", substrs=["fp = Foo @ 0x"])
- self.expect(
- "frame variable --ptr-depth=1 f1->fp", substrs=["data = 2", "fp = Foo @ 0x"]
- )
- self.expect(
- "frame variable --ptr-depth=2 f1->fp",
- substrs=["data = 2", "fp = Foo @ 0x", "data = 1"],
- )
-
- frame = self.frame()
- self.assertTrue(frame.IsValid())
- self.assertEqual(
- 2, frame.GetValueForVariablePath("f1->fp.object.data").GetValueAsUnsigned()
- )
- self.assertEqual(
- 2, frame.GetValueForVariablePath("f1->fp->data").GetValueAsUnsigned()
- )
- self.assertEqual(
- 1,
- frame.GetValueForVariablePath(
- "f1->fp.object.fp.object.data"
- ).GetValueAsUnsigned(),
- )
- self.assertEqual(
- 1, frame.GetValueForVariablePath("f1->fp->fp->data").GetValueAsUnsigned()
- )
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/invalid/TestDataFormatterInvalidStdUniquePtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/invalid/TestDataFormatterInvalidStdUniquePtr.py
deleted file mode 100644
index 9cacfce989769..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/invalid/TestDataFormatterInvalidStdUniquePtr.py
+++ /dev/null
@@ -1,4 +0,0 @@
-import lldbsuite.test.lldbinline as lldbinline
-from lldbsuite.test.decorators import *
-
-lldbinline.MakeInlineTest(__file__, globals(), [no_debug_info_test])
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/invalid/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/invalid/main.cpp
deleted file mode 100644
index b12cab231695b..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/invalid/main.cpp
+++ /dev/null
@@ -1,11 +0,0 @@
-// Test that we don't crash when trying to pretty-print structures that don't
-// have the layout our data formatters expect.
-namespace std {
-template<typename T, typename Deleter = void>
-class unique_ptr {};
-}
-
-int main() {
- std::unique_ptr<int> U;
- return 0; //% self.expect("frame variable U", substrs=["unique_ptr", "{}"])
-}
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/main.cpp
deleted file mode 100644
index dd0072764d4e6..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/main.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-#include <memory>
-#include <string>
-
-struct Deleter {
- void operator()(void *) {}
-
- int a;
- int b;
-};
-
-struct Foo {
- int data;
- std::unique_ptr<Foo> fp;
-};
-
-int main() {
- std::unique_ptr<char> nup;
- std::unique_ptr<int> iup(new int{123});
- std::unique_ptr<std::string> sup(new std::string("foobar"));
-
- std::unique_ptr<char, Deleter> ndp;
- std::unique_ptr<int, Deleter> idp(new int{456}, Deleter{1, 2});
- std::unique_ptr<std::string, Deleter> sdp(new std::string("baz"),
- Deleter{3, 4});
-
- std::unique_ptr<Foo> fp(new Foo{3});
-
- // Set up a structure where we have a loop in the unique_ptr chain.
- Foo* f1 = new Foo{1};
- Foo* f2 = new Foo{2};
- f1->fp.reset(f2);
- f2->fp.reset(f1);
-
- return 0; // Set break point at this line.
-}
|
labath
reviewed
Jul 4, 2025
Collaborator
labath
left a comment
labath
left a comment
There was a problem hiding this comment.
Looks good modulo the failure in the libstdc++ version. The type name comes out different, presumably because libstdc++ doesn't contain the magic to print the template as std::string.
Testing string name printing is not really relevant for the unique_ptr formatter, so I'd delete the check from there. Testing that SomeTemplate<std::string> prints the right way is useful, but would be better of in the test for preferred name printing (if we don't do that already).
Member
Author
Agreed. Removed the typename checks One final difference between the formatters was that the libstdc++ formatter created a synthetic child for the dereferenced object and calls it |
Michael137
commented
Jul 4, 2025
|
|
||
| # We need debug info tuning for lldb in order to emit the preferred name for | ||
| # std::string. See https://reviews.llvm.org/D145803. | ||
| CXXFLAGS_EXTRAS := -std=c++14 -glldb |
Member
Author
There was a problem hiding this comment.
We can probably drop these too now since we don't check the typenames.
Michael137
commented
Jul 4, 2025
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/unique_ptr/Makefile
Outdated
Show resolved
Hide resolved
labath
approved these changes
Jul 7, 2025
Collaborator
I don't think there's any harm in supporting those, though I'd be surprised if many people are aware of that functionality. I kind of like this "hidden child" concept, but it feels like it ought to be just slightly more discoverable. |
Collaborator
|
(The test is failing due to std::make_unique not being defined. I'd be fine both with bringing back -std=c++14 and using explicit construction.) |
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Jul 25, 2025
…er tests into generic test (llvm#147031) The libc++ test was a subset of the tests in libstdc++. This test moves the libc++ test into `generic` and somne additional test-cases from `libstdc++` (specifically the recursive unique_ptr case). It turns out the libstdc++ formatter supports dereferencing using the "object" or "obj" names. We could either drop those from the tests or support the same for libc++. I took the latter approach but don't have strong opinions on this. Split out from llvm#146740 (cherry picked from commit 074ccde)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The libc++ test was a subset of the tests in libstdc++. This test moves the libc++ test into
genericand somne additional test-cases fromlibstdc++(specifically the recursive unique_ptr case). It turns out the libstdc++ formatter supports dereferencing using the "object" or "obj" names. We could either drop those from the tests or support the same for libc++. I took the latter approach but don't have strong opinions on this.Split out from #146740