[PATCH] D97813: [FileCheck] Fix -dump-input per-pattern diagnostic indexing

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

thopre added inline comments.


================
Comment at: llvm/test/FileCheck/dump-input/annotations.txt:564-565
 ; Verbose diagnostics are suppressed but not errors.
-; LAB: {{.*}}error:{{.*}}
-; LAB: {{.*}}possible intended match{{.*}}
-
-; LAB:         <<<<<<
-; LAB-NEXT:               1: lab0 
-; LAB-V-NEXT:  label:1'0     ^~~~
-; LAB-V-NEXT:  label:1'1     ^~~~
-; LAB-NEXT:    label:3'0         X error: no match found
-; LAB-NEXT:               2: foo 
-; LAB-NEXT:    label:3'0     ~~~~
-; LAB-NEXT:               3: lab1 
-; LAB-NEXT:    label:3'0     ~~~~~
-; LAB-NEXT:    label:3'1     ?     possible intended match
-; LAB-NEXT:               4: bar 
-; LAB-NEXT:    label:3'0     ~~~~
-; LAB-NEXT:    >>>>>>
-; LAB-NOT:     {{.}}
+; LAB:{{.*}}error:{{.*}}
+; LAB:{{.*}}error:{{.*}}
+;         LAB:<<<<<<
----------------
jdenny wrote:
> thopre wrote:
> > That seems a bit brittle to assume there's always 2 irrelevant errors first. Also it's difficult to check for correctness without running the test manually to see what it produces. What's the actual output?
> As my comment on line 563 attempts to say, my goal was to check that "normal" error diagnostics are not suppressed by input dumps even though the latter repeats the same error diagnostics.  To make the test easier to maintain, I figured it was sufficient to check that no diagnostics were omitted.  I didn't check for extra diagnostics or incorrect text as all that should be tested elsewhere.
> 
> Maybe my testing strategy is wrong.  Do you feel I should type out those error diagnostics?  Or does the comment just need improvement?
> 
> And I botched the number: it should be 3 diagnostics.  Maybe I should have a `-implicit-check-not='error:'` to make sure there are no extra diagnostics.  And maybe I should use `LAB-COUNT-3:`.
> 
I would spell them out then. For all I know, these two LAB directives could catch different errors than the one below (future FileCheck could emit some new error). If you don't want to hardcode the error message, some middle ground would be to match the line number of the error and perhaps a significant word in the error.

E.g.:

LAB: .chk:3:error:{{.*}}no match

For this case I thought the full error makes more sense.


================
Comment at: llvm/test/FileCheck/dump-input/annotations.txt:576
+;    LAB-NEXT:            4: labelB 
+;  LAB-V-NEXT:label:4        ^~~~~~
+;    LAB-NEXT:check:3        ~~~~~~
----------------
jdenny wrote:
> thopre wrote:
> > Why does labelB have only one annotation but there other labels have 2?
> Each CHECK-LABEL matches once to find the end of a section and then again as the last directive in the section, so matching occurs like this:
> 
> - first section:
>     - `label:2'0` matches `labelA` to find the end of the section
>     - `check:1`, the first directive in the section, matches `text`
>     - `label:2'1`, the last directive in the section, matches `labelA` again
> - second section:
>     - `label:4` matches `labelB` to find the end of the section
>     - `check:3`, the first directive in the section, fails to match, so no more directives in the section are processed
> - third section:
>     - `label:6'0` matches `labelC`
>     - `check:5` matches `labelC`
>     - `label:6'1` fails to match
> - fourth section:
>     - `label:8'0` matches `labelD`
>     - `not:7` requires the next directive in the section to be matched to establish its search range
>     - thus, `label:8'1` matches `labelD`
>     - `not:7` finds no match
> - fifth section:
>     - `label:10'0` matches `labelE`
>     - `not:9` requires the next to directive in the section to be matched to establish its search range
>     - thus, `label:10'1` matches `labelE`
>     - `not:9` unexpectedly matches `textD`
> - sixth section:
>     - `label:12'0` matches `labelF`
>     - `not:11` requires the next to directive in the section to be matched to establish its search range
>     - thus, `label:12'1` matches `labelF`
>     - `not:11` finds no match
Thank you for the detailed explanation. It's crystal clear now.


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

https://reviews.llvm.org/D97813