[libc++] Keep unary_function and binary_function in C++17 for one more release

[ldionne]

Summary

In LLVM 15, we added the deprecation markup for unary_function and
binary_function for >= C++11, and we also removed it for >= C++17.
While this is in accordance with the Standard, it's also a bit quick
for our users, since there was no release in which the classes were
marked as deprecated before their removal.

We noticed widespread breakage due to this, and after months of trying
to fix downstream failures, I am coming to the conclusion that users
will be better served if we give them one release where unary_function
is deprecated but still provided even in >= C++17.

[ldionne]

If accepted, this patch would only go towards the LLVM 15 branch. The behavior for LLVM 16 is already the way we want it.

Note that I will be shipping this patch in Apple's libc++ no matter what we do here, simply because we've noticed too many failures caused by this and we need more time to address them. I think the greater community would also benefit from a 1-release grace period where we produce a deprecation warning but don't break people's code, but I can also live with the hard stance we've taken so far.

[Mordante]

In D134473#3809725, @ldionne wrote:

If accepted, this patch would only go towards the LLVM 15 branch. The behavior for LLVM 16 is already the way we want it.

Note that I will be shipping this patch in Apple's libc++ no matter what we do here, simply because we've noticed too many failures caused by this and we need more time to address them. I think the greater community would also benefit from a 1-release grace period where we produce a deprecation warning but don't break people's code, but I can also live with the hard stance we've taken so far.

Mainly for my curiosity what is the issue to use _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION at Apple?

If this is an issue for Apple, will that also be an issue for "less connected" companies? I mainly ask since I don't know how many other (large) companies are affected by this libc++ change. (I know Google likes to follow HEAD closely they are used to adapt to upstream changes quickly, other companies are probably not as agile.)

I love to deprecate things and remove cruft, but in the end libc++ should serve the needs of its customers. So if the current deprecation/removal policy is an issue, maybe we should reconsider our policy.

To me if this breaks Apple +1 to backport it for all our users.

LGTM I leave the final approval to @philnik .

libcxx/docs/ReleaseNotes.rst

147

This is the typical wording we use.

[philnik]

LGTM with green CI and comments addressed.

libcxx/docs/ReleaseNotes.rst

147

Could we move the in the next release part into it's own sentence? Something like ... and above. This will be enforced in LLVM 16.. I find the current wording quite confusing.

libcxx/include/__functional/binary_function.h

26 ↗

(On Diff #462278)

Can we get a message here? Something like binary_function will be removed in LLVM 16 from C++17 and above. The wording isn't great, but it's nicer than having to dig through release notes to find out. Same for unary_function.

[ldionne]

In D134473#3812335, @Mordante wrote:

In D134473#3809725, @ldionne wrote:

If accepted, this patch would only go towards the LLVM 15 branch. The behavior for LLVM 16 is already the way we want it.

Note that I will be shipping this patch in Apple's libc++ no matter what we do here, simply because we've noticed too many failures caused by this and we need more time to address them. I think the greater community would also benefit from a 1-release grace period where we produce a deprecation warning but don't break people's code, but I can also live with the hard stance we've taken so far.

Mainly for my curiosity what is the issue to use _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION at Apple?

If this is an issue for Apple, will that also be an issue for "less connected" companies? I mainly ask since I don't know how many other (large) companies are affected by this libc++ change. (I know Google likes to follow HEAD closely they are used to adapt to upstream changes quickly, other companies are probably not as agile.)

The issue is not to use _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION itself. It's with coordinating tens of teams to do it. At the end of the day, it's much easier if folks get a deprecation warning and then fix it over the next ~N months at their own pace. If I try to put myself in someone else's shoes, they could be in a situation where they have slightly older dependencies (e.g. older open-source projects) that still use unary_function and binary_function. They then face the dilemma: either they can't compile as C++17, or they can't update to a new Clang/libc++, or they have to update their code and their dependencies to remove references to xx_function (or add _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION to the build instructions). Depending on the setup, this can range from "trivial" (e.g. what I would imagine for Google) to annoying but achievable (for Apple) to unachievable in less than ~6 months (for IDK who). Since we don't lose very much, I think we might as well try to cater to everyone on that spectrum in this case.

I love to deprecate things and remove cruft, but in the end libc++ should serve the needs of its customers. So if the current deprecation/removal policy is an issue, maybe we should reconsider our policy.

I don't think our policy is an issue. I simply think we waited far too long to add the deprecation warnings for those two widely used types, and then we decided to both deprecate AND remove them in the same release. In hindsight, this was a poor decision and I think we should have deprecated it first, and then removed it. If anything is to change with our policy, I think it would be reasonable to never both deprecate and remove an API in a single release, since it can cause these issues. Of course, not all APIs are as widely used as binary_function and unary_function (yes, you'd be surprised how omnipresent they are).

libcxx/docs/ReleaseNotes.rst

147

Yeah, you're right. I reworked the wording.

