[PATCH] D54769: [FileCheck] New option -warn

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 26 17:30:26 PST 2018


probinson added a comment.

In D54769#1308918 <https://reviews.llvm.org/D54769#1308918>, @SjoerdMeijer wrote:

> And I do remember a possibly valid use case for emitting a diagnostic. I've seen quite a few tests with names like `test-pr12345.ll`, that were reproducers of compiler crashes, but only contained CHECK-LABELS. I called them dubious tests before (but we don't want to use that term), because these tests are happy when a functions compiles, but don't tests anything whereas they probably should. There are probably a few exceptions where only CHECK-LABELs are okay, so I think a warning along the lines of "you're only checking labels, is that what you really want?" should be fair?


In practices, a LABEL-only test works exactly like the same test with all the -LABEL suffixes removed.  That is, it still does check that the specified strings are present.  We could decree that a LABEL-only test is invalid, and make that an error instead of a warning, but only after fixing the offending tests that are in the code base now.

Given that FileCheck is primarily a non-interactive tool, warnings aren't particularly beneficial.  Even when I'm writing a test, typically I don't run FileCheck directly, I'm running the test under `lit`.  I won't even see a diagnostic unless I run `lit -v`, which I probably won't if the test passes first try.
So, errors (that can be silenced, if we can't fix all the problems up front) are the way to go.


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

https://reviews.llvm.org/D54769





More information about the llvm-commits mailing list