hokein 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.
Unfortunately, we don't have any unit tests for all these utility functions.
The modified function is only used in the `readability-isolate-declaration`
check.
I agree that we should add one, will file a bug.
================
Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.h:38
template <typename TokenKind, typename... TokenKinds>
SourceLocation findPreviousAnyTokenKind(SourceLocation Start,
const SourceManager &SM,
----------------
ilya-biryukov wrote:
> Would `findPrevious` have the same problem on the first token of the file?
> Can be hard to check without unit-tests, though.
it depends on the caller. Calling this function directly would probably get
into infinite loop. As this function is only used in
`readability-isolate-declaration` check, it seems that the check adds some
additional guard code before calling this function, it is probably safe, I
assume. (I could not figure out a case that causes the problem).
================
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();
----------------
ilya-biryukov wrote:
> 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.
yeah, I thought that we shouldn't run into this corner case, I tweaked the code
to fix this case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67654/new/
https://reviews.llvm.org/D67654
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
