[libcxx-commits] [PATCH] D76256: [libc++] Require the use of clang-verify in .fail.cpp tests that don't fail without it
Eric Fiselier via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Mar 23 08:09:22 PDT 2020
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
In D76256#1930417 <https://reviews.llvm.org/D76256#1930417>, @ldionne wrote:
> In D76256#1930359 <https://reviews.llvm.org/D76256#1930359>, @EricWF wrote:
> > These tests still require that "arbitrary" warning flag be enabled.
> I don't understand? The only reason these tests were using that flag was to cause a compilation error (instead of a warning) on GCC. If we properly require that we're able to check the diagnostics, which is the goal, we can stop trying to make them fail arbitrarily on GCC.
Previously these tests required `-Werror=-Wunused-result`. Now they require `-Wunused-result`. The requirement for an arbitrary flag remains, only the spelling changed.
That said, I'm fine with this.
> Basically, it's kind of as if we had said "for these tests (and these tests only), enable -`Werror=unused-typedef` on GCC cause we know that's going to make them fail. Instead, my point is that we can just disable them cause they're not doing much good.
>> I'm also a bit afraid that the `verify-support` tag will get accidentally disabled and these tests along with it.
> I think we need to trust our `config.py` here. I agree it's complicated and I'm working on fixing it, but this change is actually one step exactly in that direction.
Every Clang we test against should support `-verify`. How about we simply say `// UNSUPPORTED: gcc`?
That would assuage my concern.
>> Also, do we care about GCC?
> These tests already don't do much on GCC, they just confirm that `-Werror-unused-result` works properly.
LGTM if we agree on the `// UNSUPPORTED: gcc` change.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits