[libcxx-commits] [PATCH] D140632: [NFC][libc++] Removes concepts tests.

Fri Dec 30 05:38:54 PST 2022

philnik accepted this revision.
philnik added a comment.
This revision is now accepted and ready to land.

In D140632#4018515 <https://reviews.llvm.org/D140632#4018515>, @Mordante wrote:

> In D140632#4016552 <https://reviews.llvm.org/D140632#4016552>, @h-vetinari wrote:
>
>>> Clang trunk doesn't claim support: https://godbolt.org/z/hes3nah8s. I'm actually quite surprised this doesn't break the CI. I remember having problems with at least Clang 15 when trying to use conditionally trivial special member functions.
>>
>> See https://reviews.llvm.org/D128619; it's implemented, but there are still a large number of open concept issues so the feature macro hasn't been set.
>
> That might be true, but this part seems to work. So I really would like to land this.
> We normally don't use feature-test macros in our test, instead we define a helper macro which we use in the tests.
> Since it works it makes little sense to add a helper macro to do the right thing., s

I'm not a huge fan of enabling the tests when we know that the implementation is still incomplete and buggy (which is the reason the FTM hasn't been bumped yet). Though I guess we do that already in other places, so I guess this looks good to me. We should reconsider that position though IMO, since it results in workarounds where nobody knows in the end why they existed in the first place.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140632