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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
