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

[Joel E. Denny via llvm-dev]

On Sat, May 19, 2018 at 11:38 AM, Joel E. Denny <jdenny.ornl at gmail.com>
wrote:

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

The bugzilla is here:

https://bugs.llvm.org/show_bug.cgi?id=37532

The debugging mode patch is here:

https://reviews.llvm.org/D47114

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