[libcxx-commits] [libcxx] d2cb198 - [libc++] Make future_error constructor standard-compliant

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Thu Oct 5 06:12:27 PDT 2023


Author: Marek Kurdej
Date: 2023-10-05T09:11:49-04:00
New Revision: d2cb198f25c82bf77bb113763771590cc79a21a4

URL: https://github.com/llvm/llvm-project/commit/d2cb198f25c82bf77bb113763771590cc79a21a4
DIFF: https://github.com/llvm/llvm-project/commit/d2cb198f25c82bf77bb113763771590cc79a21a4.diff

LOG: [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 at gmail.com>

Added: 
    libcxx/test/std/thread/futures/futures.future_error/ctor.pass.cpp
    libcxx/test/std/thread/futures/futures.future_error/types.compile.pass.cpp

Modified: 
    libcxx/docs/ReleaseNotes/18.rst
    libcxx/include/future
    libcxx/src/future.cpp
    libcxx/test/std/thread/futures/futures.future_error/code.pass.cpp
    libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp
    libcxx/utils/data/ignore_format.txt

Removed: 
    libcxx/test/std/thread/futures/futures.future_error/types.pass.cpp


################################################################################
diff  --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 0a5e99984fa75e9..640e6ce0769582c 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/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
 ----------------------------------
 

diff  --git a/libcxx/include/future b/libcxx/include/future
index 273e4175e604bd9..f372b8e6ad5d8d8 100644
--- a/libcxx/include/future
+++ b/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();
     }
 }

diff  --git a/libcxx/src/future.cpp b/libcxx/src/future.cpp
index fa1004e13571287..3383b506a2fab3d 100644
--- a/libcxx/src/future.cpp
+++ b/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();
     }

diff  --git a/libcxx/test/std/thread/futures/futures.future_error/code.pass.cpp b/libcxx/test/std/thread/futures/futures.future_error/code.pass.cpp
index 99d2ade796e4589..22d74bba803d9b8 100644
--- a/libcxx/test/std/thread/futures/futures.future_error/code.pass.cpp
+++ b/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;

diff  --git a/libcxx/test/std/thread/futures/futures.future_error/ctor.pass.cpp b/libcxx/test/std/thread/futures/futures.future_error/ctor.pass.cpp
new file mode 100644
index 000000000000000..bce34105a50922b
--- /dev/null
+++ b/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;
+}

diff  --git a/libcxx/test/std/thread/futures/futures.future_error/types.pass.cpp b/libcxx/test/std/thread/futures/futures.future_error/types.compile.pass.cpp
similarity index 72%
rename from libcxx/test/std/thread/futures/futures.future_error/types.pass.cpp
rename to libcxx/test/std/thread/futures/futures.future_error/types.compile.pass.cpp
index 8e77c01e73cea40..478f0c2a676aa8b 100644
--- a/libcxx/test/std/thread/futures/futures.future_error/types.pass.cpp
+++ b/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, "");

diff  --git a/libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp b/libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp
index 48ae35b6eb883dd..fba2bd80d7f89a0 100644
--- a/libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp
+++ b/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;
 }

diff  --git a/libcxx/utils/data/ignore_format.txt b/libcxx/utils/data/ignore_format.txt
index 50bb890ed748f1b..24e207169c81481 100644
--- a/libcxx/utils/data/ignore_format.txt
+++ b/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


        


More information about the libcxx-commits mailing list