[libcxx-commits] [PATCH] D118507: [libc++] Merge _LIBCPP_HAS_NO_RANGES into _LIBCPP_HAS_NO_CONCEPTS. NFC

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__config:882
 
-#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_HAS_NO_CONCEPTS)
-#define _LIBCPP_HAS_NO_RANGES
----------------
Mordante wrote:
> How about keeping this and test it for `_LIBCPP_HAS_NO_INCOMPLETE_RANGES` too? Wouldn't that solve our problem with not testing `_LIBCPP_HAS_NO_INCOMPLETE_RANGES` everywhere. Of course that would mean we keep both `_LIBCPP_HAS_NO_CONCEPTS` and `_LIBCPP_HAS_NO_RANGES`.
> 
> Note that adding `_LIBCPP_HAS_NO_INCOMPLETE_RANGES` was a kind of hurried last minute fix for the LLVM 13 release, but in hindsight updating this part of the `__config` sounds like a good idea.
I'm not sure I understand, but maybe I do.
We have two orthogonal criteria for `#if`'ing stuff out right now:
- Does it (perhaps transitively) depend on the compiler's support for C++20 `concept`/`requires` syntax?
- Does it (perhaps transitively) depend on the vendor's commitment to `std::ranges` support in the library?
I claim that the former should be called `_LIBCPP_HAS_NO_CONCEPTS` and the latter should be called `_LIBCPP_HAS_NO_INCOMPLETE_RANGES`. IIUC, you're saying that it would cause less churn if we also introduced
```
#if defined(_LIBCPP_HAS_NO_CONCEPTS) || defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
 #define _LIBCPP_HAS_NO_RANGES
#endif
```
So then most of the places guarded by `_LIBCPP_HAS_NO_RANGES` today would be correct; and probably some of them are wrong, but maybe it would be easier to audit for those (few?) places.

Is that the right idea?

It also occurs to me that it would aid understanding if we renamed `s/_LIBCPP_HAS_NO_RANGES/_LIBCPP_HAS_NO_RANGES_NAMESPACE/`. That makes it clearer the exact scope we're talking about (more than `<ranges>`, all of `std::ranges`; but //not// including the One Ranges Proposal stuff that lies outside `std::ranges`, such as `std::forward_iterator`.) Do we agree on that scope, and if so, do we agree on the renaming?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118507