[libcxx-commits] [PATCH] D99567: [libc++] Make future_error constructor standard-compliant.

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Mar 30 07:19:47 PDT 2021

Quuxplusone added inline comments.

Comment at: libcxx/include/future:505-514
+#if _LIBCPP_STD_VER <= 14
     future_error(error_code __ec);
-#if _LIBCPP_STD_VERS > 14
-    explicit future_error(future_errc _Ev) : logic_error(), __ec_(make_error_code(_Ev)) {}
+#if _LIBCPP_STD_VER > 14
`make_error_code` should remain called via ADL; it's a customization point.
If your de-ADL'ed version passes the test suite, then we need more tests around this.

The pre-existing ctor sets up `logic_error(__ec.message())` appropriately, so we want to find a way of duplicating that behavior. Also, I recommend adding a test for the message under `test/libcxx/`!

I'm suggesting that there be a private constructor from `error_code` (to avoid calling the user's `make_error_code` twice), but that it be tagged with a private tag to prevent people from stumbling into it accidentally. Your current patch leaves the existing ctor in place, but marks it private in C++17 and later; I think that's more likely to lead to confusion.
struct MyErrC { operator future_errc() const; operator error_code() const; };
auto err = std::future_error(MyErrC());

Comment at: libcxx/include/future:1350
-                      future_error(make_error_code(future_errc::broken_promise))
+                      future_error(future_errc::broken_promise)
Maybe pull up the `));` from the next line while you're here. Ditto line 1493.

Comment at: libcxx/test/std/thread/futures/futures.future_error/ctor.fail.cpp:26
+  std::error_code ec = std::make_error_code(std::future_errc::no_state);
+  std::future_error f(ec); // expected-error {{calling a private constructor}}
Per my suggestion above, I think "accidentally stumbling into a private constructor" is never a good thing. This specific error message indicates that we should do better.

Comment at: libcxx/test/std/thread/futures/futures.future_error/what.pass.cpp:33
-        std::future_error f(std::make_error_code(std::future_errc::broken_promise));
+        std::future_error f(std::future_errc::broken_promise);
         LIBCPP_ASSERT(std::strcmp(f.what(), "The associated promise has been destructed prior "
Good; but how come these lines work in C++14 mode? Implicit conversion from `future_errc` to `error_code`, right?
In that case, your changes in `test/std/thread/futures/futures.future_error/code.pass.cpp` are too conservative; you can eliminate the `#else` condition and just run those tests in all modes.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list