xmake check clang.tidy passes non-C++ files by default

[yh-sb]

Xmake Version

2.9.4

Operating System Version and Architecture

Windows 11 23H2 22631.4037

Describe Bug

When non C++ files are present in sources, for example Windows icon file icon.rc

    if is_host("windows") then
        add_files("icon.rc")
    end

Then it passed to clang-tidy and check is broken since .rc is not a C++ file:

> xmake check clang.tidy

2 warnings and 2 errors generated.
Error while processing D:\dev\cpp\opengl-app\icon.rc.
D:\dev\cpp\opengl-app\icon.rc:1:1: error: unknown type name 'IDI_ICON1' [clang-diagnostic-error]
    1 | IDI_ICON1 ICON DISCARDABLE "icon.ico"
      | ^
D:\dev\cpp\opengl-app\icon.rc:1:11: error: variable 'ICON' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables,-warnings-as-errors]
    1 | IDI_ICON1 ICON DISCARDABLE "icon.ico"
      |           ^
D:\dev\cpp\opengl-app\icon.rc:1:11: error: variable 'ICON' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage,-warnings-as-errors]
D:\dev\cpp\opengl-app\icon.rc:1:15: error: expected ';' after top level declarator [clang-diagnostic-error]
    1 | IDI_ICON1 ICON DISCARDABLE "icon.ico"
      |               ^
      |               ;
2 warnings treated as errors
error: execv(clang-tidy -p C:\Users\Y\AppData\Local\Temp\.xmake\240908\_295E351DD70344108584FA1227B2FF10.dir\compile_commands.json D:\dev\cpp\opengl-app\icon.rc) failed(1)

I know about -f *.cpp option, but it's slightly inconvenient to pass it each time.

Expected Behavior

By default need to ignore such non C++ files when passing files to clang-tidy.

Maybe need to have regex pattern to ignored filenames or something like this?

Project Configuration

add_rules("mode.debug", "mode.release")

target("app")
    add_files(
        "main.cpp"
    )
    
    if is_host("windows") then
        add_files("icon.rc")
    end

Additional Information and Error Logs

There is code which handles adding files to the sourcelist which passed to clang-tidy:

xmake/xmake/modules/private/check/checkers/clang/tidy.lua

Line 138 in dbf5b76

_add_target_files(sourcefiles, target)

And then:

xmake/xmake/modules/private/check/checkers/clang/tidy.lua

Line 72 in dbf5b76

table.join2(sourcefiles, (target:sourcefiles()))

[SirLynix]

You shouldn't use is_host but is_plat to test platforms, even though I don't think it fixes this issue

[yh-sb]

Why I shouldn't use is_host since icon.rc is handled by all toolchains (MSVC, MinGW and Clang) on Windows?

[SirLynix]

is_host tests the operating system xmake is running on, is_plat tests the target platform the executable is built for.

is_host("windows") would return false on Linux compiling for Windows using Clang/MinGW for example.

In your case you should use is_plat("windows", "mingw")

[SirLynix]

Clang is a toolchain, not a platform, when you're using Clang you're still targeting the windows platform. (windows platform doesn't mean MSVC, it means MSVC-compatible, which includes Clang). If you're using MinGW Clang, you're on the mingw platform as well. (the only difference between windows and mingw platforms is their ABI, and since they're not compatible xmake differentiate them)

[yh-sb]

Meanwhile I am adding new option to clang.tidy check:

    -i,   ignore                   Set files ignore pattern
                                   e.g.
                                       - xmake check clang.tidy -i '*.rc;*.proto'
                                   By default it's *.rc

[SirLynix]

Maybe we should reverse it and make it as "included extensions". Source files can include anything using rules (even binary files, e.g. utils.bin2c).

[waruqi]

try this patch. #5586

[yh-sb]

Now sourcefiles is completly empty and clang-tidy is not called at all.

[waruqi]

try it again. xmake update -s dev

[yh-sb]

Now it works fine!

By the way, I noticed that clang-tidy called multiple times (once per each c++ file). Have you considered that all c++ files can be passed to clang-tidy at once?
clang-tidy [options] <source0> [... <sourceN>]
Moreover, it leads to the situations when clang-tidy errors for new files are not shown till the errors in first files are fixed.

I am asking because it's much more easier to fix all c++ files once (for example from clang-tidy CI output) than fixing one file, then discovering new errors in second file and so on.

[waruqi]

try it again. #5592

But I'm not sure if this will cause problems if there are over 10000 of source files in a large project. There may also be a limit on the length of the parameters. (e.g. windows argument limit)

Perhaps testing one by one is the most stable way.

[yh-sb]

I tested it and it works fine. All C++ project files are passed in the one row (one clang-tidy call).

Regarding cli option string length limit:
We can pass files to clang-tidy by relative paths instead of absolute path. It saves the space in cli options string. I tested it manually by passing relative paths (from the project root directory where top xmake.lua is located) and it works fine as well.

[yh-sb]

Also worth to mention this PR in clang project: #103499 avoid limits on number of sources by accepting a params file

[waruqi]

I tested it and it works fine. All C++ project files are passed in the one row (one clang-tidy call).

Regarding cli option string length limit: We can pass files to clang-tidy by relative paths instead of absolute path. It saves the space in cli options string. I tested it manually by passing relative paths (from the project root directory where top xmake.lua is located) and it works fine as well.

It doesn't solve the problem. The length limit still exists, when too much source files are encountered.

[waruqi]

Also worth to mention this PR in clang project: #103499 avoid limits on number of sources by accepting a params file

But it's not supported right now.