Open adrianimboden opened 1 year ago
Thanks for the PR, and the detailed description. Sorry for the late reply.
I think that users do not include the system paths very often (that would mean they use system libraries, which in bazel should be rare)
Unfortunately, C++ standard doesn't make a difference between different inclusion styles. There's at least one big C++ shop, that (for historical reasons) uses <> includes for everything. (It is a valid decision imo: the difference between a system library and a user library is often very thin or ambiguous) Therefore, as it stands, this PR is a breaking change, and I would like to avoid merging it. Perhaps make it opt-it?
I have a maybe controversial addon here (probably wen can make it configurable somehow), but I think this might help other users as well.
Background: our clang-tidy config (probably others as well) have this configuration inside:
That way we include our own code that only lives in the header files in the clang-tidy check.
Actually, the config is a little bit simplified. It looks like this in reality:
The reason for this overly complex regex is:
external/
)Now this worked for most things in the codebase, until I saw a special case:
include_prefix
orstrip_include_prefix
, then paths likepath/to/$PACKAGE/_virtual_includes/$TARGET
are generated and put on the include pathThe best solution would be to change clang-tidy of course. This change here would probably help other users as well as it excludes those paths by putting them into the system includes.
The patch should not generate problems because:
_virtual_includes
should not be a common folder name that a user chooses manuallyHeaderFilterRegex
is unset, this change has no effect as everything is excluded anyway