vsapsai accepted this revision. vsapsai added a comment. This revision is now accepted and ready to land.
Looks good to me. Just watch the build bots in case some of them are strict with warnings and require `(void)AddFilename(Filename)`. As for the case when the input file is also an extra dependency, I think we can ignore that for now because extra dependencies are supposed to be sanitizer blacklists. Even if that changes in the future, I expect extra dependencies to stay orthogonal to the input. ================ 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: ---------------- dstenb wrote: > vsapsai wrote: > > 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. > Do you mean replace the two checks with that? Isn't there a risk that that > would match with `dependency-gen-extradeps-phony.c:`, which the not-checks > would not pick up then? > > I added a CHECK-NEXT check for the input file so that we match that whole > dependency entry at least. You are right, you need to be careful to make sure that we match .c file only as a dependency and not as a target. Good solution with CHECK-NEXT. Now I'm happy with the test as expected output is clearly visible and we protect from undesired output. https://reviews.llvm.org/D44568 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
