[PATCH] D80301: [yaml][clang-tidy] Fix new line YAML serialization

Dmitry Polukhin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 5 04:22:48 PDT 2020


DmitryPolukhin added inline comments.


================
Comment at: llvm/lib/Support/YAMLTraits.cpp:894
+                                           std::string &Val) {
+  Val.clear();
+  size_t CurrentPos = 0;
----------------
njames93 wrote:
> If you want to do the same here...
> ```
>   SmallVector<StringRef, 8> Lines;
>   Scalar.split(Lines, "\n\n");
>   Val = llvm::join(Lines, "\n");
>   return StringRef();```
Personally I don't like approach with string split because it is potentially less efficient due to memory allocations in case of multiple newlines that can be avoided. BUT I would like to move the needle and fix this real bug in clang ASAP. So I'm ready to rewrite it to something that (1) works and (2) moves review forward. So thank you for review and please take another look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80301





More information about the cfe-commits mailing list