shafik added inline comments.

================
Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp:23
+};
+static struct {
+  uint64_t cap = 5;
----------------
This is much nicer then the previous raw arrays and self-documents with the 
fields, nice!


================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:144
+    return retval;
+  if (!llvm::checkedAdd(reinterpret_cast<intptr_t>(buffer),
+                        static_cast<intptr_t>(utf8_encoded_len)))
----------------
Wouldn't we want `checkedAddUnsigned`? This would also mean casting to 
`uintptr_t`.


================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:474
       uint8_t *next_data = nullptr;
       auto printable = escaping_callback(data, data_end, next_data);
       auto printable_bytes = printable.GetBytes();
----------------
Not your code but these `auto` seem unnecessary.  Especially `printable` is 
that just a `bool`?  Same comment in similar code above.


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:530
+
+    // When the small-string optimization takes place, the data must fit in the
+    // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
----------------
So `short_mode` means SSO? 


================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:555
+    size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+    const uint64_t cap = cap_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+    if (size == LLDB_INVALID_OFFSET || cap == LLDB_INVALID_OFFSET || cap < 
size)
----------------
What does `cap` represent? It is not obvious in this context.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73860/new/

https://reviews.llvm.org/D73860



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to