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