[PATCH] D56415: NFC: Port QueryParser to StringRef

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 8 08:05:11 PST 2019


aaron.ballman added inline comments.


================
Comment at: clang-query/QueryParser.cpp:36
+  if (Line.front() == '#') {
+    Line = {};
     return StringRef();
----------------
steveire wrote:
> kristina wrote:
> > I don't think this is the best way of handling it, in fact I'm pretty certain you're leaking memory here.
> Hmm, I didn't know we could get a mem leak with StringRef. Can you explain?
There's no memory leak here that I can see -- `StringRef` is non-owning.


================
Comment at: clang-query/QueryParser.cpp:33
+  if (Line.empty())
+    return StringRef(Line.begin(), 0);
 
----------------
Why not just return `Line`?


================
Comment at: clang-query/QueryParser.cpp:37
+    Line = {};
     return StringRef();
   }
----------------
Why not just return `Line` here as well?


================
Comment at: clang-query/QueryParser.cpp:40
 
-  const char *WordBegin = Begin;
-
-  while (true) {
-    ++Begin;
-
-    if (Begin == End || isWhitespace(*Begin))
-      return StringRef(WordBegin, Begin - WordBegin);
-  }
+  StringRef Word = Line.take_while([](char c) { return !isWhitespace(c); });
+  Line = Line.drop_front(Word.size());
----------------
Can you use `Line.take_until(isWhitespace);`?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56415





More information about the cfe-commits mailing list