[libc++] Make future_error constructor standard-compliant

This patch removes the non compliant constructor of std::future_error
and adds the standards compliant constructor in C++17 instead.

Note that we can't support the constructor as an extension in all
standard modes because it uses delegating constructors, which require
C++11. We could in theory support the constructor as an extension in
C++11 and C++14 only, however I believe it is acceptable not to do that
since I expect the breakage from this patch will be minimal.

If it turns out that more code than we expect is broken by this, we can
reconsider that decision.

This was found during D99515.

Differential Revision: https://reviews.llvm.org/D99567
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>

libcxx/docs/ReleaseNotes/18.rst

@@ -87,6 +87,9 @@ Deprecations and Removals
   warning). ``_LIBCPP_ENABLE_ASSERTIONS`` will be removed entirely in the next release and setting it will become an
   error. See :ref:`the hardening documentation <using-hardening-modes>` for more details.
 
+- The non-conforming constructor ``std::future_error(std::error_code)`` has been removed. Please use the
+  ``std::future_error(std::future_errc)`` constructor provided in C++17 instead.
+
 Upcoming Deprecations and Removals
 ----------------------------------
 

libcxx/include/future

@@ -44,14 +44,15 @@ error_condition make_error_condition(future_errc e) noexcept;
 
 const error_category& future_category() noexcept;
 
-class future_error
-    : public logic_error
-{
+class future_error : public logic_error {
 public:
-    future_error(error_code ec);  // exposition only
-    explicit future_error(future_errc); // C++17
+    explicit future_error(future_errc e); // since C++17
+
     const error_code& code() const noexcept;
     const char*       what() const noexcept;
+
+private:
+    error_code ec_;             // exposition only
 };
 
 template <class R>
@@ -516,12 +517,25 @@ make_error_condition(future_errc __e) _NOEXCEPT
     return error_condition(static_cast<int>(__e), future_category());
 }
 
+_LIBCPP_NORETURN inline _LIBCPP_HIDE_FROM_ABI
+#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
+_LIBCPP_AVAILABILITY_FUTURE_ERROR
+#endif
+void __throw_future_error(future_errc __ev);
+
 class _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_AVAILABILITY_FUTURE_ERROR future_error
     : public logic_error
 {
     error_code __ec_;
+
+    future_error(error_code);
+    friend void __throw_future_error(future_errc);
+    template <class> friend class promise;
+
 public:
-    future_error(error_code __ec);
+#if _LIBCPP_STD_VER >= 17
+    _LIBCPP_HIDE_FROM_ABI explicit future_error(future_errc __ec) : future_error(std::make_error_code(__ec)) {}
+#endif
 
     _LIBCPP_INLINE_VISIBILITY
     const error_code& code() const _NOEXCEPT {return __ec_;}
@@ -530,10 +544,7 @@ public:
     ~future_error() _NOEXCEPT override;
 };
 
-_LIBCPP_NORETURN inline _LIBCPP_INLINE_VISIBILITY
-#ifndef _LIBCPP_HAS_NO_EXCEPTIONS
-_LIBCPP_AVAILABILITY_FUTURE_ERROR
-#endif
+// Declared above std::future_error
 void __throw_future_error(future_errc __ev)
 {
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
@@ -1359,9 +1370,7 @@ promise<_Rp>::~promise()
     if (__state_)
     {
         if (!__state_->__has_value() && __state_->use_count() > 1)
-            __state_->set_exception(make_exception_ptr(
-                      future_error(make_error_code(future_errc::broken_promise))
-                                                      ));
+            __state_->set_exception(make_exception_ptr(future_error(make_error_code(future_errc::broken_promise))));
         __state_->__release_shared();
     }
 }
@@ -1502,9 +1511,7 @@ promise<_Rp&>::~promise()
     if (__state_)
     {
         if (!__state_->__has_value() && __state_->use_count() > 1)
-            __state_->set_exception(make_exception_ptr(
-                      future_error(make_error_code(future_errc::broken_promise))
-                                                      ));
+            __state_->set_exception(make_exception_ptr(future_error(make_error_code(future_errc::broken_promise))));
         __state_->__release_shared();
     }
 }

libcxx/src/future.cpp

