[PATCH] D53710: [FileCheck] Warn if a prefix is only used in LABEL checks

Joel E. Denny via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 9 08:30:36 PST 2018


jdenny added a comment.

In https://reviews.llvm.org/D53710#1292880, @SjoerdMeijer wrote:

> Agreed with all previous comments! And yes, I messed up my last lit test example :-( and I looked at yours and Paul's, and agree. But anyway, we have a plan...


No problem.  It looks like we're converging.

> Thus, I think it is already under and option, which is not enabled by default.

It's enabled by default at least for anyone who runs lit with -v or -a.  I don't know how noisy that could be, but running with more of LLVM's test suites would help to determine that.

> The reason I am asking, is that I am afraid that when we put this under a new, separate option (if I understood correctly), people won't use it, and there's perhaps little benefit of having it?

Maybe so, but starting with it off by default gives everyone a chance to try it out before deciding it should be on by default.  That is, a follow-up patch could change the default to on.

We all seem to agree it's easy to make mistakes with FileCheck, and Paul has already suggested a second possible warning, which I also think is valuable.  A -W infrastructure would make it easy for us to experiment with new warnings without bothering people who don't want the noise. -W[no]-foo lets people choose the opposite of the default, and -Werror makes it hard to miss warnings people value.


https://reviews.llvm.org/D53710





More information about the llvm-commits mailing list