ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.
================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:62
+ SourceLocation location() const { return Location; }
+ SourceLocation endLocation() const {
+ return Location.getLocWithOffset(Length);
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > sadly, this would need documentation - it's the one-past-the-end
> > > location, not the last character in the token, not the location of the
> > > "next" token, or the location of the "next" character...
> > >
> > > I do wonder whether this is actually the right function to expose...
> > > Do you ever care about end but not start? (The reverse seems likelier).
> > > Having two independent source location accessors obscures the invariant
> > > that they have the same file ID.
> > >
> > > I think exposing a `FileRange` accessor instead might be better, but for
> > > now you could also make callers use
> > > `Tok.location().getLocWithOffset(Tok.length())` until we know it's the
> > > right pattern to encourage
> > Added a comment. With the comment and an implementation being inline and
> > trivial, I don't think anyone would have trouble understanding what this
> > method does.
> (this is ok. I do think a FileRange accessor would make client code more
> readable/less error-prone. Let's discuss offline)
I've added a corresponding accessor (and a version of it that accepts a range
of tokens) to D61681.
I'd keep it off this patch for now, will refactor in a follow-up.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59887/new/
https://reviews.llvm.org/D59887
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
