[Lldb-commits] [PATCH] D97165: [lldb] Add deref support and tests to shared_ptr synthetic

Raphael Isemann via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 22 08:27:47 PST 2021

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

You can use `expect_var_path` to test `frame var` (LLDB calls the frame var input "expressions paths", hence the `_path` suffix). That way you can test all of these things for you without having to rely on just finding substrs in the output or having to depend on the way LLDB prints the result:

  self.expect_var_path("sp_str", summary="\"hello\" strong=1 weak=1",
                       type="std::shared_ptr<std::basic_string<char, std::char_traits<char>, std::allocator<char> > >",
      children = [
          ValueCheck(name="__ptr_", summary="\"hello\"")

Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:11
+class LibcxSharedPtrDataFormatterTestCase(TestBase):
`Libcx` typo. FWIW, you don't have to give these classes a unique name anymore.

Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:17
+    def test_with_run_command(self):
+        """Test that that file and class static variables display correctly."""
+        self.build()
Comment + function name don't really fit to the test

Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:21
+        (self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(self, '// break here',
+                lldb.SBFileSpec("main.cpp", False))
You only need `lldbutil.run_to_source_breakpoint(self, '// break here', lldb.SBFileSpec("main.cpp"))` (the variable assignments and so on aren't necessary)

Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:25
+            substrs=['(std::shared_ptr<int>) sp_empty = nullptr {',
+                               '__ptr_ = 0x0',
+                               '}'])
FWIW, this isn't really testing anything. This just tests that the pointer starts with '0x0', but even non-nullptrs start usually with 0x0 as we pad them with zeroes.

In theory you could test that this is equal to 0x0000000000000000 but then we have the test suite depend on the pointer size.

I think the best solution is to have the test suite figure out what a hex nullptr is on the target and then just provide that as utility variable in lldbtest. But that's out of the scope for this test, so I would say you can just leave it out until someone has time to implement that.

Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp:1
+#include <cstdio>
+#include <memory>
Unused include?

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list