<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 23, 2020 at 11:41 AM Louis Dionne via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">ldionne added a comment.<br>
<br>
In D76256#1936968 <<a href="https://reviews.llvm.org/D76256#1936968" rel="noreferrer" target="_blank">https://reviews.llvm.org/D76256#1936968</a>>, @EricWF wrote:<br>
<br>
> > These tests already don't do much on GCC, they just confirm that `-Werror-unused-result` works properly.<br>
><br>
> LGTM if we agree on the `// UNSUPPORTED: gcc` change.<br>
<br>
<br>
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.<br>
<br></blockquote><div><br></div><div>It should be obvious from the // requires tags why GCC is unsupported.</div><div>If you disagree we could add comments above each // REQUIRES line.</div><div><br></div><div>I just really dislike having tests opt-in rather than opt-out.</div><div>A test that fails to opt-in correctly manifests as a silent bug.</div><div>A test that fails to opt-out correctly is immediately discovered.</div><div><br></div><div>This wouldn't be the first time `config.py` has mis-configured and disabled tests.</div><div><br></div><div>/Eric</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Repository:<br>
  rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D76256/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D76256/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D76256" rel="noreferrer" target="_blank">https://reviews.llvm.org/D76256</a><br>
<br>
<br>
<br>
</blockquote></div></div>