vsapsai marked 3 inline comments as done.
vsapsai added inline comments.

================
Comment at: include/clang/Lex/PPCallbacks.h:263
+  /// \brief Callback invoked when a has_include directive is read.
+  virtual void HasInclude(SourceLocation Loc, const FileEntry *File) {
+  }
----------------
rsmith wrote:
> This callback seems pretty unhelpful in the case where lookup of the file 
> failed (you just get a source location). Could we pass in the `FileName` and 
> `IsAngled` flag too (like we do for `InclusionDirective`)? I think that's 
> what (eg) a callback tracking anti-dependencies (for a more sophisticated 
> build system that can handle them) would want.
Added. Also added `FileType` so we can ignore system headers for `-MMD`. And 
tweaked doxygen comment to be more consistent with other callbacks.


================
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
----------------
dexonsmith wrote:
> 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`.
I've added a test with system headers. Overall, my testing strategy was to test 
all cases for the following dimensions:
* file exists/file doesn't exist;
* file is user/system header;
* `__has_include`/`__has_include_next` is used.

But I don't test the full matrix as I don't think it provides enough value.


================
Comment at: test/Frontend/dependency-gen-has-include.c:17
+#if __has_include("a/header.h")
+#endif
----------------
bruno wrote:
> Can you also add a testcase for `__has_include_next`?
Added.


https://reviews.llvm.org/D30882



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to