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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 26 17:00:49 PST 2018


dblaikie 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?


I'd probably err towards saying "this (whatever it is - but the conservative behavior in any given situation) is the default behavior, you can pass a flag if you want the other" - if we're sure having a flag & that schism in readability, etc, is worthwhile rather than constraining all tests to the conservative behavior. Having a warning seems like it'd be hard to maintain (for the same reason compiler warnings that aren't -Werror are hard to maintain - the difference here is we control both the use and the tool, so there's no need to make warnings non-errors like we have to for the public interface to the compiler) - when I go and edit a test and it flags this warning, how do I know if the original author intended it or not? If they have to put a specific flag there to say "hey, this is the filecheck behavior I want" that seems better to me.


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

https://reviews.llvm.org/D54769





More information about the llvm-commits mailing list