erenon / bazel_clang_tidy

Run clang-tidy on Bazel C++ targets directly, efficiently, with caching enabled
MIT License
106 stars 57 forks source link

C headers can mistakenly be recognized as C++ #78

Open yuyawk opened 2 days ago

yuyawk commented 2 days ago

Hi.

While I'm using bazel_clang_tidy at a01e5e262e6d604c21ed11c420a2a245397b995a, I noticed my C header with the extension ".h" is mistakenly recognized as C++.

The cause of this issue is this line. When I changed its condition to src.extension in ["c", "h"], the header was correctly recognized as C.

I'd like to suggest changing that line so that the tool can correctly recognize C/C++, also taking other extensions (e.g. ".C") into account.

Any opinions?

erenon commented 2 days ago

I'm not sure :( I fear that projects use .h files in their C++ programs, and expect those to be considered C++, making this a breaking change. Unfortunately, the official rules_cc (https://bazel.build/reference/be/c-cpp) does not really make a difference between C and C++ headers, as far as I can tell. Can you think of a solution that is backward compatible? what's your use-case?

yuyawk commented 2 days ago

I fear that projects use .h files in their C++ programs, and expect those to be considered C++, making this a breaking change.

Yeah that's exactly the pain point. Indeed some people prefer to use ".h" also for C++, as ISO C++ core guideline implies...

Can you think of a solution that is backward compatible?

My idea is to add a command-line option to overwrite the list of the extensions for C. When the command-line option is not set, the behavior will be the same.

what's your use-case?

In my use case, C and C++ files can coexist in a project, but they have separate extensions. That is, ".c" and ".h" for C, and ".cpp" and ".hpp" for C++.

erenon commented 2 days ago

This'd still break for larger projects, where the two conventions are mixed. What about a tag on each such target? Would that work for you?

yuyawk commented 2 days ago

What about a tag on each such target? Would that work for you?

I guess the use of tags will also be fine for my use case.