[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

55

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

85–94

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

55

Done

85–94

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

37–40

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?

85–94

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 &) {
+using StringElementType = StringPrinter::StringElementType;
+
+static constexpr std::pair<const char *, Format>
+getElementTraits(StringElementType ElemType) {
+  switch (ElemType) {
+  case StringElementType::UTF8:
+    return std::make_pair("u8", lldb::eFormatUnicode8);
+  case StringElementType::UTF16:
+    return std::make_pair("u", lldb::eFormatUnicode16);
+  case StringElementType::UTF32:
+    return std::make_pair("U", lldb::eFormatUnicode32);
+  default:
+    return std::make_pair(nullptr, lldb::eFormatInvalid);
+  }
+}
+
+template <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);
-  options.SetPrefixToken("u8");
+  options.SetPrefixToken(getElementTraits(ElemType).first);
 
-  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 <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);
+  std::string value;
+  StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
+
+  constexpr auto ElemTraits = getElementTraits(ElemType);
+  valobj.GetValueAsCString(ElemTraits.second, value);
+
+  if (!value.empty())
+    stream.Printf("%s ", value.c_str());
+
+  options.SetData(std::move(data));
   options.SetStream(&stream);
-  options.SetPrefixToken("u");
+  options.SetPrefixToken(ElemTraits.first);
+  options.SetQuote('\'');
+  options.SetSourceSize(1);
+  options.SetBinaryZeroIsTerminator(false);
 
-  if (!StringPrinter::ReadStringAndDumpToStream<
-          StringPrinter::StringElementType::UTF16>(options)) {
-    stream.Printf("Summary Unavailable");
-    return true;
-  }
+  return StringPrinter::ReadBufferAndDumpToStream<ElemType>(options);
 
-  return true;
+bool lldb_private::formatters::Char8StringSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+  return CharStringSummaryProvider<StringElementType::UTF8>(valobj, stream);
 }
 
-bool lldb_private::formatters::Char32StringSummaryProvider(
+bool lldb_private::formatters::Char16StringSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  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);
-  options.SetPrefixToken("U");
-
-  if (!StringPrinter::ReadStringAndDumpToStream<
-          StringPrinter::StringElementType::UTF32>(options)) {
-    stream.Printf("Summary Unavailable");
-    return true;
-  }
+  return CharStringSummaryProvider<StringElementType::UTF16>(valobj, stream);
 
-  return true;
+bool lldb_private::formatters::Char32StringSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
+  return CharStringSummaryProvider<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;
 
bool lldb_private::formatters::WCharStringSummaryProvider(
   StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
   options.SetLocation(valobj_addr);
   options.SetProcessSP(process_sp);
   options.SetStream(&stream);
   options.SetPrefixToken("L");
 
   switch (wchar_size) {
   case 8:
-    return StringPrinter::ReadStringAndDumpToStream<
-        StringPrinter::StringElementType::UTF8>(options);
+    return StringPrinter::ReadStringAndDumpToStream<StringElementType::UTF8>(
+        options);
   case 16:
-    return StringPrinter::ReadStringAndDumpToStream<
-        StringPrinter::StringElementType::UTF16>(options);
+    return StringPrinter::ReadStringAndDumpToStream<StringElementType::UTF16>(
+        options);
   case 32:
-    return StringPrinter::ReadStringAndDumpToStream<
-        StringPrinter::StringElementType::UTF32>(options);
+    return StringPrinter::ReadStringAndDumpToStream<StringElementType::UTF32>(
+        options);
   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<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<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<StringElementType::UTF32>(valobj, stream);
 }
 
 bool lldb_private::formatters::WCharSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
   DataExtractor data;
   Status error;
   valobj.GetData(data, error);
 
bool lldb_private::formatters::WCharSummaryProvider(
   options.SetStream(&stream);
   options.SetPrefixToken("L");
   options.SetQuote('\'');
   options.SetSourceSize(1);
   options.SetBinaryZeroIsTerminator(false);
 
   switch (wchar_size) {
   case 8:
-    return StringPrinter::ReadBufferAndDumpToStream<
-        StringPrinter::StringElementType::UTF8>(options);
+    return StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF8>(
+        options);
   case 16:
-    return StringPrinter::ReadBufferAndDumpToStream<
-        StringPrinter::StringElementType::UTF16>(options);
+    return StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF16>(
+        options);
   case 32:
-    return StringPrinter::ReadBufferAndDumpToStream<
-        StringPrinter::StringElementType::UTF32>(options);
+    return StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF32>(
+        options);
   default:
     stream.Printf("size for wchar_t is not valid");
     return true;
   }
   return true;
 }