[PATCH] D86222: Fix PR46880: Fail CHECK-NOT with undefined variable

Thu Aug 20 06:47:20 PDT 2020

thopre added a comment.

In D86222#2228316 <https://reviews.llvm.org/D86222#2228316>, @jhenderson wrote:

> In D86222#2228193 <https://reviews.llvm.org/D86222#2228193>, @thopre wrote:
>
>> There seems to be a number of failures like due to pattern like (taken from clang/test/CodeGen/debug-info-extern-call.c):
>>
>> // DECLS-FOR-EXTERN-NOT: !DICompileUnit({{.*}}retainedTypes: ![[RETTYPES:[0-9]+]]
>> // DECLS-FOR-EXTERN-NOT: ![[RETTYPES]] = !{
>>
>> I think FileCheck should forbid variable definition in a CHECK-NOT pattern. Granted with this patch we'll catch all the uses of variables defined in CHECK-NOT but we won't catch if they are not used.
>
> I have no idea what that is even trying to do. A definition within a CHECK-NOT doesn't make sense - either the pattern will match, enabling the capture of the variable, and FileCheck to fail (because of a positive CHECK-NOT match), or it will not match, meaning the variable can't be used. In other words, I think forbidding variable definitions in CHECK-NOT makes sense. It's probably worth bringing this up on llvm-dev and cfe-dev though, to give people a head's up/ask for opinions. There may be a way this actually is useful, but if so, I don't know it.

I have no idea either. I've found another case of 2 different invocation of FileCheck with different prefix where one invocation sets a variable and the other use it. I understand the intent having seen the output but I have so far no idea on how to fix it.

>> This commit makes match error a failure for CHECK-NOT and changes PrintNoMatch to print all CHECK-NOT failure in a CHECK-NOT block rather than just the first one.
>
> Are these two inherently linked, or could you split them into two separate changes?

I think they can be separated if that change land first.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86222/new/

https://reviews.llvm.org/D86222