Page MenuHomePhabricator

[FileCheck] Add docs for --allow-empty
ClosedPublic

Authored by sameerarora101 on Jul 13 2020, 7:48 AM.

Details

Summary

This diff adds documentation for allow-empty flag under FileCheck
docs.

Diff Detail

Event Timeline

sameerarora101 created this revision.Jul 13 2020, 7:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2020, 7:48 AM
jhenderson added inline comments.Jul 14 2020, 12:37 AM
llvm/docs/CommandGuide/FileCheck.rst
186

Perhaps worth saying what the default behaviour is when this option is not specified, e.g. "By default, empty input is rejected."

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

I think the idea is to prevent unexpected empty output from occurring in cases when --allow-empty is not specified. In a case where all a user cares about is that some string doesn't appear in the output, that might help make the test more robust (because they expect some output, just not what they specified), although honestly I'm not convinced, hence my proposal in the mailing list to change it to --expect-empty.

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

If testing for the absence of an error message, FileCheck could give a successful return value even though a testcase failed to compile. Having FileCheck defaults to erroring on empty input allows to catch such a case. However there are genuine cases where one might be fine with an empty input which is why --allow-empty is needed.

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

I think the idea is to prevent unexpected empty output from occurring in cases when --allow-empty is not specified. In a case where all a user cares about is that some string doesn't appear in the output, that might help make the test more robust (because they expect some output, just not what they specified), although honestly I'm not convinced, hence my proposal in the mailing list to change it to --expect-empty.

I think it could happen if you have a testcase that you wish to test the absence of a pattern in debug output. You might still want to compile the testcase in release mode if before a fix it used to segfault at compilation. So you'd have empty output in release mode and non empty output in debug mode. Just a thought though.

sameerarora101 marked an inline comment as done.Jul 14 2020, 6:43 AM

Specifying default behavior.

jhenderson accepted this revision.Jul 14 2020, 6:55 AM

LGTM, but please give a chance for others to make comments.

This revision is now accepted and ready to land.Jul 14 2020, 6:55 AM
thopre accepted this revision.Jul 14 2020, 7:10 AM

LGTM too

MaskRay added a comment.EditedJul 14 2020, 10:27 AM

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

I think the idea is to prevent unexpected empty output from occurring in cases when --allow-empty is not specified. In a case where all a user cares about is that some string doesn't appear in the output, that might help make the test more robust (because they expect some output, just not what they specified), although honestly I'm not convinced, hence my proposal in the mailing list to change it to --expect-empty.

I don't know why we want to make empty input a built-in special behavior... In that case, the test can make the intention explicit by specifying an option, say, --expect-non-empty:

# RUN: command | FileCheck %s --expect-non-empty
# CHECK-NOT: aaa
# CHECK-NOT: bbb

Since pure NOT checks are unreliable, so many tests are doing:

# RUN: command --enable | FileCheck %s
# CHECK: aaa
# CHECK: bbb
# RUN: command --disable | FileCheck %s --check-prefix=NO
# NO-NOT: aaa
# NO-NOT: bbb

I agree that in this case adding --expect-non-empty can be cumbersome to the users.

https://lists.llvm.org/pipermail/llvm-dev/2020-July/143344.html discusses --allow-empty, but I still don't get why we need --allow-empty.

I think the idea is to prevent unexpected empty output from occurring in cases when --allow-empty is not specified. In a case where all a user cares about is that some string doesn't appear in the output, that might help make the test more robust (because they expect some output, just not what they specified), although honestly I'm not convinced, hence my proposal in the mailing list to change it to --expect-empty.

I don't know why we want to make empty input a built-in special behavior... In that case, the test can make the intention explicit by specifying an option, say, --expect-non-empty:

Because it's pretty rare to expect empty - so making the default a bit more likely to catch bugs might be a good tradeoff.

(I tend to get pretty pushy about making sure tests aren't just "negative" (eg: "does anything other than crash" or "does anything other than <this>") - but they still slip through a lot, so having a mechanical feature that might catch a few issues that would otherwise slip through those less restrictive tests seems potentially useful)

Not sure it'd make a /huge/ deal either way - and it'd be hard to evaluate the cost/benefit. (since the benefit is in all the tests that have correctly diagnosed a bug due to their output being empty & that being a default failure - but we don't have a way to search for those instances)

smeenai accepted this revision.Aug 4 2020, 2:28 PM

You can go ahead and commit this.

This revision was landed with ongoing or failed builds.Aug 7 2020, 1:28 PM
This revision was automatically updated to reflect the committed changes.