[PATCH] D86222: Fix PR46880: Fail CHECK-NOT with undefined variable

Fri Mar 26 10:16:20 PDT 2021

jdenny added a comment.

It seems the main issue left to address is tests that this patch breaks.

================
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:check:2'0     X error: match failed for invalid pattern
+; SUBST-POS-NEXT:check:2'1                             undefined variable: UNDEF
----------------
thopre wrote:
> jdenny wrote:
> > Please restore the correct alignment here and elsewhere in this file.
> > 
> > (I'm starting to think it might have been better to use `-strict-whitespace` in most of these tests.  I find myself turning it on temporarily anyway while adjusting tests because the squashed whitespace makes debugging confusing.  My original goal for leaving it off was to make test updates easier if we were to adjust formatting, but that rarely happens.  What do you think?)
> If whitespace is important it should be on. If formatting changes then we do want things to fail where whitespace is important.
The first test in this file checks whitespace in formatting.  The remaining tests do not check whitespace, but I find them to be more legible when at least aligned, such as aligning the `error` and `possible intended match` lines above with the other diagnostics.  To help maintain that and make debugging easier, maybe `-strict-whitespace` is worthwhile.  One difficulty is that some tests involve multiple verbosity levels, across which whitespace varies, but regexes could help there.

It seems we're accumulating many stylistic changes to make to these tests, so maybe I'll write a patch one day to tackle it all.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86222