[PATCH] D67654: [clang-tidy] Fix a potential infinite loop in readability-isolate-declaration check.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Do we have unit tests for clang-tidy? If yes, could we also add unit-tests for this function?

The modified function only needs a source manager and lang options, these are easy to mock.



================
Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.h:38
 template <typename TokenKind, typename... TokenKinds>
 SourceLocation findPreviousAnyTokenKind(SourceLocation Start,
                                         const SourceManager &SM,
----------------
Would `findPrevious` have the same problem on the first token of the file?
Can be hard to check without unit-tests, though.


================
Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.h:66
     Optional<Token> CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
-
-    if (!CurrentToken)
+    if (!CurrentToken || CurrentToken->is(tok::eof))
       return SourceLocation();
----------------
We seem to be missing a corner case here: what if folks are searching for 'tok::eof'?

Currently we would return invalid location, but given the function's signature I'd expect it to return location of 'tok::eof' in that case.

I bet we don't run into this in practice, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67654