[libcxx-commits] [PATCH] D76256: [libc++] Require the use of clang-verify in .fail.cpp tests that don't fail without it

[Eric Fiselier via libcxx-commits]

On Mon, Mar 23, 2020 at 11:41 AM Louis Dionne via Phabricator <
reviews at reviews.llvm.org> wrote:

> ldionne added a comment.
>
> In D76256#1936968 <https://reviews.llvm.org/D76256#1936968>, @EricWF
> wrote:
>
> > > 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.
>
>
> So, more generally, would you be OK with dropping the notion of `//
> REQUIRES: verify-support` and instead translate that into `// UNSUPPORTED:
> gcc`? I believe the best would be to simplify the logic for how we
> currently determine the `verify-support` feature by just checking whether
> we're using GCC, but leave `// REQUIRES: verify-support` as that provides
> more information semantically than `// UNSUPPORTED: gcc`. WDYT? If we're on
> the same page, I can do that as a follow up change.
>
>
It should be obvious from the // requires tags why GCC is unsupported.
If you disagree we could add comments above each // REQUIRES line.

I just really dislike having tests opt-in rather than opt-out.
A test that fails to opt-in correctly manifests as a silent bug.
A test that fails to opt-out correctly is immediately discovered.

This wouldn't be the first time `config.py` has mis-configured and disabled
tests.

/Eric


>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D76256/new/
>
> https://reviews.llvm.org/D76256
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200323/e8f2c0c0/attachment.html>