Page MenuHomePhabricator

[FileCheck] Abort if -NOT is combined with another suffix
ClosedPublic

Authored by probinson on Feb 24 2016, 5:49 PM.

Details

Summary

The -NOT suffix by itself is implicitly DAG, but -DAG-NOT is ignored.
And as handy as -SAME-NOT and -NEXT-NOT might be, they are actually
not supported. Saying so will reduce the incidence of bogus tests.

Separate reviews will fix the LLVM and Clang tests that have this problem.

Diff Detail

Repository
rL LLVM

Event Timeline

probinson updated this revision to Diff 48996.Feb 24 2016, 5:49 PM
probinson retitled this revision from to [FileCheck] Abort if -NOT is combined with another suffix.
probinson updated this object.
probinson added reviewers: jyknight, dexonsmith.
probinson added a subscriber: llvm-commits.

LLVM test fixes are in D17588.
Clang test fixes are in D17589.
I made those separate because they'll have to be committed before the FileCheck change goes in.

jyknight accepted this revision.Feb 25 2016, 10:50 AM
jyknight edited edge metadata.

Sounds good after the prerequisite changes go in.

This revision is now accepted and ready to land.Feb 25 2016, 10:50 AM
arsenm added a subscriber: arsenm.Feb 25 2016, 11:11 AM

Another good thing to catch would be CHECK-DAGs mixed with CHECK-NOTs, which does not work as expected

Another good thing to catch would be CHECK-DAGs mixed with CHECK-NOTs, which does not work as expected

What are your expectations?

Another good thing to catch would be CHECK-DAGs mixed with CHECK-NOTs, which does not work as expected

What are your expectations?

I often want to check a sequence of N instructions are in any order, but also excluding others between them. IIRC, the -NOT may just be a sub-problem of multiple identical CHECK-DAGs.

Something like
; CHECK-DAG: add
; CHECK-DAG: sub
; CHECK-DAG: add

Will pass with only 1 add in the sequence. The -NOT to exclude some intermixed instructions acts as a barrier between the sections of CHECK-DAGs.

The -NOT to exclude some intermixed instructions acts as a barrier between the sections of CHECK-DAGs.

Okay, that's how it's documented to work. And some experimentation suggests that despite the documentation, that's not actually how it works. Huh.
A CHECK-DAG followed by CHECK-NOT causes the CHECK-DAG match to bound the range for the CHECK-NOT.
However, a CHECK-NOT followed by a CHECK-DAG does not cause the CHECK-DAG to bound the range of the CHECK-NOT.

That feels more like a bug in FileCheck, than a mistake made by the test-writer? In this patch I'm trying to avoid test-writer mistakes.

@probinson: if you're on a general crusade against more such test-writer mistakes, here's another one:

  • CHECK lines without the requisite colon

I've fixed a few individual instances of this before, but when I sat down to try to get FileCheck to warn about it, I couldn't see how to do it without making a mess. Perhaps you've got a better handle on how to do it?

@probinson: if you're on a general crusade against more such test-writer mistakes, here's another one:

  • CHECK lines without the requisite colon

    I've fixed a few individual instances of this before, but when I sat down to try to get FileCheck to warn about it, I couldn't see how to do it without making a mess. Perhaps you've got a better handle on how to do it?

I was not necessarily on a general campaign (although clearly FileCheck and its documentation could use a little love). Your suggestion in particular could be subject to false positives (for example if you said "RUN: ... -check-prefix MYSTUFF" you don't want to report that RUN line).

Another one I've tripped over is that if you specify multiple -check-prefix options, FileCheck won't detect typos. It insists that any specified combination of prefixes must result in a nonzero number of checks, but it doesn't verify that each individual prefix occurs at least once.

Another one I've tripped over is that if you specify multiple -check-prefix options, FileCheck won't detect typos. It insists that any specified combination of prefixes must result in a nonzero number of checks, but it doesn't verify that each individual prefix occurs at least once.

I would not want it to verify that each individual prefix occurs at least once

Another one I've tripped over is that if you specify multiple -check-prefix options, FileCheck won't detect typos. It insists that any specified combination of prefixes must result in a nonzero number of checks, but it doesn't verify that each individual prefix occurs at least once.

I would not want it to verify that each individual prefix occurs at least once

What is your use-case for a -check-prefix option that specifies a prefix that does not exist?

This revision was automatically updated to reflect the committed changes.