Skip to content

Conversation

@Michael137
Copy link
Member

This combines the libc++ and libstdc++ test cases. The libstdcpp tests were a subset of the libc++ test, so this patch moves the libcxx test into generic and removes the libstdcpp test entirely.

Split out from #146740

@Michael137 Michael137 requested a review from labath July 5, 2025 11:24
@Michael137 Michael137 requested a review from JDevlieghere as a code owner July 5, 2025 11:24
@llvmbot llvmbot added the lldb label Jul 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 5, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This combines the libc++ and libstdc++ test cases. The libstdcpp tests were a subset of the libc++ test, so this patch moves the libcxx test into generic and removes the libstdcpp test entirely.

Split out from #146740


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

6 Files Affected:

  • (renamed) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/Makefile (-3)
  • (renamed) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py (+12-13)
  • (renamed) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/main.cpp ()
  • (removed) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile (-8)
  • (removed) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py (-52)
  • (removed) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp (-26)
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/Makefile
similarity index 57%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/Makefile
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/Makefile
index 654e4b15bd568..99998b20bcb05 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/Makefile
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/Makefile
@@ -1,6 +1,3 @@
 CXX_SOURCES := main.cpp
 
-CXXFLAGS := -O0
-USE_LIBSTDCPP := 1
-
 include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py
similarity index 89%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py
index 25f616ff61046..3f73f157b3a0e 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/TestDataFormatterStdSharedPtr.py
@@ -1,5 +1,5 @@
 """
-Test lldb data formatter for libc++ std::shared_ptr.
+Test lldb data formatter for std::shared_ptr.
 """
 
 import lldb
@@ -9,11 +9,8 @@
 
 
 class TestCase(TestBase):
-    @add_test_categories(["libc++"])
-    def test_shared_ptr_variables(self):
+    def do_test(self):
         """Test `frame variable` output for `std::shared_ptr` types."""
-        self.build()
-
         (_, process, _, bkpt) = lldbutil.run_to_source_breakpoint(
             self, "// break here", lldb.SBFileSpec("main.cpp")
         )
@@ -56,16 +53,8 @@ def test_shared_ptr_variables(self):
         self.assertRegex(valobj.summary, r"^10( strong=1)? weak=0$")
         self.assertNotEqual(valobj.child[0].unsigned, 0)
 
-        if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(
-            [">", "16.0"]
-        ):
-            string_type = "std::string"
-        else:
-            string_type = "std::basic_string<char, std::char_traits<char>, std::allocator<char> > "
-
         valobj = self.expect_var_path(
             "sp_str",
-            type="std::shared_ptr<" + string_type + ">",
             children=[ValueCheck(name="__ptr_", summary='"hello"')],
         )
         self.assertRegex(valobj.summary, r'^"hello"( strong=1)? weak=0$')
@@ -115,3 +104,13 @@ def test_shared_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(["libc++"])
+    def test_libcxx(self):
+        self.build(dictionary={"USE_LIBCPP": 1})
+        self.do_test()
+
+    @add_test_categories(["libstdcxx"])
+    def test_libstdcxx(self):
+        self.build(dictionary={"USE_LIBSTDCPP": 1})
+        self.do_test()
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/main.cpp
similarity index 100%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/shared_ptr/main.cpp
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
deleted file mode 100644
index c1c8b4a2a0a53..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/Makefile
+++ /dev/null
@@ -1,8 +0,0 @@
-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
-include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
deleted file mode 100644
index a6856177858a3..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
+++ /dev/null
@@ -1,52 +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 StdSmartPtrDataFormatterTestCase(TestBase):
-    @add_test_categories(["libstdcxx"])
-    @expectedFailureAll(bugnumber="llvm.org/pr50861", compiler="gcc")
-    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"],
-        )
-
-        self.expect("frame variable nsp", substrs=["nsp = nullptr"])
-        self.expect("frame variable isp", substrs=["isp = 123"])
-        self.expect("frame variable ssp", substrs=['ssp = "foobar"'])
-
-        self.expect("frame variable nwp", substrs=["nwp = nullptr"])
-        self.expect("frame variable iwp", substrs=["iwp = 123"])
-        self.expect("frame variable swp", substrs=['swp = "foobar"'])
-
-        self.expect("frame variable *nsp", substrs=["*nsp = <parent is NULL>"])
-        self.expect("frame variable *isp", substrs=["*isp = 123"])
-        self.expect("frame variable *ssp", substrs=['*ssp = "foobar"'])
-        self.expect("frame variable *fsp", substrs=["*fsp = (mem = 5)"])
-
-        self.expect("frame variable fsp->mem", substrs=["(int) fsp->mem = 5"])
-
-        self.runCmd("continue")
-
-        self.expect("frame variable nsp", substrs=["nsp = nullptr"])
-        self.expect("frame variable isp", substrs=["isp = nullptr"])
-        self.expect("frame variable ssp", substrs=["ssp = nullptr"])
-
-        self.expect("frame variable nwp", substrs=["nwp = nullptr"])
-        self.expect("frame variable iwp", substrs=["iwp = nullptr"])
-        self.expect("frame variable swp", substrs=["swp = nullptr"])
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
deleted file mode 100644
index 6e4b869e1c8e0..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-#include <memory>
-#include <string>
-
-struct Foo {
-  int mem = 5;
-};
-
-int
-main()
-{
-    std::shared_ptr<char> nsp;
-    std::shared_ptr<int> isp(new int{123});
-    std::shared_ptr<std::string> ssp = std::make_shared<std::string>("foobar");
-    std::shared_ptr<Foo> fsp = std::make_shared<Foo>();
-
-    std::weak_ptr<char> nwp;
-    std::weak_ptr<int> iwp = isp;
-    std::weak_ptr<std::string> swp = ssp;
-
-    nsp.reset(); // Set break point at this line.
-    isp.reset();
-    ssp.reset();
-    fsp.reset();
-
-    return 0; // Set break point at this line.
-}

