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

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

jdenny 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:<<<<<<
----------------
thopre wrote:
> 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.
Maybe you're right.  It at least makes it easier to understand.

I've adjusted the tests that this patch touches.  I haven't adjusted other tests in this file.  That can happen in another patch one day.


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

https://reviews.llvm.org/D97813