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