Get exception ptr CPO #249

Garcia6l20 · 2021-04-04T07:54:22Z

Create new get_exception_ptr CPO allowing users to control how any kind of error type will be internally converted to an std::exception_ptr.

- add get_exception_ptr_test

- add set_error(exception_ptr_convertible) overload in _awaitable_base

examples/linux/io_epoll_test.cpp

examples/linux/io_uring_test.cpp

include/unifex/get_exception_ptr.hpp

include/unifex/await_transform.hpp

include/unifex/get_exception_ptr.hpp

- std::exception_ptr forwarding - std::exception -> std::exception_ptr conversion - tag_invoke(get_exception_ptr, ...) customization points

- make any_sender_of use get_exception_ptr CPO - make async_scope use get_exception_ptr CPO

CMakeLists.txt

include/unifex/get_exception_ptr.hpp

include/unifex/get_stop_token.hpp

include/unifex/get_exception_ptr.hpp

include/unifex/as_exception_ptr.hpp

- some formatting - other pr comments

- update as_exception_ptr_test

include/unifex/get_exception_ptr.hpp

examples/linux/file_coroutines_test.cpp

include/unifex/as_exception_ptr.hpp

include/unifex/await_transform.hpp

include/unifex/as_exception_ptr.hpp

include/unifex/any_sender_of.hpp

examples/linux/file_coroutines_test.cpp

- set_error now uses as_exception_ptr to transform Errors to exception_ptr - rollback: - any_sender_of.hpp - async_scope.hpp - await_transform.hpp - cleanup tag_invocable/nothrow_tag_invocable concepts - file_coroutine_test does not used __FILE__ macro - sync_wait now requires only one set_error(exception_ptr) method

include/unifex/as_exception_ptr.hpp

ericniebler · 2021-05-01T00:07:43Z

include/unifex/as_exception_ptr.hpp

+    tag_invoke(tag_t<as_exception_ptr>, std::error_code&& error) noexcept {
+      return make_exception_ptr(
+          std::system_error{std::forward<std::error_code>(error)});
+    }


I would prefer if this were an overload in _fn rather than a tag_invoke overload at namespace scope.

include/unifex/as_exception_ptr.hpp

include/unifex/receiver_concepts.hpp

ericniebler · 2021-05-01T00:14:06Z

include/unifex/receiver_concepts.hpp

