Jinjie-Huang wrote:

@tahonermann Thank you for review and sharing the helpful context.​

> I am worried that this diagnostic will be quite noisy for some projects. I 
> think a more useful improvement would be more diagnostics for cases where 
> `clang` and `clang-cl` would resolve a header file differently. I see you 
> already found the one existing case of such a diagnostic 
> [here](https://github.com/llvm/llvm-project/blob/e29d4ebb89705162b588caf69b45581f1da3a45c/clang/lib/Lex/HeaderSearch.cpp#L1000-L1012).

Yes, I agree that it could potentially generate noise, especially if a project 
intentionally uses header files with the same name and carefully designs the 
-Ioptions. 
The consideration here is: currently, this diagnostic is disabled by default. 
Perhaps we could enable it in debugging scenarios, such as when sanitizers are 
active, to help users discover subtle API mismatch problems. Alternatively, it 
could be grouped and enabled alongside something like the[ "ShadowAll 
DiagGroup"](https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticGroups.td#L797),
 or we could consider enabling it by default at a more appropriate time in the 
future? After all, having numerous header files with the same name in a project 
does not seem to be a recommended practice, it could indeed lead to some 
difficult-to-trace problems.


> Is there special handling for `#include_next`? If not, wouldn't effectively 
> every use of `#include_next` be accompanied by this new warning? Or am I 
> misunderstanding something?

According to my observations, not every #include_nextis accompanied by this new 
warning. Let me explain how it works (please correct me if I'm wrong):

The MSVC rule does not support 'include_next', as you mentioned, so the search 
primarily relies on the regular '-I' strategy. The current lookup is 
implemented using an iterator 
([code](https://github.com/llvm/llvm-project/blob/main/clang/lib/Lex/HeaderSearch.cpp#L1021-L1027)).
 When Include_nextcalls HeaderSearch::LookupFile, if FromDiris not empty, the 
iterator is set to start from the position right after the previously searched 
path, thereby avoiding finding the same file again.

Thus, include_next is naturally compatible with this mechanism (thanks to the 
baseline implementation). Unless there is a header file with the same name in 
subsequent search paths, no warning will be triggered—and if one is triggered, 
that is actually the expected behavior.


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