[llvm-dev] RFC: [FileCheck] CHECK-DAG for multiple occurrences of string

[Joel E. Denny via llvm-dev]

On Wed, May 16, 2018 at 12:38 PM, Joel E. Denny <jdenny.ornl at gmail.com>
wrote:

> On Wed, May 16, 2018 at 12:24 PM, <paul.robinson at sony.com> wrote:
>
>>
>>
>> > -----Original Message-----
>> > From: Justin Bogner [mailto:justin at justinbogner.com] On Behalf Of
>> Justin
>> > Bogner
>> > Sent: Wednesday, May 16, 2018 12:16 PM
>> > To: llvm-dev
>> > Cc: jdenny.ornl at gmail.com; Robinson, Paul
>> > Subject: Re: [llvm-dev] RFC: [FileCheck] CHECK-DAG for multiple
>> > occurrences of string
>> >
>> > Paul Robinson <paul.robinson at sony.com> writes:
>> > >> From: Joel E. Denny [mailto:jdenny.ornl at gmail.com]
>> > >>> On Mon, May 7, 2018 at 12:56 PM, <paul.robinson at sony.com> wrote:
>> > >>>> 1. In a CHECK-DAG group, don't let the matches for patterns
>> overlap.
>> > >>>> 2. Add a new CHECK-DAG-N directive, where N is some integer, to
>> > express
>> > >>>> that a pattern must have N non-overlapping matches.
>> > >>>
>> > >>> I think #1 is much more intuitive and easy to describe/document than
>> > #2.
>> > >>> Changing the meaning of DAG in that way is highly unlikely to affect
>> > any
>> > >>> existing test, IMO.  And if it does, my first expectation is that
>> the
>> > test
>> > >>> wasn't doing what the author intended anyway.
>> > >>
>> > >> I implemented #1, and I now have 105 new unexpected tests failures
>> from
>> > >> llvm core (58), clang (45), openmp runtime (1), and compiler-rt (1)
>> > test
>> > >> suites.
>> > ...
>> > >> Here are some potential paths forward:
>> > >>
>> > >> P1. Give up on approach #1 (non-overlapping CHECK-DAG) and go back to
>> > >> #2 (CHECK-DAG-N).  I don't prefer this, but it would be less work.
>> > >>
>> > >> P2. Make CHECK-DAG non-overlapping but fix all test cases before
>> > >> committing that change.  Patch reviews will surely be slow.
>> > >>
>> > >> P3. Make non-overlapping CHECK-DAG a special mode that's off by
>> default
>> > >> until all test suites have been fixed.
>> > >
>> > > I vote for P3.  It's easier to divide up the work if the feature is in
>> > > upstream FileCheck, and it's easy for someone doing the work to enable
>> > > the feature in their local branch.  I would not bother making it a
>> > > command-line switch, unless we really do come up with cases where the
>> > > overlapping matches are absolutely the right solution for some class
>> of
>> > > tests.
>> >
>> > I feel like a variant of this where this mode is on by default would
>> > make more sense. That is,
>> >
>> > 1. Modify CHECK-DAG so that overlapping patterns aren't allow, unless
>> >    --deprecated-allow-overlapping-check-dag or something is passed to
>> >    FileCheck.
>> > 2. Add the flag to the 105 current test failures
>> > 3. File bugs and/or create reviews to remove the flag from each of the
>> >    tests.
>> >
>> > There are two reasons I prefer this approach. First, new tests are
>> > "correct by default" while we clean up the existing problematic tests.
>> > Second, out of tree work has an easy path forward and reasonable
>> > forewarning. They can add the flag temporarily and fix problematic tests
>> > when time permits, rather it suddenly breaking their CI whenever open
>> > source flips the switch.
>>
>> Those are excellent points, and that approach totally makes sense.
>> --paulr
>>
>>
> Make sense.  I'll work on putting the patch in phabricator and creating
> the bugzilla issue.  I'll get to implementing the debugging mode eventually.
>

Sorry for the delay.  It's in phabricator here:

https://reviews.llvm.org/D47106

The list of test failures has changed slightly.  I'm re-running and will
create a bugzilla soon.

Thanks.

Joel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180519/9beaff33/attachment.html>