libcxx/include/__functional/binary_function.h

26 ↗

(On Diff #462278)

Upon considering this again, I think this part of the diff should not exist, just like it doesn't on our main branch.

[ldionne]

Address comments

[thieta]

IIRC this was closed right @ldionne so this patch can be closed?

[ldionne]

Yes, this was landed on release/15.x (and only there) as a cherry-pick as 8d802f78fa8c91707ecea1ae4ab141ba835429bc.

libcxx/docs/ReleaseNotes.rst

 - Setting a custom debug handler with ``std::__libcpp_debug_function`` is not
   supported anymore. Please migrate to using the new support for
   :ref:`assertions <assertions-mode>` instead.
 
 - ``std::function`` has been removed in C++03. If you are using it, please remove usages
   or upgrade to C++11 or later. It is possible to re-enable ``std::function`` in C++03 by defining
   ``_LIBCPP_ENABLE_CXX03_FUNCTION``. This option will be removed in LLVM 16.
 
-- ``unary_function`` and ``binary_function`` are no longer available in C++17 and C++20.
-  They can be re-enabled by defining ``_LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION``.
-  They are also marked as ``[[deprecated]]`` in C++11 and later. To disable deprecation warnings
-  you have to define ``_LIBCPP_DISABLE_DEPRECATION_WARNINGS``. Note that this disables
-  all deprecation warnings.
+- ``unary_function`` and ``binary_function`` are now marked as ``[[deprecated]]`` in C++11 and later.
+  Deprecation warnings can be disabled by defining ``_LIBCPP_DISABLE_DEPRECATION_WARNINGS``, however
+  this disables all deprecation warnings, not only those for ``unary_function`` and ``binary_function``.
+  Also note that starting in LLVM 16, ``unary_function`` and ``binary_function`` will be removed entirely
+  (not only deprecated) in C++17 and above, as mandated by the Standard.
 
 - The contents of ``<codecvt>``, ``wstring_convert`` and ``wbuffer_convert`` have been marked as deprecated.
   To disable deprecation warnings you have to define ``_LIBCPP_DISABLE_DEPRECATION_WARNINGS``. Note that this
   disables all deprecation warnings.
 
 - The ``_LIBCPP_DISABLE_EXTERN_TEMPLATE`` macro is not honored anymore when defined by
   users of libc++. Instead, users not wishing to take a dependency on libc++ should link
   against the static version of libc++, which will result in no dependency being

libcxx/include/__config

 #  if defined(_LIBCPP_ENABLE_CXX17_REMOVED_FEATURES)
 #    define _LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR
 #    define _LIBCPP_ENABLE_CXX17_REMOVED_BINDERS
 #    define _LIBCPP_ENABLE_CXX17_REMOVED_RANDOM_SHUFFLE
 #    define _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS
 #    define _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION
 #  endif // _LIBCPP_ENABLE_CXX17_REMOVED_FEATURES
 
+// Leave the deprecation notices in by default, but don't remove unary_function and
+// binary_function entirely just yet. That way, folks will have one release to act
+// on the deprecation warnings.
+#  ifndef _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION
+#    define _LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION
+#  endif
+
 #  if defined(_LIBCPP_ENABLE_CXX20_REMOVED_FEATURES)
 #    define _LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_MEMBERS
 #    define _LIBCPP_ENABLE_CXX20_REMOVED_ALLOCATOR_VOID_SPECIALIZATION
 #    define _LIBCPP_ENABLE_CXX20_REMOVED_BINDER_TYPEDEFS
 #    define _LIBCPP_ENABLE_CXX20_REMOVED_NEGATORS
 #    define _LIBCPP_ENABLE_CXX20_REMOVED_RAW_STORAGE_ITERATOR
 #    define _LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS
 #  endif // _LIBCPP_ENABLE_CXX20_REMOVED_FEATURES

libcxx/test/libcxx/depr/depr.func.adaptor.typedefs/typedefs.depr_in_cxx17.verify.cpp

 
 #include <functional>
 #include <map>
 #include <memory>
 #include <optional>
 #include <utility>
 #include "test_macros.h"
 
+// This test is disabled in LLVM 15 because we enable the removed
+// unary_function and binary_function classes by default, which means
+// that we also don't produce deprecation warnings for using their members.
+
+// expected-no-diagnostics
+#if 0
 void test_functional()
 {
     {
         using T = std::plus<int>;
         T::result_type a;          // expected-warning {{is deprecated}}
         T::first_argument_type b;  // expected-warning {{is deprecated}}
         T::second_argument_type c; // expected-warning {{is deprecated}}
         (void)a;
void test_map()
         T::result_type a;          // expected-warning {{is deprecated}}
         T::first_argument_type b;  // expected-warning {{is deprecated}}
         T::second_argument_type c; // expected-warning {{is deprecated}}
         (void)a;
         (void)b;
         (void)c;
     }
 }
+#endif