[PATCH] D96653: [FileCheck] Add neighboring annotations for -dump-input-filter=error

[Joel E. Denny via Phabricator via llvm-commits]

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