[PATCH] Allow multiple check prefixes in FileCheck

[Daniel Sanders]

Sorry for the slow response. I was out of the office on Friday.

I've a patch that adds a pair of tests that demonstrate that FileCheck is skipping checks, the patch also adds the debugging messages I was using while writing this email. The tests only differ in the order that -check-prefix=A and -check-prefix=B are specified on the command line and contain a single B-DAG directive. check-a-b-has-b.txt fails with the message "error: no check strings found with prefixes 'A:', 'B:'" but check-b-a-has-b.txt passes. If FindFirstMatchingPrefix behaved as you describe, then both tests should behave in the same way.

The problem is that FindFirstMatchingPrefix iterates over the prefixes and then short-circuits the rest of the loop when it finds a match. The match is the first occurrence of that particular prefix in the buffer but it is not necessarily the first occurrence of any of the prefixes. In the attached check-a-b-has-b.txt, there are four (I was expecting two, I'll explain below) calls to FindFirstMatchingPrefix. All of these matches match the A prefix. The first two match the A in the RUN line, the third and fourth matches match the A in 'B-DAG'. When the third call to FindFirstMatchingPrefix returns the 'A' in 'B-DAG', the caller (ReadCheckFile) substrs the buffer and permanently loses the missed B prefix.

The unexpected calls to FindFirstMatchingPrefix appears to be the result of a seemingly harmless off-by-one error that was introduced in r188221. The 'Buffer.substr()' on line 762 is trying to remove the prefix from the buffer but fails to account for the extra character it was keeping to check for word boundaries. The one-character prefixes we have used in our tests then match a second time before being removed from the buffer.

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Matt Arsenault
> Sent: 29 August 2013 18:12
> To: reviews+D1374+public+721da8dd7b49ee6f at llvm-reviews.chandlerc.com
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Allow multiple check prefixes in FileCheck
> 
> 
> On Aug 29, 2013, at 7:28 , Daniel Sanders <Daniel.Sanders at imgtec.com>
> wrote:
> 
> >
> >  It looks like FindFirstMatchingPrefix will only match CheckPrefixes[1] after
> all instances of the CheckPrefixes[0] have been discovered. Doesn't this lead
> to skipping some of the checks?
> 
> No, it just searches for the first one that matches in the buffer. It doesn't
> advance the point it's searching from in any way while doing that.
> 
> >
> >  Using test/FileCheck/check-dag-multi-prefix.txt as an example. I think it
> would match the A prefixes on lines 19-21, fail to find any further A prefixes,
> then match the B prefixes on lines 23-26. I don't think the B prefixes on lines
> 15-17 would be added to CheckStrings in ReadCheckFile(). The test would still
> pass but it won't have performed all the checks.
> >
> >  It might also skip the second block of B's. I think it would match the 'A' in 'B-
> DAG', then line 739 of FileCheck.cpp would discard the 'B-' preventing the B
> prefix from being matched.
> >
> > http://llvm-reviews.chandlerc.com/D1374
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Tests-and-debugging-messages.patch
Type: application/octet-stream
Size: 2173 bytes
Desc: 0001-Tests-and-debugging-messages.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130902/48ca360d/attachment.obj>