[llvm-dev] [FileCheck] Fix --strict-whitespace --match-full-lines

[Robinson, Paul via llvm-dev]

> -----Original Message-----
> From: Tom de Vries [mailto:Tom_deVries at mentor.com]
> Sent: Thursday, December 15, 2016 2:31 AM
> To: Robinson, Paul
> Cc: Jonathan Roelofs; llvm-dev at lists.llvm.org
> Subject: Re: [llvm-dev] [FileCheck] Fix --strict-whitespace --match-full-
> lines
> 
> On 14/12/16 18:48, Robinson, Paul wrote:
> > Please send patches to llvm-commits not llvm-dev.
> >
> > Writing FileCheck tests has pitfalls. A test along these lines:
> >
> > bla0
> > CHECK:bla1
> >
> > will actually pass, because the CHECK pattern is also part of the input
> > so it will readily match itself.  You want the CHECK lines not to match
> > themselves, which you can easily do by introducing {{}} into the (middle
> > of the) pattern.  That is:
> >
> > bla0
> > CHECK:{{bla1}}
> >
> > will still pass (incorrectly), while
> >
> > bla0
> > CHECK:bla{{1}}
> >
> > will fail (correctly).  A correct FileCheck test would thus be
> >
> > bla1
> > CHECK:bla{{1}}
> >
> >
> 
> I see. ISTM though that the FileCheck tests are written in a style that
> is not the typical usage of FileCheck.
> 
> As far as I've seen sofar, a typical FileCheck usage looks like:
> ...
> <comment-token> RUN: ignore-comments-and-translate %s \
> <comment-token> RUN: | FileCheck %s
> 
> ... input for translation ...
> 
> <comment-token> CHECK: ... check translation ...
> <comment-token> CHECK: ... check translation ...
> ...
> 
> So if you run the FileCheck tests in a similar way, like this:
> ...
> ; RUN: sed 's/^;.*$//' %s \
> ; RUN: | FileCheck %s
> ...
> you don't have to obfuscate the check patterns to work around this
> problem.

An intriguing trick, unfortunately it didn't work when I tried it on
Windows.  The Windows shell doesn't strip the single-quotes, so sed
doesn't understand the command; but a Posix shell requires the quotes,
to avoid globbing the .* part of the expression.  We could work around
that by using 'REQUIRES: shell' but at that point you're basically
never testing FileCheck on Windows, which seems like a pretty serious
hole in the test coverage.

I think we're stuck with the fake regex hack, if we want single-file
tests (for better maintainability).
--paulr

> 
> I've updated the test-case accordingly.
> 
> [ If we want to use this more often, perhaps something like this would
> be good:
> ...
> diff --git a/test/lit.cfg b/test/lit.cfg
> index d8728b6..80580c8 100644
> --- a/test/lit.cfg
> +++ b/test/lit.cfg
> @@ -202,6 +202,9 @@
>     lli += ' -mtriple='+config.host_triple+'-elf'
>   config.substitutions.append( ('%lli', lli ) )
> 
> +config.substitutions.append( ('%filter-txt-comments',
> +                              'sed \'s/^;.*$//\'') )
> +
>   # Similarly, have a macro to use llc with DWARF even when the host is
> win32.
>   llc_dwarf = 'llc'
>   if re.search(r'win32', config.target_triple):
> ...
> ]
> 
> > (I didn't include this in my recent FileCheck Follies lightning talk,
> 
> +1 viewcount.
> 
> > because testing FileCheck itself is kind of an obscure corner of the
> > LLVM world and I was already bumping up against the 5-minute time
> limit.)
> > --paulr
> >
> >
> >> -----Original Message-----
> >> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
> Tom
> >> de Vries via llvm-dev
> >> Sent: Wednesday, December 14, 2016 4:38 AM
> >> To: llvm-dev at lists.llvm.org
> >> Cc: Jonathan Roelofs
> >> Subject: [llvm-dev] [FileCheck] Fix --strict-whitespace --match-full-
> lines
> >>
> >> Hi,
> >>
> >> this patch fixes a problem with leading/trailing whitespace matching
> for
> >> FileCheck --strict-whitespace --match-full-lines.
> >>
> >> Consider a text file:
> >> ...
> >> $ cat DUMP
> >> bla1
> >> bla2
> >>   bla3
> >> bla4
> >>   bla5
> >> ...
> >>
> >> with some leading and trailing spaces, made more visible like this:
> >> ...
> >> $ sed 's/ /_/g' DUMP
> >> bla1
> >> bla2
> >> _bla3
> >> bla4_
> >> _bla5_
> >> ...
> >>
> >> and a FileCheck file CHECK to match DUMP:
> >> ...
> >> $ cat CHECK
> >> // CHECK-LABEL:bla1
> >> // CHECK-NEXT:bla2
> >> // CHECK-NEXT: bla3
> >> // CHECK-NEXT:bla4
> >> // CHECK-NEXT: bla5
> >> ...
> >>
> >> with whitespace made more visible like this:
> >> ...
> >> $ sed 's/ /_/g' CHECK
> >> //_CHECK-LABEL:bla1
> >> //_CHECK-NEXT:bla2
> >> //_CHECK-NEXT:_bla3
> >> //_CHECK-NEXT:bla4_
> >> //_CHECK-NEXT:_bla5_
> >> ...
> >>
> >> When trying the match, it fails:
> >> ...
> >> $ cat DUMP | FileCheck CHECK --strict-whitespace --match-full-lines
> >> CHECK:3:16: error: expected string not found in input
> >> // CHECK-NEXT: bla3
> >>                 ^
> >> <stdin>:3:2: note: scanning from here
> >>   bla3
> >>   ^
> >> ...
> >>
> >> I expect the match to succeed, because I expect leading and trailing
> >> whitespace _not_ to be ignored, because the documentation states:
> >> ...
> >>   --match-full-lines
> >>
> >>      By default, FileCheck allows matches of anywhere on a line. This
> >> option will require all positive matches to cover an entire line.
> >> Leading and trailing whitespace is ignored, unless --strict-whitespace
> >> is also specified.
> >> ...
> >>
> >> After adding some debug code to FileCheck (which I proposed here on
> >> llvm-dev ml as '[FileCheck] Add --verbose'), we can see where things go
> >> wrong:
> >> ...
> >> $ cat DUMP | /home/vries/gt/build/./bin/FileCheck CHECK
> >> --strict-whitespace --match-full-lines --verbose
> >> CHECK:3:16: note: RegEx string match: '^bla3$'
> >> // CHECK-NEXT: bla3
> >> ...
> >>
> >> The resulting regexp string is '^bla3$' instead of '^ bla3$'.
> >>
> >> The patch fixes this, and makes the behavior match the documentation.
> >>
> >> I ran the llvm/test/FileCheck tests, no regressions.
> >>
> 
> Ran check-all this time.
> 
> Any further comments? OK for trunk?
> 
> Thanks,
> - Tom