[PATCH] D96653: [FileCheck] Add neighboring annotations for -dump-input-filter=error
Joel E. Denny via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 26 07:07:47 PST 2021
jdenny added a comment.
In D96653#2589769 <https://reviews.llvm.org/D96653#2589769>, @thopre wrote:
> Though I agree with the problem of special casing, I'd like to add that the disctinction between positive and negative match directives will be needed for supporting numeric substitution block using a variable defined on the same directive.
I don't follow. I might have missed a discussion. Could you provide a quick example?
> Regarding CHECK-LABEL, can't we have a data structure that allow browsing annotation by start line/point and end line/point? Is the position of CHECK-LABEL annotations in the list of annotation an artifact of how CHECK-LABEL are processed or is there a need for those annotation to be before another directive starting on the next line?
The former. D77607 <https://reviews.llvm.org/D77607> caused this. I still feel like D77607 <https://reviews.llvm.org/D77607> is generally right. The strange part to me is that a preceding directive's search range ends at the end of the CHECK-LABEL match. It seems like it ought to end at the start of the CHECK-LABEL match. `-dump-input` reveals that CHECK-LABEL behavior is odd:
$ cat input
foo
$ cat check
CHECK: foo
CHECK-LABEL: foo
$ FileCheck -vv check < input |& tail -6
<<<<<<
1: foo
label:2 ^~~
check:1 ^~~
label:2 X error: no match found
>>>>>>
Per input line, annotations are sorted by time. The CHECK-LABEL matches first. Then the CHECK matches the same text. Then CHECK-LABEL runs again and fails. I suppose the reason for this behavior is to catch poorly chosen labels.
Hopefully you see the value of D77607 <https://reviews.llvm.org/D77607>'s sort by time: it make the FileCheck behavior clearer even when that behavior is odd.
>> What do you think? Looking at the implementation, I think search ranges would be easy to capture for errors.
>
> So you'd show neighboring lines around search range start and end?
They should be included via `--dump-input-context=N`... except sometimes in the bizarre case someone sets N=0, but I'm not too worried about that.
> That looks like a better characterisation of what this patch was trying to do so I agree it seems like a good idea.
Thanks for the review. I'll work on this.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96653/new/
https://reviews.llvm.org/D96653
More information about the llvm-commits
mailing list