+      (requires (!tag_invocable<_set_error_fn, Receiver, Error>) &&
+         tag_invocable<decltype(as_exception_ptr), Error>)
+    auto operator()(Receiver&& r, Error&& error) const noexcept
+    -> _result_t<Receiver, std::exception_ptr> {


You need to update the _result_t alias to account for this overload. Look how it is done elsewhere with if constexpr in other customization points.

I added a _select function (reproduced from connect CPO), I hope it is fine.

lewissbaker · 2021-04-21T10:47:23Z

include/unifex/receiver_concepts.hpp

+      (requires (!tag_invocable<_set_error_fn, Receiver, Error>) &&
+         tag_invocable<decltype(as_exception_ptr), Error>)


The way you have constrained this requirement means that it will always convert the error to an exception_ptr if it can and the set_error() method can be invoked with an exception_ptr.

I think that what we want instead is to try to invoke set_error() with the provided type and only if there is no tag_invoke() or member function set_error() overload that accepts that type then, if the error type can be converted to exception_ptr and you can invoke set_error (either by tag_invoke or member function) with an exception_ptr then do that as a fallback.

So the order of resolution would be:

tag_invoke w/ original error type

set_error member fn w/ original error type

tag_invoke w/ exception_ptr AND error type is convertible to exception_ptr

set_error member fn w/ exception_ptr AND error type is convertible to exception_ptr

Yes, I missed it.
I added a test case to validate this.

- taking new overload into account

- add as_exception_ptr.set_error test case

lewissbaker

Apologies for taking so long to get back to this.
A couple of additional comments.

If you don't have bandwidth for this just let me know and I'll fix it up.

lewissbaker · 2021-10-23T02:40:14Z

include/unifex/as_exception_ptr.hpp

+      }
+
+      // default std::error_code to std::exception_ptr conversion
+      std::exception_ptr operator()(std::error_code&& error) const noexcept {


I think the std::error_code can be passed by-value here (it is cheap to copy, and passing by-value allows callers to pass lvalues to this function)

Suggested change

std::exception_ptr operator()(std::error_code&& error) const noexcept {

std::exception_ptr operator()(std::error_code error) const noexcept {

lewissbaker · 2021-10-23T03:00:34Z

include/unifex/receiver_concepts.hpp

+      (requires (!tag_invocable<_set_error_fn, Receiver, Error>) AND
+         std::invocable<decltype(&Receiver::set_error), Receiver, Error>)


I think that there is another case that still needs to be handled in here.

At the moment this code has the following order of preference in which it resolves the implementation of unifex::set_error(r, err):

tag_invoke(set_error, r, err) if that is well-formed; otherwise

r.set_error(err) if that is well-formed; otherwise

r.set_error(as_exception_ptr(err)) if that is well-formed; otherwise

set_error(r, err) call is ill-formed

I think that we probably need to have an extra case inserted into here to handle the case where tag_invoke(set_err, r, err) is ill-formed but tag_invoke(set_err, r, as_exception_ptr(err)) is well-formed.
I think this should probably sit in between cases 2 and 3 above, although it's possible you could argue it should go between cases 1 and 2.

This could possibly be handled with a two-stage resolution process using a helper.

struct set_error_fn { private: template(typename Receiver, typename Error) (requires tag_invocable<set_error_fn, Receiver, Error>) static void dispatch(Receiver&& r, Error&& err) noexcept { static_assert(is_nothrow_tag_invocable_v<set_error_fn, Receiver, Error>); static_assert(std::is_void_v<tag_invoke_result_t<set_error_fn, Receiver, Error>>); tag_invoke(set_error_fn{}, (Receiver&&)r, (Error&&)err); } template(typename Receiver, typename Error> (requires (!tag_invocable<set_error_fn, Receiver, Error> AND has_member_set_error<Receiver, Error>) static void dispatch(Receiver&& r, Error&& err) noexcept { static_cast<Receiver&&>(r).set_error(static_cast<Error&&>(err)); } template<typename Receiver, typename Error, typename = void> static constexpr bool can_dispatch_v = false; template<typename Receiver, typename Error> static constexpr bool can_dispatch_v<Receiver, Error, std::void_t<decltype(dispatch(std::declval<Receiver>(), std::declval<Error>()))>> = true; public: template(typename Receiver, typename Error) (requires can_dispatch_v<Receiver, Error>) void operator()(Receiver&& r, Error&& err) const noexcept { dispatch((Receiver&&)r, (Error&&)err); } template(typename Receiver, typename Error) (requires (!can_dispatch_v<Receiver, Error>) AND callable<decltype(as_exception_ptr), Error> AND can_dispatch_v<Receiver, std::exception_ptr>) void operator()(Receiver&& r, Error&& err) const noexcept { dispatch((Receiver&&)r, as_exception_ptr((Error&&)err)); } };

Are we allowed to use syntax like if constexpr ( requires { is_this_ill_formed(error); }) {...} else {...} ? Wich is OK in GCC11 and latest MSVC. It would greatly simplify this kind of dispatching stuff.

After double-check this syntax does not work on MSVC (I was actualy using clang-cl)

lewissbaker · 2021-10-23T03:33:36Z

include/unifex/tag_invoke.hpp

+  UNIFEX_CONCEPT nothrow_tag_invocable =
+      (is_nothrow_tag_invocable_v<CPO, Args...>);


Usually the nothrow type-trait queries haven't been represented as concepts but rather as boolean type-traits.
I'm not sure exactly why that is, though. Maybe @ericniebler has some thoughts?

Precedence for one. The fact that it's all syntax and no semantics for another. Neither of which are particularly compelling reasons.

ericniebler · 2022-06-29T20:04:34Z

include/unifex/as_exception_ptr.hpp

+      }
+
+      // default std::error_code to std::exception_ptr conversion
+      std::exception_ptr operator()(std::error_code&& error) const noexcept {


Suggested change

std::exception_ptr operator()(std::error_code&& error) const noexcept {

std::exception_ptr operator()(std::error_code error) const noexcept {

ericniebler · 2022-07-03T23:57:39Z

include/unifex/receiver_concepts.hpp

    template(typename Receiver, typename Error)
-      (requires (!tag_invocable<_set_error_fn, Receiver, Error>))
+      (requires (!tag_invocable<_set_error_fn, Receiver, Error>) AND
+         std::invocable<decltype(&Receiver::set_error), Receiver, Error>)


std::invocable<decltype(&Receiver::set_error), Receiver, Error>

This is not the right way to check for a .set_error(Error) member function. Consider that Receiver::set_error could be a template. In that case, &Receiver::set_error is ill-formed. Check how such constraints are checked elsewhere; you'll need to test the validity of the expression rec.set_error(err) in a SFINAE context (or with a concept) to get a correct answer.

ericniebler · 2022-07-03T23:58:17Z

include/unifex/receiver_concepts.hpp

+    template(typename Receiver, typename Error)
+      (requires (!tag_invocable<_set_error_fn, Receiver, Error>) AND
+        (!std::invocable<decltype(&Receiver::set_error), Receiver, Error>) AND
+         std::invocable<decltype(&Receiver::set_error), Receiver, std::exception_ptr> AND


Same thing here about testing for a .set_error(e) member function.

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 4, 2021

Garcia6l20 force-pushed the get_exception_ptr branch 2 times, most recently from 2beb643 to 859347c Compare April 4, 2021 08:26

Garcia6l20 added 2 commits April 4, 2021 11:39

get_exception_ptr CPO

f8d2291

- add get_exception_ptr_test

use get_exception_ptr in _awaitable_base

cfb4ff0

- add set_error(exception_ptr_convertible) overload in _awaitable_base

ericniebler requested changes Apr 6, 2021

View reviewed changes

Garcia6l20 force-pushed the get_exception_ptr branch 3 times, most recently from 1a7c082 to 32ba5da Compare April 8, 2021 13:24

Garcia6l20 added 3 commits April 8, 2021 16:18

fix c++17 compilation

812e58a

widening get_exception_ptr CPO

bb8f46b

- std::exception_ptr forwarding - std::exception -> std::exception_ptr conversion - tag_invoke(get_exception_ptr, ...) customization points

generic set_error handling

ec30d89

- make any_sender_of use get_exception_ptr CPO - make async_scope use get_exception_ptr CPO

Garcia6l20 requested a review from ericniebler April 8, 2021 17:43

ericniebler requested changes Apr 12, 2021

View reviewed changes

Garcia6l20 force-pushed the get_exception_ptr branch from 32ba5da to e4cc6f6 Compare April 15, 2021 20:19

Garcia6l20 commented Apr 15, 2021

View reviewed changes

include/unifex/as_exception_ptr.hpp Show resolved Hide resolved

Garcia6l20 added 3 commits April 16, 2021 00:09

add coroutine file test

2461d80

rename get_exception_ptr to as_exception_ptr

e4cc6f6

- some formatting - other pr comments

add nothrow_tag_invokable concept

aca13cb

- update as_exception_ptr_test

lewissbaker reviewed Apr 16, 2021

View reviewed changes

Garcia6l20 force-pushed the get_exception_ptr branch from 0b20553 to 532a343 Compare April 21, 2021 05:23

ericniebler requested changes May 1, 2021

View reviewed changes

lewissbaker reviewed May 1, 2021

View reviewed changes

Garcia6l20 added 4 commits May 18, 2021 14:27

use overload for default conversion

ecad7f0

avoid ambiguous overloading

d326b09

use AND macro

9b57372

update set_error_fn_::_result_t

8d75bad

- taking new overload into account

Garcia6l20 requested review from ericniebler and lewissbaker May 18, 2021 16:56

fix set_error always calling as_exception_ptr

a19d14a

- add as_exception_ptr.set_error test case

lewissbaker reviewed Oct 24, 2021

View reviewed changes

ericniebler requested changes Jul 3, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get exception ptr CPO #249

Get exception ptr CPO #249

Garcia6l20 commented Apr 4, 2021

ericniebler May 1, 2021

ericniebler May 1, 2021

Garcia6l20 May 18, 2021

lewissbaker Apr 21, 2021

Garcia6l20 May 18, 2021

lewissbaker left a comment

lewissbaker Oct 23, 2021

ericniebler Jun 29, 2022

lewissbaker Oct 23, 2021

lewissbaker Oct 23, 2021

Garcia6l20 Oct 27, 2021

Garcia6l20 Oct 27, 2021

lewissbaker Oct 23, 2021

ericniebler Oct 24, 2021

ericniebler Jun 29, 2022

ericniebler Jul 3, 2022

ericniebler Jul 3, 2022

		(requires (!tag_invocable<_set_error_fn, Receiver, Error>) &&
		tag_invocable<decltype(as_exception_ptr), Error>)

	std::exception_ptr operator()(std::error_code&& error) const noexcept {
	std::exception_ptr operator()(std::error_code error) const noexcept {

		UNIFEX_CONCEPT nothrow_tag_invocable =
		(is_nothrow_tag_invocable_v<CPO, Args...>);

Get exception ptr CPO #249

Are you sure you want to change the base?

Get exception ptr CPO #249

Conversation

Garcia6l20 commented Apr 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lewissbaker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment