ecnelises wrote:

A few additional comments/questions:

1. While I know this is disabled by default, have we considered the compilation 
time impact when it is enabled? Since getFileRef has caching mechanisms (IIUC 
it only calls stat), the overhead should theoretically be light. However, for 
projects with deep search paths, the linear scan for every quoted include might 
still add up. It might be worth verifying this on a larger build.


2. Clang has a complex set of options to adjust include paths. I think it would 
be valuable to add test cases for flags like `-idirafter`, `-iquote`, and 
`-isystem-after`. This ensures the logic behaves consistently.

3. Please add a release note to *Improvements to Clang's diagnostics* section.

BTW, In compilers, warnings usually indicate an unexpected or potentially 
dangerous behavior. Here, header shadowing is the expected behavior. But since 
header searching is worded as 'implementation-defined' in standard 
([cpp.include](https://eel.is/c++draft/cpp.include#1)), it's also fine as an 
extension to me.

Basically, this change looks helpful for large codebases. I think this can 
proceed after addressing the nits in tests and release notes, unless there are 
further concerns about the design.

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