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

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added a comment.

Would the neighbouring work if the error is on a CHECK-NOT and there's another CHECK-NOT immediately preceding and/or following it? Wouldn't the diagnostics for those other CHECK-NOT be the neighbouring diagnostics when what we would want is the neighbouring positive match (i.e. ignore CHECK-NOT)? In other word, with the following directives:

CHECK: start
CHECK-NOT: foo
CHECK-NOT: 42
CHECK-NOT: bar
CHECK: end

and an input consisting of:

start
// lots of stuff
42
// lots more stuff
end

would that reveal lines start and end or some of the stuff because the neighbouring code is looking at the neighbouring CHECK-NOT diagnostics?



================
Comment at: llvm/test/FileCheck/dump-input/filter-error-neighbor/check-label-follows.txt:2
+; The neighboring behavior of -dump-input-filter=error reveals input line 14
+; below (neighbor of an error), and -dump-input-context=2 reveals the
+; surrounding lines.  Those lines are where the actual problem is: the "host
----------------



================
Comment at: llvm/test/FileCheck/dump-input/filter-error-neighbor/check-next-same.txt:1
+; The neighboring behavior of -dump-input-filter=error reveals input line 1
+; below (neighbor preceding an error), and -dump-input-context=2 reveals input
----------------
Why is the error on input line 2 for a CHECK-SAME?


================
Comment at: llvm/test/FileCheck/dump-input/filter-error-neighbor/multiline.txt:3
+; input line (line 5 below) or the first input line (line 28 below) of an
+; annotation neighboring an error (line 16 below), depending on whether it
+; precedes or follows the error.  -dump-input-context=2 reveals the lines
----------------
I think the error line is confusing because it is not clear that the error refer to an **input** error as opposed to an **annotation** error. And that is precisely what confused me on my first reading of this patch. If I understood correctly the code, a NOT **annotation** error as in the range of lines from the first line after the preceding positive match directive (CHECK or CHECK-LABEL here) to the last line before the following positive match directive. i.e. the **input** error is in input line 16 but the **annotation** error goes from line 6 to line 27.


================
Comment at: llvm/test/FileCheck/dump-input/filter-error-neighbor/multiline.txt:14
+; CHECK-LABEL has additional special handling in that its match is revealed if
+; it precedes the last line of an error, but that special handling alone
+; wouldn't be sufficient for CHECK-LABEL here: it wouldn't reveal the first
----------------



================
Comment at: llvm/test/FileCheck/dump-input/filter-error-neighbor/multiline.txt:15
+; it precedes the last line of an error, but that special handling alone
+; wouldn't be sufficient for CHECK-LABEL here: it wouldn't reveal the first
+; CHECK-LABEL's match because the CHECK-NOT match has multiple lines.
----------------
I'm confused. Doesn't the CHECK-NOT diagnostic run from line 6 to 27 and that's why line 3-7(5+2 context lines) and 25-29 (27+2 context lines) are revealed? Should that be talking about the span of the CHECK-NOT annotation/diagnostic instead of the CHECK-NOT match (line 16-17)?

My understanding is that the special handling does not cover the first label because it is only for a label immediately after the last line of a NOT, there's no symmetric special handling for before the error. Forgive me if I've misunderstood again.


================
Comment at: llvm/test/FileCheck/dump-input/filter-error-neighbor/multiline.txt:1-11
+; -dump-input-filter=error reveals only the first or last line of an annotation
+; neighboring an error, depending on whether it precedes or follows the error.
+; This test checks that we don't mix up first and last.  Otherwise, a preceding
+; or following annotation can accidentally be omitted altogether.  Of course,
+; we must have multiline annotations so that first != last.
+;
+; This test also checks that CHECK-LABEL is handled in the same way.  That is,
----------------
jdenny wrote:
> thopre wrote:
> > Could you do like the other tests and explain what input lines are revealed by this code?
> Done.  In most tests, I also clarified which lines are specifically revealed by `-dump-input-filter=error` vs. the lines added by `-dump-input-context=2`.
That's very nice, thank you


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

https://reviews.llvm.org/D96653