ljmf00 added a comment.
In D112658#3092586 <https://reviews.llvm.org/D112658#3092586>, @labath wrote:
> Thanks for doing this. Just a couple of small remarks.
Can you re-review?
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:35
-bool lldb_private::formatters::Char8StringSummaryProvider(
- ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
- ProcessSP process_sp = valobj.GetProcessSP();
- if (!process_sp)
- return false;
+namespace {
+
----------------
labath wrote:
> We don't use [[
> https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | anonymous
> namespaces]] like this. `static` is sufficient.
Done
================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp:76-85
+ if (ElemType == StringPrinter::StringElementType::UTF8) {
+ options.SetPrefixToken("u8");
+ valobj.GetValueAsCString(lldb::eFormatUnicode8, value);
+ } else if (ElemType == StringPrinter::StringElementType::UTF16) {
+ options.SetPrefixToken("u");
+ valobj.GetValueAsCString(lldb::eFormatUnicode16, value);
+ } else if (ElemType == StringPrinter::StringElementType::UTF32) {
----------------
labath wrote:
> Maybe a helper function like `pair<StringRef, Format>
> getElementTraits(StringElementType)` would reduce the repetition further? Or
> possibly it could be a static array indexed by `ElemType`.
Done. I used `constexpr auto`, I'm not sure if `auto` is a practice here in the
LLVM codebase or I should explicitly specify the pair type.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112658/new/
https://reviews.llvm.org/D112658
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
