[clang] [clang-tools-extra] Clang/Preprocessor: Not add headers of __has_include into DepColloctor (PR #120673)

Michael Spencer via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 15 11:28:00 PST 2025


Bigcheese wrote:

> > It's intentional that Clang reports these as dependencies. What issue do you have where you think this is the wrong behavior?
> 
> A test case of glibc emit error for this behavior:
> 
> * check-local-headers
> 
> > More specifically, https://reviews.llvm.org/D30882 originally added the behavior. This references an internal Apple bug that occurred when you have code that checks for the presence of a header but does not include it. Not reporting that dependency can lead to failed builds or miscompiles in incremental builds.
> 
> So, maybe we need to add an OS-specific check?

There's nothing OS specific about the issue, it's just that we ran into it. It's a fundamental issue of how `__has_include` works. For example in a normal incremental build, if you remove the the header file that was only referenced by a `__has_include`, if it's not in the dependency list then nothing will be rebuilt. This is wrong, as it should be rebuilt. You can also run into cases where a separate source file that does actually conditionally include that header is rebuilt, and now you have out of sync object files that can lead to a broken program.

This also causes problems for tools like ccache or some types of distributed builds. They need to know about all inputs.

I believe that gcc is wrong to not report these as dependencies, and thus glibc's check-local-headers.sh script is wrong too.

I do think it's fine to restrict when we report the dependency some though. For example `#if 0 && __has_include(...)` is always false, and so the presence of the file doesn't matter. Same with a truly empty `#if` body. However I view these as a very minor incremental build optimization. No actual tool I'm aware of has issues with reporting a file that exists and you have access to as a dependency.

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


More information about the cfe-commits mailing list