PiotrZSL added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp:84-85 + Lexer::findNextToken(Start, SM, LangOpts); + if (!CurrentToken || !CurrentToken->is(tok::comment)) + return CurrentToken; + ---------------- carlosgalvezp wrote: > I'm not sure this logic is correct - `if (CurrentToken` is there to ensure > that `CurrentToken` is not a nullopt before dereferencing via > `CurrentToken->is`. It should be: > > ``` > if (CurrentToken && !CurrentToken->is(tok::comment)) > return CurrentToken; > ``` Current code is correct, it's inversion of what we had in while, simply if no token, or token is not comment, then leave. ================ Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp:99 - while (Loc < Range.getEnd()) { + while (Loc <= Range.getEnd()) { if (Loc.isMacroID()) ---------------- carlosgalvezp wrote: > This is a bit unintuitive if we compare it with e.g. STL iterators - `end()` > is always "one-past" the end. What's the convention for `SourceRange`, does > `end` contain a valid end range, or one-past-the-end? In SourceRange end is pointing to last token, not to last token + 1. This is inclusive range. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146881/new/ https://reviews.llvm.org/D146881 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits