[clang] [clang] Support header shadowing diagnostics in Clang header search (PR #162491)

[Jinjie Huang via cfe-commits]

Jinjie-Huang wrote:

Thanks for replies!

> I would not be in favor of that example being diagnosed because I think it would interfere with too much intentional use of #include_next.
> 
> If the intent is to only issue a diagnostic if a header file is included via double quotes and is found relative to the including file and is also found in a different location via the header search path, I think that might make sense (this is what your test case exercises). Use of #include_next in such cases seems somewhat unlikely (if you are using #include_next, you probably want to be relying completely on header search path ordering). I'm not sure if there is a good way to validate how much existing code might be impacted though.

Yes, the initial goal was quite simple: when user #include "header.h" or <header.h>, if multiple copies of header.h exist in the search paths (e.g., include1 and include2), we want to notify the user that the compiler selected include1/header.h, and the first-found one may have shadowed the second one from include2.

The motivation is that we encountered this situation in a large internal project: when the main project implicitly introduces multiple versions of a third-party library, it can lead to dependencies on identical header files in different paths, which may cause ODR issues and a bit hard to debug. So we also want to see what the community's opinion about this diagnostics.

For users of include_next, this warning would indeed be noise. Therefore, the current idea is to disable this warning by default, requiring the user to manually enable it, or to enable it automatically when a sanitizer is turned on, in order to avoid daily disruption. And based on observations, our internal uses of #include_next are quite rare, so the original idea was mainly to help identify issues in the standard #include scenarios. Does this sound reasonable?

> I wonder if the existing ext_pp_include_search_ms diagnostic should be incorporated into this one. Technically, that shouldn't be an extension warning since header file resolution isn't defined by the C or C++ standards.

Thanks for suggestion, it might be possible to incorporate the logic of the new warning into 'ext_pp_include_search_ms', I’ll give it a try.

https://github.com/llvm/llvm-project/pull/162491