[libcxx-commits] [PATCH] D115977: [libc++] Implement P1425R4 (Iterator pair constructors for std::stack and std::queue)

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision as: Mordante.
Mordante added a comment.

In D115977#3223872 <https://reviews.llvm.org/D115977#3223872>, @Quuxplusone wrote:

> In D115977#3223823 <https://reviews.llvm.org/D115977#3223823>, @jloser wrote:
>
>> I haven't finished reviewing this in full yet, but do we need to use `enable_if_t` instead of concepts/`requires` since this is C++23 code? I.e. is our requirement of using older `AppleClang` version in C++23 mode an issue with this (and therefore we must use `enable_if_t` still)? The consistency argument is fine - I'm asking purely from a technical standpoint.
>
> If you want it to be present in `-std=c++2b` mode, on old compilers without full concepts support, then it can't depend on full concepts support (although technically maybe those old compilers support //enough// of concepts that a simple `requires`-clause would be OK... but let's not go down that rabbit hole, let's just wait a couple more years). If you guard it under `_LIBCPP_HAS_NO_CONCEPTS`, then it //won't be present// on old compilers without full concepts support, so I don't think that would be a good idea either. Finally, you could provide two different implementations — one under `_LIBCPP_HAS_NO_CONCEPTS` and one not — but that would be twice the work, and require going down a rabbit hole involving "twice the tests," so let's not do that either.

IIRC the only compiler using `_LIBCPP_HAS_NO_CONCEPTS` is AppleClang. Unfortunately our CI isn't running the latest AppleClang yet so we need to support not concept builds.

LGTM, just make sure the Apple build failure isn't due to your changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115977