[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In https://reviews.llvm.org/D30882#1075407, @ddunbar wrote:

> In https://reviews.llvm.org/D30882#1074822, @dexonsmith wrote:
>
> > I don't think this is quite right.  I know at least `make`-based incremental builds wouldn't deal well with this.
>
>
> This is actually not a novel problem w.r.t. this patch. The exact same situation comes up with Makefile-included .d files and when one of the referenced headers is removed.
>
> This is typically solved somewhere in the build system, for example Make has `-include`, and Ninja and llbuild have explicit support for this situation.


Of course, yes, I was wrong.  Thanks for the correction.

> I agree we might want to tread cautiously on how we change the .d output, but in this case I think it might be safe.
> 
> If we decide this isn't safe, then we may want to consider a new flag which tracks *all* anti-dependencies (file's for which Clang checked existence but did not exist), and include that here. The major concern with that approach is it is a much larger list, and while it would support being substantially more correct, it is also well beyond what people currently expect out of the build system + compiler generated deps approaches.

Even though this is likely safe, it seems really noisy.  Consider the case where someone has `__has_include(<missing>)` -- we'll get an entry for every `-I`, `-F`, `-isystem`, and `-iframework`.  If we're going to add that noise, I prefer a separate flag that covers all anti-dependencies.



================
Comment at: test/Frontend/dependency-gen-has-include.c:11-15
+// RUN: %clang -MD -MF %t.dir/file.deps %s -fsyntax-only -I %t.dir
+// RUN: FileCheck -input-file=%t.dir/file.deps %s
+// CHECK: dependency-gen-has-include.o
+// CHECK: dependency-gen-has-include.c
+// CHECK: a/header.h
----------------
This covers quoted includes when there is a single `-I`.  I think there are a couple of other cases worth testing:
- Multiple `-I` arguments: all should be listed.
- Angle includes: all `-isystem` should be listed too.
- Also `-F` and `-iframework`.


https://reviews.llvm.org/D30882