[libcxx-commits] [PATCH] D118736: [libc++] Guard std::ranges under _LIBCPP_HAS_NO_INCOMPLETE_RANGES.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

I am somewhat worried about this. On the one hand this is the "correct" thing to do, however I am concerned that this could actually end up removing some features we've shipped in LLVM 13 (accidentally), and hence break users.

So far, I get the feeling that we've (mistakenly) used `_LIBCPP_HAS_NO_INCOMPLETE_RANGES` as a proxy for something closer to "no ranges functions and types", not for "nothing in the ranges namespace". So, for example, we've been happily providing concepts that are related to ranges (which as a result makes it possible to provide stuff like `span` constructors). I suspect we'd have to touch way fewer files if we still allowed a couple of ranges-related functionality to creep in despite `_LIBCPP_HAS_NO_INCOMPLETE_RANGES` being defined, but I am having trouble defining what should and should not be guarded precisely. This is something that's harder to do in hindsight -- but in our defence, that's one of the first time trying to ship such a huge feature that spreads over months and months of development, so hiccups were to expect.

I'm not really sure how to cut the cake, but perhaps I'd try to only remove:

- the content in `__algorithm`, `__filesystem`, `__functional`
- "concrete types" in `__iterator`, like `__iterator/common_iterator.h`, `counted_iterator.h`, etc.
- the uninitialized algorithms
- the "concrete views" in `__ranges/`, like `common_view.h`, `copyable_box.h`, `drop_view.h`, etc.

In particular, that should leave us with most of the concepts defined unless I'm mistaken. And those concepts should be reasonable stable AFAICT, and they will enable most of the important stuff like `span` and `string_view`. What do you think?



================
Comment at: libcxx/include/__iterator/concepts.h:29
 
-#if !defined(_LIBCPP_HAS_NO_CONCEPTS)
+#if !defined(_LIBCPP_HAS_NO_CONCEPTS) && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
 
----------------
Geez, this one really hurts. We're so close to being able to provide these concepts. Are we only missing `iter_move`?


================
Comment at: libcxx/include/span:213
 
-#if !defined(_LIBCPP_HAS_NO_CONCEPTS)
+#if !defined(_LIBCPP_HAS_NO_CONCEPTS) && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
     template <class _It,
----------------
This one hurts too, because that's amongst the only ways we have to construct a `span`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118736