[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

[Volodymyr Sapsai via Phabricator via cfe-commits]

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