check_clang_tidy.py is not enforcing CHECK-MESSAGES in header files

[carlosgalvezp]

When trying to create a unit test for this issue, I realized that check_clang_tidy is currently not handling the CHECK-MESSAGES lines from header files.

This can be reproduced with the following change:

--- a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/duplicate-include.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/duplicate-include.h
@@ -6,7 +6,7 @@ extern int g;
 extern int h;
 #include "duplicate-include2.h"
 extern int i;
-// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: foo include

After that change, run ninja check-clang-tools again. Result: all the tests continue to pass. I expect this test to fail since now the message is different.

Is this expected? How can we fix this?

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Carlos Galvez (carlosgalvezp)

When trying to create a unit test for [this issue](https://github.com/llvm/llvm-project/issues/118009), I realized that check_clang_tidy / FileCheck is currently not handling the CHECK-MESSAGES lines from header files.

This can be reproduced with the following change:

--- a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/duplicate-include.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/duplicate-include.h
@@ -6,7 +6,7 @@ extern int g;
 extern int h;
 #include "duplicate-include2.h"
 extern int i;
-// CHECK-MESSAGES: :[[@<!-- -->LINE-2]]:1: warning: duplicate include
+// CHECK-MESSAGES: :[[@<!-- -->LINE-2]]:1: warning: foo include
After that change, run ninja check-clang-tools again. Result: all the tests continue to pass. I expect this test to fail since now the message is different.

Is this expected? How can we fix this?

[pogo59]

I suspect the FileCheck invocation is not being told to look at the header file. FileCheck does not process #include directives, or any other C preprocessor directives; it looks only at the exact file you give it on the command line. CHECK directives must be in that file.

[LegalizeAdulthood]

It's not an issue with FileCheck per se, it's run_clang_tidy.py doesn't have support for comparing changes to header files. I had a change that attempted to enable this: https://github.com/LegalizeAdulthood/llvm-project/tree/legalize-run-clang-tidy

However, that branch is really stale and doesn't rebase cleanly because the python script has had every line changed by modifying the indent level and the string literal style.

[carlosgalvezp]

I see, thanks for the input! I'll update the issue title accordingly

[LegalizeAdulthood]

I had always intended to address this (hence the branch), but I was impeded on keeping my progress current by the massive changes to the python script. At this point, the best approach is to look at the changes that I made to the script and re-implement them from scratch, possibly modifying some things.

Basically, you have to do for header files what is currently done for source files (this is from memory, so it may not be 100% accurate):

• Copy the 'sanitized' header files to a location where clang-tidy will modify them during test execution
• Adjust the command-line arguments to clang-tidy to instruct it to apply fixes to the relevant header files
• Run FileCheck on the modified header files to ensure that the modifications were correctly applied
• Adjust tests to invoke the run script with appropriate arguments to indicate that headers are to be validated

[zeyi2]

Update: macro-to-enum.cpp is also influenced by this issue (see #172391).

After some debugging I think that the testcases in macro-to-enum.cpp are already broken. I tried manually running clang-tidy over the test file with -fix --checks='-*,modernize-macro-to-enum' -header-filter='.*' and the headers aren't changed anyways.

So even the script is enhanced, this file still can't handle headers correctly. To solve this, I think a (partially) rewrite of the test file is needed, which is out of scope of this issue.