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

[via llvm-dev]

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