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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
