[Lldb-commits] [PATCH] D25726: Improve the libstdc++ smart pointer formatters

[Pavel Labath via lldb-commits]

labath added a comment.

Looks good from my side. Enrico, do you want to have a look at this?

A couple tiny comments below. Also, be sure to properly reformat and reorder the headers.



================
Comment at: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py:41
+        self.expect("frame variable ssp.count", substrs=['count = 1'])
+        self.expect("frame variable ssp.weak_count", substrs=['weak_count = 1'])
 
----------------
Could you add a test that accesses a non-existing subobject of `ssp`, to make sure it does something not-completely-broken.


================
Comment at: source/Plugins/Language/CPlusPlus/LibStdcppSmartPointer.cpp:27
+
+class LibStdcppSharedPtrSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+ public:
----------------
What do you think about renaming this class to something shorter (SharedPtrFrontEnd, SharedPtrSynthetic, simply FrontEnd ?). Since it's local to a cpp, I don't think we need to be so explicit, and it seems wasteful to use half of the line length for the class name.


================
Comment at: source/Plugins/Language/CPlusPlus/LibStdcppSmartPointer.cpp:89
+    bool success = false;
+    uint64_t count = m_weak_obj->GetValueAsUnsigned(0, &success) - 1;
+    if (success) {
----------------
It would be useful to add a short comment explaining the `-1`


================
Comment at: source/Plugins/Language/CPlusPlus/LibStdcppSmartPointer.cpp:138
+    return 3;
+  return UINT32_MAX;
+}
----------------
I know you just copied it, but this seems wrong (size_t can be 64-bit). Will this work if you just return `~0`.


https://reviews.llvm.org/D25726