[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 12 03:21:01 PST 2019
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:261
+ bool AcceptRight = Right != All.end() && !(Loc < Right->location());
+ bool AcceptLeft = Right != All.begin() && !((Right - 1)->endLocation() < Loc);
+ return llvm::makeArrayRef(Right - (AcceptLeft ? 1 : 0),
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > Maybe compare offsets instead of locations?
> > It might be more code, but it would read much cleaner.
> >
> > It always feels weird to compare source locations for anything other than storing them in `std::map`
> I find the `SM.getFileOffset(...)` everywhere pretty hard to read and obscures the actual locations.
> But agree about the operators, I've added the rest. Having `operator<` is pretty evil, but I don't think the others are particularly incrementally evil.
I find it to be the only way to write code, which unambiguously says what it wants to do with source locations.
Not saying it's pretty, but I the source location API was designed to be used this way. Arguably, using something else in special corner cases does more harm than good (hard-to-read, non-conventional code).
But up to you.
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:270
+ for (const syntax::Token &Tok : spelledTokensTouching(Loc, Tokens))
+ if (Tok.kind() == tok::identifier)
+ return &Tok;
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > NIT: add braces around `if` statement
> Is there some reference for preferred LLVM style for this, or personal preference? (Real question, as this comes up a bunch)
>
> I ask because that's my *least*-preferred option - no braces > braces on for > braces on both > braces on (only) if.
>
> Added braces to the `for` (maybe that's what you meant?)
Not sure if it's in LLVM style guide, but files inside Syntax folder definitely use this style: put braces everywhere until you reach the last level of nesting for non-leaf statements (i.e. having other statements as children, e.g. loops,. if statements, etc)
It's my personal preference, happy to discuss whether having this makes sense.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71356/new/
https://reviews.llvm.org/D71356
More information about the cfe-commits
mailing list