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

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev marked 4 inline comments as done.
fedor.sergeev added inline comments.


================
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:
> chandlerc wrote:
> > fedor.sergeev wrote:
> > > 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.
> > FWIW, I prefer something other than `{"", ""}`. I have a mild preference for {StringRef(), StringRef()}, but it is very mild.
> Fair enough, I don't feel too strongly (if there's any specific motivation for the aversion to {"", ""} I'd be curious to hear it, for reference/understanding/etc).
One particular motivating aspect is that "" creates a special object and that is only to create an empty StringRef which does not need anything than just length == 0.

Asked people on IRC and none of them chose "","" variant ;)
Also, Zygoloid mentioned that { "", "" } could be treated as a call to constructor that takes two iterators (there is none such constructor presently though).

Anyway I will go for the most uncontroversial StringRef() version, despite it being somewhat chatty. I dont mind typing a few extra characters :)


Repository:
  rL LLVM

https://reviews.llvm.org/D54336