kastiglione added inline comments.
================
Comment at:
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:11
+
+class LibcxSharedPtrDataFormatterTestCase(TestBase):
+
----------------
teemperor wrote:
> `Libcx` typo. FWIW, you don't have to give these classes a unique name
> anymore.
also copy & paste. What name should I give it, if not unique?
================
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()
----------------
teemperor wrote:
> Comment + function name don't really fit to the test
ha, this is copypasta from the test for unique_ptr, which also contains this.
================
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))
+
----------------
teemperor wrote:
> You only need `lldbutil.run_to_source_breakpoint(self, '// break here',
> lldb.SBFileSpec("main.cpp"))` (the variable assignments and so on aren't
> necessary)
more copy, thanks
================
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',
+ '}'])
----------------
teemperor wrote:
> 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.
I could use a regex for now.
================
Comment at:
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/main.cpp:1
+#include <cstdio>
+#include <memory>
----------------
teemperor wrote:
> Unused include?
pasta, thanks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97165/new/
https://reviews.llvm.org/D97165
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
