[Lldb-commits] [PATCH] D112658: [lldb] Refactor C/C++ string and char summary providers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for doing this. Just a couple of small remarks.



================
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 {
+
----------------
We don't use [[ https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | anonymous namespaces]] like this. `static` is sufficient.


================
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) {
----------------
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`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112658