@@ -204,9 +204,7 @@ promise<void>::~promise()
     {
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
         if (!__state_->__has_value() && __state_->use_count() > 1)
-            __state_->set_exception(make_exception_ptr(
-                      future_error(make_error_code(future_errc::broken_promise))
-                                                      ));
+            __state_->set_exception(make_exception_ptr(future_error(future_errc::broken_promise)));
 #endif // _LIBCPP_HAS_NO_EXCEPTIONS
         __state_->__release_shared();
     }

libcxx/test/std/thread/futures/futures.future_error/code.pass.cpp

@@ -9,49 +9,43 @@
 // UNSUPPORTED: no-threads
 
 // <future>
-
+//
 // class future_error
-//     future_error(error_code __ec);  // exposition only
-//     explicit future_error(future_errc _Ev) : __ec_(make_error_code(_Ev)) {} // C++17
-
-// const error_code& code() const throw();
+//
+// const error_code& code() const noexcept;
 
-#include <future>
 #include <cassert>
+#include <future>
+#include <utility>
 
 #include "test_macros.h"
 
-int main(int, char**)
-{
-    {
-        std::error_code ec = std::make_error_code(std::future_errc::broken_promise);
-        std::future_error f(ec);
-        assert(f.code() == ec);
-    }
-    {
-        std::error_code ec = std::make_error_code(std::future_errc::future_already_retrieved);
-        std::future_error f(ec);
-        assert(f.code() == ec);
-    }
-    {
-        std::error_code ec = std::make_error_code(std::future_errc::promise_already_satisfied);
-        std::future_error f(ec);
-        assert(f.code() == ec);
-    }
-    {
-        std::error_code ec = std::make_error_code(std::future_errc::no_state);
-        std::future_error f(ec);
-        assert(f.code() == ec);
-    }
-#if TEST_STD_VER > 14
-    {
-        std::future_error f(std::future_errc::broken_promise);
-        assert(f.code() == std::make_error_code(std::future_errc::broken_promise));
-    }
-    {
-        std::future_error f(std::future_errc::no_state);
-        assert(f.code() == std::make_error_code(std::future_errc::no_state));
-    }
+int main(int, char**) {
+  ASSERT_NOEXCEPT(std::declval<std::future_error const&>().code());
+  ASSERT_SAME_TYPE(decltype(std::declval<std::future_error const&>().code()), std::error_code const&);
+
+  // Before C++17, we can't construct std::future_error directly in a standards-conforming way
+#if TEST_STD_VER >= 17
+  {
+    std::future_error const f(std::future_errc::broken_promise);
+    std::error_code const& code = f.code();
+    assert(code == std::make_error_code(std::future_errc::broken_promise));
+  }
+  {
+    std::future_error const f(std::future_errc::future_already_retrieved);
+    std::error_code const& code = f.code();
+    assert(code == std::make_error_code(std::future_errc::future_already_retrieved));
+  }
+  {
+    std::future_error const f(std::future_errc::promise_already_satisfied);
+    std::error_code const& code = f.code();
+    assert(code == std::make_error_code(std::future_errc::promise_already_satisfied));
+  }
+  {
+    std::future_error const f(std::future_errc::no_state);
+    std::error_code const& code = f.code();
+    assert(code == std::make_error_code(std::future_errc::no_state));
+  }
 #endif
 
   return 0;

libcxx/test/std/thread/futures/futures.future_error/ctor.pass.cpp

@@ -0,0 +1,44 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: no-threads
+// UNSUPPORTED: c++03, c++11, c++14
+
+// <future>
+
+// class future_error
+//
+// explicit future_error(future_errc e); // since C++17
+
+#include <cassert>
+#include <future>
+#include <type_traits>
+
+int main(int, char**) {
+  {
+    std::future_error f(std::future_errc::broken_promise);
+    assert(f.code() == std::make_error_code(std::future_errc::broken_promise));
+  }
+  {
+    std::future_error f(std::future_errc::future_already_retrieved);
+    assert(f.code() == std::make_error_code(std::future_errc::future_already_retrieved));
+  }
+  {
+    std::future_error f(std::future_errc::promise_already_satisfied);
+    assert(f.code() == std::make_error_code(std::future_errc::promise_already_satisfied));
+  }
+  {
+    std::future_error f(std::future_errc::no_state);
+    assert(f.code() == std::make_error_code(std::future_errc::no_state));
+  }
+
+  // Make sure the constructor is explicit
+  static_assert(!std::is_convertible_v<std::future_errc, std::future_error>);
+
+  return 0;
+}

libcxx/test/std/thread/futures/futures.future_error/types.pass.cpp → libcxx/test/std/thread/futures/futures.future_error/types.compile.pass.cpp

@@ -15,12 +15,4 @@
 #include <future>
 #include <type_traits>
 
-#include "test_macros.h"
-
-int main(int, char**)
-{
-    static_assert((std::is_convertible<std::future_error*,
-                                       std::logic_error*>::value), "");
-
-  return 0;
-}
+static_assert(std::is_convertible<std::future_error*, std::logic_error*>::value, "");

libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp

@@ -13,39 +13,53 @@
 //
 // XFAIL: stdlib=apple-libc++ && target={{.+}}-apple-macosx10.{{9|10|11}}
 
-// <future>
+// VC Runtime's std::exception::what() method is not marked as noexcept, so
+// this fails.
+// UNSUPPORTED: target=x86_64-pc-windows-msvc
 
+// <future>
+//
 // class future_error
+//
+// const char* what() const noexcept;
 
-// const char* what() const throw();
-
-#include <future>
-#include <cstring>
 #include <cassert>
+#include <future>
+#include <string_view>
+#include <utility>
 
 #include "test_macros.h"
 
-int main(int, char**)
-{
-    {
-        std::future_error f(std::make_error_code(std::future_errc::broken_promise));
-        LIBCPP_ASSERT(std::strcmp(f.what(), "The associated promise has been destructed prior "
-                      "to the associated state becoming ready.") == 0);
-    }
-    {
-        std::future_error f(std::make_error_code(std::future_errc::future_already_retrieved));
-        LIBCPP_ASSERT(std::strcmp(f.what(), "The future has already been retrieved from "
-                      "the promise or packaged_task.") == 0);
-    }
-    {
-        std::future_error f(std::make_error_code(std::future_errc::promise_already_satisfied));
-        LIBCPP_ASSERT(std::strcmp(f.what(), "The state of the promise has already been set.") == 0);
-    }
-    {
-        std::future_error f(std::make_error_code(std::future_errc::no_state));
-        LIBCPP_ASSERT(std::strcmp(f.what(), "Operation not permitted on an object without "
-                      "an associated state.") == 0);
-    }
+int main(int, char**) {
+  ASSERT_NOEXCEPT(std::declval<std::future_error const&>().what());
+  ASSERT_SAME_TYPE(decltype(std::declval<std::future_error const&>().what()), char const*);
+
+  // Before C++17, we can't construct std::future_error directly in a standards-conforming way
+#if TEST_STD_VER >= 17
+  {
+    std::future_error const f(std::future_errc::broken_promise);
+    char const* what = f.what();
+    LIBCPP_ASSERT(what == std::string_view{"The associated promise has been destructed prior "
+                                           "to the associated state becoming ready."});
+  }
+  {
+    std::future_error f(std::future_errc::future_already_retrieved);
+    char const* what = f.what();
+    LIBCPP_ASSERT(what == std::string_view{"The future has already been retrieved from "
+                                           "the promise or packaged_task."});
+  }
+  {
+    std::future_error f(std::future_errc::promise_already_satisfied);
+    char const* what = f.what();
+    LIBCPP_ASSERT(what == std::string_view{"The state of the promise has already been set."});
+  }
+  {
+    std::future_error f(std::future_errc::no_state);
+    char const* what = f.what();
+    LIBCPP_ASSERT(what == std::string_view{"Operation not permitted on an object without "
+                                           "an associated state."});
+  }
+#endif
 
   return 0;
 }

libcxx/utils/data/ignore_format.txt

@@ -5614,9 +5614,6 @@ libcxx/test/std/thread/futures/futures.errors/equivalent_int_error_condition.pas
 libcxx/test/std/thread/futures/futures.errors/future_category.pass.cpp
 libcxx/test/std/thread/futures/futures.errors/make_error_code.pass.cpp
 libcxx/test/std/thread/futures/futures.errors/make_error_condition.pass.cpp
-libcxx/test/std/thread/futures/futures.future_error/code.pass.cpp
-libcxx/test/std/thread/futures/futures.future_error/types.pass.cpp
-libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp
 libcxx/test/std/thread/futures/futures.overview/future_errc.pass.cpp
 libcxx/test/std/thread/futures/futures.overview/future_status.pass.cpp
 libcxx/test/std/thread/futures/futures.overview/is_error_code_enum_future_errc.pass.cpp