[PATCH] D71356: [Tooling/Syntax] Helpers to find spelled tokens touching a location.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.


================
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:
> > kbobyrev wrote:
> > > ilya-biryukov wrote:
> > > > 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.
> > > I guess it's a personal preference (also for me), but I don't think there is a strict guideline on that. Interestingly enough, I think there is a piece of code in the styleguide that looks exactly like the code you had: https://llvm.org/docs/CodingStandards.html#turn-predicate-loops-into-predicate-functions
> > > 
> > > Some Clang subprojects tend to put braces everywhere though.
> > > 
> > > That being said, I guess no braces at all would be the best option here.
> > Yeah, so LLVM style is unclear, but has examples with no braces.
> > Could be the argument to get rid of those.
> > 
> > I'd still stick to my personal preferences where I can, but if everyone thinks no braces is better, happy to go with this option.
> I think we're good here - I'm fine with the braces on `for`, and it sounds like that's what you wanted.
> (I guess I misread "braces around if statement" as "braces around if body")
Ah, great, we're on the same page then!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71356