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

[Vedant Kumar via Phabricator via lldb-commits]

vsk marked 4 inline comments as done.
vsk added inline comments.


================
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 {
----------------
shafik wrote:
> 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. 
Sure, these can be folded together.


================
Comment at: lldb/source/DataFormatters/StringPrinter.cpp:554
+template <>
+bool StringPrinter::ReadBufferAndDumpToStream<StringElementType::UTF8>(
+    const ReadBufferAndDumpToStreamOptions &options) {
----------------
shafik wrote:
> 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.
No, because the ASCII specialization calls the UTF8 specialization, and the compiler complains that the use occurs before the definition. I'll just reorder the specializations of ReadStringAndDumpToStream so that things match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843