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

[Joel E. Denny via llvm-dev]

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.

Thanks.

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