[libcxx-commits] [PATCH] D106763: [libc++][RFC] Disable incomplete library features.

Tue Jul 27 14:07:23 PDT 2021

Mordante marked an inline comment as done.
Mordante added a comment.

In D106763#2908310 <https://reviews.llvm.org/D106763#2908310>, @Quuxplusone wrote:

> FWIW, my general reaction is that this patch does more harm (to end-users) than good. But (1) it's above my pay grade, and (2) from the end-user's perspective, Clang 13 won't be any //less// featureful than Clang 12; it'll just be more self-deprecatingly whiny about its continued lack of features.

It makes it possible for vendors to avoid shipping ABI unstable features to their customers. This gives us the freedom the break the ABI in our `<ranges>` implementation when that gives us the opportunity to create a better implementation.

> Consider that `__has_include(<format>)` will now return the "wrong" value; it would be better and simpler if you could just figure out a way to stop the build system from installing that header in `include/c++/v1/` at all. However, this is not a new problem: e.g., `__has_include(<variant>)` already has the "wrong" value in C++03/11/14 mode; we definitely have no way to fix that.

IMO `__has_include(<variant>)` is the wrong tool for the job. If a user wants to to test the status of the headers they can use `__has_include(<version>)` and when that's available test the appropriate feature-test macro.

================
Comment at: libcxx/include/format:64
+#if defined(_LIBCPP_HAS_NO_INCOMPLETE_FORMAT)
+# error "The Format library is not supported by this configuration of libc++"
+#endif
----------------
Quuxplusone wrote:
> Mordante wrote:
> > ldionne wrote:
> > > Maybe this should say something like `The Format library is not complete and stable yet, so this configuration of libc++ does not allow its usage.`. Or anything else that explains a bit _why_ it's not supported (otherwise users tend to be angry).
> > I'm not objecting against making the text a bit more verbose. I just looked at what similar code did. I'll make a separate patch to improve these places.
> `#error "This configuration of libc++ was built without support for C++20 <format> because its ABI is not yet stable."`
> and below
> `#error "This configuration of libc++ was built without support for C++20 <ranges> because its ABI is not yet stable."`
I read this message after committing.
I settled with a slightly different wording with @ldionne, it now shows which configuration option disabled the header.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106763