@labath
Copy link
Collaborator

labath commented Jul 7, 2025

There are two failures:

  • the libc++ pointer child is called __ptr_, not pointer. I think we should clone/rename the child so that the child has a consistent name. This could be particularly useful for writing other data formatters, which may want to dereference a shared_ptr without caring which stdlib implementation they are using.
  • the libstdc++ child type is std::goo<...>::type instead of int in libc++. I think the libc++ implementation is better. The case for unification is slightly weaker here (people can always resolve the typedef manually), though I'd still do it if its easy enough.

@Michael137
Copy link
Member Author

Michael137 commented Jul 7, 2025

There are two failures:

* the libc++ pointer child is called `__ptr_`, not `pointer`. I think we should clone/rename the child so that the child has a consistent name. This could be particularly useful for writing other data formatters, which may want to dereference a shared_ptr without caring which stdlib implementation they are using.

* the libstdc++ child type is `std::goo<...>::type` instead of `int` in libc++. I think the libc++ implementation is better. The case for unification is slightly weaker here (people can always resolve the typedef manually), though I'd still do it if its easy enough.

Yup spot on! I addressed all the discrepancies in #147166 and #147165

Michael137 added a commit that referenced this pull request Jul 7, 2025
…rs consistent with each other (#147165)

This patch adjusts the libcxx and libstdcxx std::shared_ptr formatters
to look the same.

Changes to libcxx:
* Now creates a synthetic child called `pointer` (like we already do for
`std::unique_ptr`)

Changes to libstdcxx:
* When asked to dereference the pointer, cast the type of the result
ValueObject to the element type (which we get from the template argument
to std::shared_ptr).
Before:
```
(std::__shared_ptr<int, __gnu_cxx::_S_atomic>::element_type) *foo = 123
```
After:
```
(int) *foo = 123
```

Tested in #147141
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 7, 2025
…tr formatters consistent with each other (#147165)

This patch adjusts the libcxx and libstdcxx std::shared_ptr formatters
to look the same.

Changes to libcxx:
* Now creates a synthetic child called `pointer` (like we already do for
`std::unique_ptr`)

Changes to libstdcxx:
* When asked to dereference the pointer, cast the type of the result
ValueObject to the element type (which we get from the template argument
to std::shared_ptr).
Before:
```
(std::__shared_ptr<int, __gnu_cxx::_S_atomic>::element_type) *foo = 123
```
After:
```
(int) *foo = 123
```

Tested in llvm/llvm-project#147141
Michael137 added a commit that referenced this pull request Jul 7, 2025
… summary (#147166)

Depends on #147165

This adds weak/strong counts to the std::shared_ptr summary of the
libstdcxx formatters. We already do this for libcxx. This will make it
easier to consolidate the tests into a generic one (see
#147141).
…generic test

This combines the libc++ and libstdc++ test cases.
The libstdcpp tests were a subset of the libc++ test, so this patch
moves the libcxx test into `generic` and removes the libstdcpp test
entirely.

Split out from llvm#146740
@Michael137 Michael137 force-pushed the lldb/consolidate-stl-tests-shared_ptr branch from c5380d0 to f14ec44 Compare July 7, 2025 11:18
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 7, 2025
…:shared_ptr summary (#147166)

Depends on llvm/llvm-project#147165

This adds weak/strong counts to the std::shared_ptr summary of the
libstdcxx formatters. We already do this for libcxx. This will make it
easier to consolidate the tests into a generic one (see
llvm/llvm-project#147141).
@Michael137 Michael137 merged commit e14e982 into llvm:main Jul 7, 2025
9 checks passed
@Michael137 Michael137 deleted the lldb/consolidate-stl-tests-shared_ptr branch July 7, 2025 11:48
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2025
…rs consistent with each other (llvm#147165)

This patch adjusts the libcxx and libstdcxx std::shared_ptr formatters
to look the same.

Changes to libcxx:
* Now creates a synthetic child called `pointer` (like we already do for
`std::unique_ptr`)

Changes to libstdcxx:
* When asked to dereference the pointer, cast the type of the result
ValueObject to the element type (which we get from the template argument
to std::shared_ptr).
Before:
```
(std::__shared_ptr<int, __gnu_cxx::_S_atomic>::element_type) *foo = 123
```
After:
```
(int) *foo = 123
```

Tested in llvm#147141

(cherry picked from commit 6fec6a9)
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2025
… summary (llvm#147166)

Depends on llvm#147165

This adds weak/strong counts to the std::shared_ptr summary of the
libstdcxx formatters. We already do this for libcxx. This will make it
easier to consolidate the tests into a generic one (see
llvm#147141).

(cherry picked from commit 40fb90e)
Michael137 added a commit to swiftlang/llvm-project that referenced this pull request Jul 25, 2025
…generic test (llvm#147141)

This combines the libc++ and libstdc++ test cases. The libstdcpp tests
were a subset of the libc++ test, so this patch moves the libcxx test
into `generic` and removes the libstdcpp test entirely.

Split out from llvm#146740

(cherry picked from commit e14e982)
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