[PATCH] D93341: [FileCheck] Do not skip end of line in diagnostics

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

jdenny added a comment.

Thanks, I like this change as well.

> Note to reviewers: An alternative approach would be to restrict the code
> to only skip to the next line if the first character of the pattern is
> known not to match a whitespace-like character. This would respect the
> original intent but keep the inconsistency in terms of column info and
> requires more code. I've only chosen this current approach by laziness
> and would be happy to restrict the logic instead.

It's tempting to do something like that given that `X` so often appears by itself at the end of an input dump line now.  On the other hand, given that FileCheck's matching behavior can be difficult to understand, an accurate representation of its behavior seems more important than a small improvement to input dump readability.

Another alternative to consider is to skip only newlines.  In my experience, it's rare to have patterns that match newlines.  It's not impossible, so it might be worthwhile to not skip newlines when the pattern contains `[[:space:]]`.

Sorry for so many nits in the test expected output.  I intentionally didn't use `--strict-whitespace` in many of the input dump tests so that there won't need to be too many updates if we later change something trivial like whitespace padding.  For test case readability, I'd still like it to reflect the correct semantics and basic alignment.  If we're actually missing bugs as a result of not using `--strict-whitespace`, then maybe there should be a few more `--strict-whitespace` tests.



================
Comment at: llvm/test/FileCheck/dump-input-annotations.txt:445
 ; DAG-VV-NEXT: dag:4'0     !~~ discard: overlaps earlier match
+; DAG-Q-NEXT:  dag:4          X error: no match found
+; DAG-VQ-NEXT: dag:4          X error: no match found
----------------
Messages aren't aligned properly here.  To fix this, the input dump implementation needs to reserve a column for newlines.


================
Comment at: llvm/test/FileCheck/dump-input-annotations.txt:633
 ; SUBST-POS-NEXT:check:1'2                           with "DEF_MATCH2" equal to "def-match2"
-; SUBST-POS-NEXT:           2: def-match1 def-nomatch
-; SUBST-POS-NEXT:check:2'0     X~~~~~~~~~~~~~~~~~~~~~ error: no match found
+; SUBST-POS-NEXT:check:2'0               X error: no match found
 ; SUBST-POS-NEXT:check:2'1                            with "DEF_MATCH1" equal to "def-match1"
----------------
Hopefully this isn't how it  looks in the actual dump.  The `X` and error message should be at the end of the line.  Please verify and update the test case for readability.


================
Comment at: llvm/test/FileCheck/dump-input-context.txt:123
 ; W5-NEXT: check:2            ^~~~~
+; W5-NEXT: check:3'0         X error: no match found
 ; W5-NEXT:           10: foo1
----------------
Again, `X` and the message should be at the end of the line.  Please verify.


================
Comment at: llvm/test/FileCheck/dump-input-context.txt:126
+; W5-NEXT: check:3'0     ~~~~
+; W5-NEXT: check:3'1     ? possible intended match
 ; W5-NEXT:           11: foo2
----------------
The message should be at the end of the line.  Not sure about the `?`.

(Interesting that your change adds a suggested match.)


================
Comment at: llvm/test/FileCheck/dump-input-enable.txt:254
+; DUMP-ERR-NEXT:   next:2'0    ~~~~~~~
+; DUMP-ERR-NEXT:   next:2'1    ? possible intended match
 ; DUMP-ERR-NEXT:   >>>>>>
----------------
Message at end of line.


================
Comment at: llvm/test/FileCheck/dump-input-filter.txt:71
 ; ALL-NEXT: check:2       ^~~~~
+; ALL-NEXT: check:3'0         X error: no match found
 ; ALL-NEXT:           13: foo0
----------------
Shouldn't the `X` be at the newline?


================
Comment at: llvm/test/FileCheck/dump-input-filter.txt:131
 ; ANNOTATION-FULL-NEXT: check:2       ^~~~~
+; ANNOTATION-FULL-NEXT: check:3'0         X error: no match found
 ; ANNOTATION-FULL-NEXT:           13: foo0
----------------
`X` at newline?


================
Comment at: llvm/test/FileCheck/dump-input-filter.txt:191
 ; ANNOTATION-NEXT: check:2       ^~~~~
+; ANNOTATION-NEXT: check:3'0         X error: no match found
 ; ANNOTATION-NEXT:           13: foo0
----------------
`X` at newline?


================
Comment at: llvm/test/FileCheck/dump-input-filter.txt:223
 ; ERROR-NEXT: check:2       ^~~~~
+; ERROR-NEXT: check:3'0         X error: no match found
 ; ERROR-NEXT:           13: foo0
----------------
`X` at newline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93341