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

[Jonathan Roelofs via llvm-dev]

On 12/15/16 9:51 AM, Robinson, Paul wrote:
>
>> -----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.

Indeed. In general the community is moving away from using sed/awk/wc in 
tests, in favor of pure FileCheck tests, precisely because of these 
kinds of portability concerns.

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

+1


Jon

> --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

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded