[PATCH] D54336: [FileCheck] introduce CHECK-COUNT-<num> repetition directive

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added inline comments.


================
Comment at: lib/Support/FileCheck.cpp:586-588
+  if (NextChar == ':') {
+    return {Check::CheckPlain, Rest};
+  }
----------------
dblaikie wrote:
> Bit inconsistent (with LLVM style, and within this file specifically) to have {} on this single-line block (& another further down (consumeInteger))?
This one - you'r right.
consumeInteger - I wonder, I do have one comment and one statement there, does it count as a single-line block?




================
Comment at: lib/Support/FileCheck.cpp:707
   // We ran out of buffer while skipping partial matches so give up.
-  return StringRef();
+  return {StringRef(), StringRef()};
 }
----------------
dblaikie wrote:
> Does this work as {"", ""}? or does that do the wrong thing/not compile?
Direct shorter equivalent would be 
 { {}, {} }
And this one:
 { "", "" }
is a tad different, by providing a non-null data pointer (to empty constant string).
For the purpose of this change all these are the same.

As you see, originally there was just StringRef(), thats why I chose my version.
I dont feel strongly about either of these versions.


Repository:
  rL LLVM

https://reviews.llvm.org/D54336