vsapsai added inline comments.
================
Comment at: lib/Frontend/DependencyFile.cpp:182-185
for (const auto &ExtraDep : Opts.ExtraDeps) {
AddFilename(ExtraDep);
+ ++InputFileIndex;
}
----------------
This is incorrect if there are duplicates in `Opts.ExtraDeps`. Also please
update the test to have duplicate extra dependencies.
================
Comment at: test/Frontend/dependency-gen-extradeps-phony.c:6-7
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra
+// CHECK-NOT: .c:
+// CHECK: 1.extra:
----------------
I think it would be better to have a check
// CHECK: dependency-gen-extradeps-phony.c
Because you expect it to be there and it's not that simple to notice the colon
in `.c:`, so it's not immediately clear how CHECK-NOT is applied here.
================
Comment at: test/Frontend/dependency-gen-extradeps-phony.c:9-11
+// CHECK-NOT: .c:
+// CHECK: 2.extra:
+// CHECK-NOT: .c:
----------------
For these repeated CHECK-NOT please consider using `FileCheck
--implicit-check-not`. In this case it's not that important as the test is
small but it can still help to capture your intention more clearly.
Repository:
rC Clang
https://reviews.llvm.org/D44568
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
