[PATCH] D146881: [clang-tidy] Fix findNextTokenSkippingComments & rangeContainsExpansionsOrDirectives

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp accepted this revision.
carlosgalvezp added a comment.
This revision is now accepted and ready to land.

> It's usually tested indirectly by checks... But this code is used I think only in 1-2 checks.

I see. I'm still trying to understand why we haven't seen this before. If the function is an endless loop, shouldn't the checks that use this function end up in an endless loop too? Would be interesting to try and reproduce an endless loop in the tests of one of the checks that use this. Anyway the patch is fine as is, thanks for fixing!



================
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;
+
----------------
PiotrZSL wrote:
> 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.
You are right, nevermind!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146881