[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