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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 26 14:53:35 PST 2018


dblaikie added a comment.

In D54769#1308775 <https://reviews.llvm.org/D54769#1308775>, @probinson wrote:

> In D54769#1308690 <https://reviews.llvm.org/D54769#1308690>, @jdenny wrote:
>
> > In D54769#1308654 <https://reviews.llvm.org/D54769#1308654>, @dblaikie wrote:
> >
> > > You/others could checkin fixes as they find them rather than accumulating one large patch - then when it's close to done, you send out the patch to add the error to avoid regressions?
> >
> >
> > Adding -warn now makes it easier for anyone to contribute to this cleanup and doesn't hurt anyone who doesn't care about it.
>
>
> I wonder if `-allow-deprecated-dag-overlap` is a better model: an error that can be explicitly disabled.  The idea there was that over time we could rework the buggy tests, and when that's done, remove the option.  In the meantime all new tests will get complaints, which is what we want.
>
> I think @dblaikie has a point; if we want to move to a new world with less-fallible tests, it's better for new FileCheck diags to be errors that are on-by-default, and explicitly disabled in the tests that we haven't managed to fix yet.


Sounds plausible to me - if the cleanup work is big enough to merit doing it that way. (so I'd suggest evaluating on a case-by-case basis, if the cleanup work needs to be distributed amongst people (are there enough people signing up to do this work, etc))

I'd like to avoid (though I probably don't feel so strongly that it's either "this must have a timeline to cleanup/removing the deprecating flag - or we shouldn't do it at all" just "if these deprecated flags stick around in a handful of tests for years I'll be a bit sad") these things sticking around indefinitely, of course.


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

https://reviews.llvm.org/D54769





More information about the llvm-commits mailing list