[lldb] Refactor C/C++ string and char summary providers

[ljmf00]

Summary

Signed-off-by: Luís Ferreira <[e-mail]>

This patch refactors C/C++ formatters to avoid repetitive code by using templates.

[labath]

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

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

35

We don't use anonymous namespaces like this. static is sufficient.

76–85

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.

[ljmf00]

In D112658#3092586, @labath wrote:

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

Can you re-review?

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

35

Done

76–85

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.

[labath]

Looks good.

Do you have commit access, or should I check this in for you (I can also fix up the formatting in that case)?

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

36–39

This isn't the right formatting (brackets don't get their own line, and the first line is too long). Could you run the patch through clang-format please?

76–85

I'd probably spell out the type (and it looks like you already did).

[labath]

In D112658#3095656, @labath wrote:

Do you have commit access, or should I check this in for you (I can also fix up the formatting in that case)?

Raphael tells me you don't, so I've committed this as rG5e31601.

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

 #include "lldb/Utility/Stream.h"
 
 #include <algorithm>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
-bool lldb_private::formatters::Char8StringSummaryProvider(
-    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+namespace {
+
+  template<StringPrinter::StringElementType ElemType>
+  static bool CharStringSummaryProvider(ValueObject &valobj, Stream &stream) {
     ProcessSP process_sp = valobj.GetProcessSP();
     if (!process_sp)
       return false;
 
     lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
     if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
       return false;
 
     StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
     options.SetLocation(valobj_addr);
     options.SetProcessSP(process_sp);
     options.SetStream(&stream);
+    if (ElemType == StringPrinter::StringElementType::UTF8)
       options.SetPrefixToken("u8");
+    else if (ElemType == StringPrinter::StringElementType::UTF16)
+      options.SetPrefixToken("u");
+    else if (ElemType == StringPrinter::StringElementType::UTF32)
+      options.SetPrefixToken("U");
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-          StringPrinter::StringElementType::UTF8>(options)) {
+    if (!StringPrinter::ReadStringAndDumpToStream<ElemType>(options))
       stream.Printf("Summary Unavailable");
-    return true;
-  }
 
     return true;
   }
 
-bool lldb_private::formatters::Char16StringSummaryProvider(
-    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-    return false;
+  template<StringPrinter::StringElementType ElemType>
+  static bool CharSummaryProvider(ValueObject &valobj, Stream &stream) {
+    DataExtractor data;
+    Status error;
+    valobj.GetData(data, error);
 
-  lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
-  if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
+    if (error.Fail())
       return false;
 
-  StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
-  options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
-  options.SetStream(&stream);
-  options.SetPrefixToken("u");
+    std::string value;
+    StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-          StringPrinter::StringElementType::UTF16>(options)) {
-    stream.Printf("Summary Unavailable");
-    return true;
+    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) {
+      options.SetPrefixToken("U");
+      valobj.GetValueAsCString(lldb::eFormatUnicode32, value);
     }
 
-  return true;
+    if (!value.empty())
+      stream.Printf("%s ", value.c_str());
+
+    options.SetData(std::move(data));
+    options.SetStream(&stream);
+    options.SetQuote('\'');
+    options.SetSourceSize(1);
+    options.SetBinaryZeroIsTerminator(false);
+
+    return StringPrinter::ReadBufferAndDumpToStream<ElemType>(options);
   }
 
-bool lldb_private::formatters::Char32StringSummaryProvider(
-    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-    return false;
+} // namespace
 
-  lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
-  if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
-    return false;
 
-  StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
-  options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
-  options.SetStream(&stream);
-  options.SetPrefixToken("U");
+bool lldb_private::formatters::Char8StringSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+  return CharStringSummaryProvider<StringPrinter::StringElementType::UTF8>(valobj, stream);
+}
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-          StringPrinter::StringElementType::UTF32>(options)) {
-    stream.Printf("Summary Unavailable");
-    return true;
-  }
+bool lldb_private::formatters::Char16StringSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+  return CharStringSummaryProvider<StringPrinter::StringElementType::UTF16>(valobj, stream);
 
-  return true;
+bool lldb_private::formatters::Char32StringSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+  return CharStringSummaryProvider<StringPrinter::StringElementType::UTF32>(valobj, stream);
 }
 
 bool lldb_private::formatters::WCharStringSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
   ProcessSP process_sp = valobj.GetProcessSP();
   if (!process_sp)
     return false;
 
default:
     stream.Printf("size for wchar_t is not valid");
     return true;
   }
   return true;
 }
 
 bool lldb_private::formatters::Char8SummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  DataExtractor data;
-  Status error;
-  valobj.GetData(data, error);
-
-  if (error.Fail())
-    return false;
-
-  std::string value;
-  valobj.GetValueAsCString(lldb::eFormatUnicode8, value);
-  if (!value.empty())
-    stream.Printf("%s ", value.c_str());
-
-  StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
-  options.SetData(std::move(data));
-  options.SetStream(&stream);
-  options.SetPrefixToken("u8");
-  options.SetQuote('\'');
-  options.SetSourceSize(1);
-  options.SetBinaryZeroIsTerminator(false);
-
-  return StringPrinter::ReadBufferAndDumpToStream<
-      StringPrinter::StringElementType::UTF8>(options);
+  return CharSummaryProvider<StringPrinter::StringElementType::UTF8>(valobj, stream);
 }
 
 bool lldb_private::formatters::Char16SummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  DataExtractor data;
-  Status error;
-  valobj.GetData(data, error);
-
-  if (error.Fail())
-    return false;
-
-  std::string value;
-  valobj.GetValueAsCString(lldb::eFormatUnicode16, value);
-  if (!value.empty())
-    stream.Printf("%s ", value.c_str());
-
-  StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
-  options.SetData(std::move(data));
-  options.SetStream(&stream);
-  options.SetPrefixToken("u");
-  options.SetQuote('\'');
-  options.SetSourceSize(1);
-  options.SetBinaryZeroIsTerminator(false);
-
-  return StringPrinter::ReadBufferAndDumpToStream<
-      StringPrinter::StringElementType::UTF16>(options);
+  return CharSummaryProvider<StringPrinter::StringElementType::UTF16>(valobj, stream);
 }
 
 bool lldb_private::formatters::Char32SummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  DataExtractor data;
-  Status error;
-  valobj.GetData(data, error);
-
-  if (error.Fail())
-    return false;
-
-  std::string value;
-  valobj.GetValueAsCString(lldb::eFormatUnicode32, value);
-  if (!value.empty())
-    stream.Printf("%s ", value.c_str());
-
-  StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
-  options.SetData(std::move(data));
-  options.SetStream(&stream);
-  options.SetPrefixToken("U");
-  options.SetQuote('\'');
-  options.SetSourceSize(1);
-  options.SetBinaryZeroIsTerminator(false);
-
-  return StringPrinter::ReadBufferAndDumpToStream<
-      StringPrinter::StringElementType::UTF32>(options);
+  return CharSummaryProvider<StringPrinter::StringElementType::UTF32>(valobj, stream);
 }
 
 bool lldb_private::formatters::WCharSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
   DataExtractor data;
   Status error;
   valobj.GetData(data, error);