teemperor added inline comments.

================
Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/shared_ptr/TestDataFormatterLibcxxSharedPtr.py:11
+
+class LibcxSharedPtrDataFormatterTestCase(TestBase):
+
----------------
kastiglione wrote:
> 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?
You can just call it `TestCase`, but you can also keep a unique name. It 
doesn't really matter. I personally use `TestCase` because repeating the file 
name inside a file is redundant, but we used to give all tests unique class 
names, so feel free to carry on that tradition.


================
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',
+                               '}'])
----------------
kastiglione wrote:
> 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.
That would also work in the meantime.


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

Reply via email to