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; ---------------- 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. 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
