JDevlieghere added inline comments.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:474
// consume
static bool ExtractLibcxxStringInfo(ValueObject &valobj,
ValueObjectSP &location_sp,
----------------
Definitely something for another patch, but this is really bad, looking at the
signature I have no idea what's in or output. This should return an
optional<struct/pair/tuple>.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:611
case 1:
- StringPrinter::ReadBufferAndDumpToStream<
+ return StringPrinter::ReadBufferAndDumpToStream<
lldb_private::formatters::StringPrinter::StringElementType::UTF8>(
----------------
This is so much better.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:656
}
- location_sp->GetPointeeData(extractor, 0, size);
+ const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size);
+ if (bytes_read < size)
----------------
Can we move the `extractor` closer to where it's used, so basically after the
capping check?
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:663
if (prefix_token.empty())
options.SetPrefixToken(nullptr);
----------------
Not code you touched, but...
```
options.SetPrefixToken(prefix_token.empty() ? nullptr : prefix_token);
```
might prevent this visual break in setting the options.
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:679
+ StreamString scratch_stream;
+ bool success = LibcxxStringSummaryProvider<element_type>(
+ valobj, scratch_stream, summary_options, prefix_token);
----------------
`const bool`
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
