[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

Shafik Yaghmour via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri May 1 17:17:32 PDT 2020

shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM with minor comments, this is a big set of changes, I would prefer if one of the other reviewers also gave it a second look.

Comment at: lldb/source/DataFormatters/StringPrinter.cpp:49
+  const uint8_t *GetBytes() const { return &m_data[0]; }
just `m_data` it will decay to a pointer. 

Comment at: lldb/source/DataFormatters/StringPrinter.cpp:268
   case GetPrintableElementType::UTF8:
-    return [](uint8_t *buffer, uint8_t *buffer_end,
-              uint8_t *&next) -> StringPrinter::StringPrinterBufferPointer {
-      return GetPrintable(StringPrinter::StringElementType::UTF8, buffer,
-                          buffer_end, next);
+    return [escape_style](uint8_t *buffer, uint8_t *buffer_end,
+                          uint8_t *&next) -> DecodedCharBuffer {
It feels like the two lambdas here could be one by also capturing `elem_type`, it would make the lambda larger but is that a concern? Not a big deal just feels like needlessly duplicate code. 

Comment at: lldb/source/DataFormatters/StringPrinter.cpp:554
+template <>
+bool StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF8>(
+    const ReadBufferAndDumpToStreamOptions &options) {
Super nitpick, can we switch order of the `UTF8` and the `ASCII` specialization so that they appear in the same order as the `ReadStringAndDumpToStream` above.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list