[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#2569854 <https://reviews.llvm.org/D96653#2569854>, @thopre wrote:

> Would the neighbouring work if the error is on a CHECK-NOT and there's another CHECK-NOT immediately preceding and/or following it?

Ah, you're right.  There are probably ways to fix this but....

I'm becoming convinced this neighboring heuristic is the wrong approach.  First, the implementation is confusing, as evidenced by your inline comments. Second, and more importantly, it's growing special cases: CHECK-LABEL and now CHECK-NOT.  I fear it will become increasingly brittle as FileCheck evolves.

The real point, which doesn't seem brittle or confusing, is that we want to see search ranges for errors (but probably not for successful patterns as that could get quite noisy).  Actually, explicitly showing those search ranges rather than implying them via neighboring annotations is probably better for people who are still learning FileCheck's logic.

Currently, we show search ranges for unmatched patterns using `X~~`, and sometimes a fuzzy match appears within marked by `?~~`.  So now I guess I'm suggesting adding `X~~` for unexpected matches as well, so `!~~` would appear within.  However, `X~~` is currently documented to mean no match, which is wrong in the latter case.  Moreover, I already find `X~~` to be noisy for long search ranges, which most errors have.  CHECK-NOT blocks are especially cluttered.

A less noisy (in most cases) possibility is for each end of the search range to be a separate annotation.  The markers might be `[` and `]`, but there are other bracketing symbols we could choose.  For example:

  <<<<<<
           . 
           . 
           . 
          28: ....
          29: ....
          30: start
  check:1     ^~~~~
  not:2'0          [ search range start
          31: ....   
          32: ....
           .
           . 
           . 
          59: ....
          60: ....
          61: foo
  not:2'1     !~~  error: no match expected
          62: ....
          63: ....
           .
           .
           .
          88: ....
          89: ....
  not:2'2         ] search range end
          90: end
  check:3     ^~~
  >>>>>>

At first, we might do this only for unexpected matches (CHECK-SAME/CHECK-NEXT match on the wrong line, or CHECK-NOT match).  If we like it, we might then consider it for unmatched directives as well.  In other words, `X~~` would go away, and we'd have, for example:

  <<<<<<
             .
             .
             .
            28: ....
            29: ....
            30: start
  check:1       ^~~~~
  check:2'0          [    error: no match found in search range starting here
            31: ....
            32: ....
            33: ....
             .
             .
             .
            87: ....
            88: ....
  check:2'1         ] search range end
            89: end
  check:3       ^~~
  >>>>>>   

(Again, there might be a fuzzy match marked by `?~~` within, but I didn't put together an example.)

What do you think?  Looking at the implementation, I think search ranges would be easy to capture for errors.

In any case, thanks for your helpful review.  I started writing responses to each of your inline comments, but now I'm thinking it might be best to just reboot.


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

https://reviews.llvm.org/D96653