This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fix check_clang_tidy.py trivially passing default CHECK
ClosedPublic

Authored by zinovy.nis on Oct 12 2018, 5:34 AM.

Details

Summary

In https://reviews.llvm.org/D52971#1262200, @ymandel wrote:

Hi,

It looks like this change has disabled FileCheck for all clang-tidy lit tests that don't use check-prefixes. So, they all trivially pass even if their CHECK... lines are wrong. An easy repro is just to randomly modify any CHECK line in a lit file (e.g. llvm/tools/clang/tools/extra/test/clang-tidy/readability-avoid-const-params-in-decls.cpp) and run ninja check-clang-tools.

In check_clang_tidy.py, if you add back (slightly modified) lines 93-95 to the else branch (line 132), it seems to fix the problem. For example, add:

has_check_fixes = check_fixes_prefixes[0] in input_text
has_check_messages = check_messages_prefixes[0] in input_text
has_check_notes = check_notes_prefixes[0] in input_text

Diff Detail

Repository
rL LLVM

Event Timeline

zinovy.nis created this revision.Oct 12 2018, 5:34 AM
JonasToth accepted this revision.Oct 12 2018, 5:55 AM
JonasToth added a subscriber: JonasToth.

Did you verify it actually works?
Otherwise LGTM and because its a bug-fix you can commit and other concerns can be done post-commit.

test/clang-tidy/check_clang_tidy.py
129 ↗(On Diff #169379)

Nit: Could you please move this assert three lines up? It looks that its actually checking the conditions established there.

This revision is now accepted and ready to land.Oct 12 2018, 5:55 AM
This revision was automatically updated to reflect the committed changes.