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

[Justin Bogner via llvm-dev]

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.

> I suggest you file a bug with the complete list of test failures that
> non-overlapping CHECK-DAG finds.  That will help keep track of the work,
> and if anyone else feels moved to pick up some part of it, they can make
> notes in the bug so we can try to avoid duplicating effort.  Hopefully
> I can devote some time to this myself, I think it's how CHECK-DAG should
> have been done in the first place and I'm sad we didn't catch it at the
> time.
>
>> By the way, I think it would be generally useful for FileCheck to have
>> a debugging mode that prints every match.  It would be particularly
>> useful while migrating to a non-overlapping CHECK-DAG.
>>
>> Thoughts?
>>
>> Joel
>
> A debugging mode like that sounds like a useful feature for writing a
> test, independent of the migration aid.  Do it.

+1