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

Matthias Gehre via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 2 11:32:27 PDT 2020


mgehre added inline comments.


================
Comment at: llvm/lib/Support/YAMLTraits.cpp:904
+                                           std::string &Val) {
+  Val.clear();
+  size_t CurrentPos = 0;
----------------
DmitryPolukhin wrote:
> mgehre wrote:
> > I wonder whether using StringRef::split() would lead to an easier implementation (https://llvm.org/doxygen/classllvm_1_1StringRef.html#af0284e4c41c0e09c0bc4767bc77a899d)
> I'm not sure that it will be easier to read or more efficient (`StringRef::split` will require additional vector).
I don't expect the difference in efficiency to be noticable, but the code would look like
```
SmallVector<StringRef, 2> Lines;
StringRef(Val).split(Lines, '\n');
bool First = true;
for (StringRef Line: Lines) {
  if (First)
    First = false;
  else
    Out << '\n';
  Out << Line;
}
```
which I personally find easier to follow than `CurrentPos`, `LineBreakPos` and their arithmetic.


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