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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to