[PATCH] D83682: [FileCheck] Add docs for --allow-empty

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D83682#2150925 <https://reviews.llvm.org/D83682#2150925>, @MaskRay wrote:

> In D83682#2149531 <https://reviews.llvm.org/D83682#2149531>, @jhenderson wrote:
>
> > In D83682#2149473 <https://reviews.llvm.org/D83682#2149473>, @MaskRay wrote:
> >
> > > 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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83682/new/

https://reviews.llvm.org/D83682