Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get exception ptr CPO #249

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
59 changes: 59 additions & 0 deletions examples/linux/file_coroutines_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <unifex/config.hpp>

// requires both coroutine and liburing support (for now)
#if !UNIFEX_NO_COROUTINES and !UNIFEX_NO_LIBURING
# include <unifex/linux/io_uring_context.hpp>
using io_context = unifex::linuxos::io_uring_context;

# include <unifex/sync_wait.hpp>
# include <unifex/task.hpp>

using namespace unifex;

int main() {
io_context ctx{};
auto sched = ctx.get_scheduler();
inplace_stop_source stopSource;
std::thread t{[&] {
ctx.run(stopSource.get_token());
}};
scope_guard stopOnExit = [&]() noexcept {
stopSource.request_stop();
t.join();
};
sync_wait([&]() -> task<void> {
auto file = open_file_write_only(
sched, filesystem::path{"file_coroutine_test.txt"});
constexpr char hello[]{"hello\n"};
size_t offset = 0;
for (int ii = 0; ii < 42; ++ii) {
offset += co_await async_write_some_at(
file, offset, as_bytes(span{hello, sizeof(hello) - 1}));
}
std::printf("wrote %zu bytes\n", offset);
}());
return 0;
}
#else
# include <cstdio>
int main() {
printf("neither io_uring or coroutine support found\n");
return 0;
}
#endif
66 changes: 66 additions & 0 deletions include/unifex/as_exception_ptr.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once

#include <unifex/exception.hpp>
#include <unifex/tag_invoke.hpp>
#include <unifex/type_traits.hpp>

#include <memory>
#include <system_error>

#include <unifex/detail/prologue.hpp>

namespace unifex
{
namespace _as_exception_ptr
{
inline constexpr struct _fn {
// forward std::exception_ptr
std::exception_ptr operator()(std::exception_ptr eptr) const noexcept {
return eptr;
Garcia6l20 marked this conversation as resolved.
Show resolved Hide resolved
}

// default std::error_code to std::exception_ptr conversion
std::exception_ptr operator()(std::error_code&& error) const noexcept {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return make_exception_ptr(
std::system_error{(std::error_code &&) error});
}

// convert std::exception based types to std::exception_ptr
template(typename Exception)
(requires (!tag_invocable<_fn, Exception>) AND
derived_from<Exception, std::exception>)
std::exception_ptr operator()(Exception&& ex) const noexcept {
return make_exception_ptr((Exception &&) ex);
}

// use customization point
// to resolve ErrorCode -> std::exception_ptr conversion
template(typename ErrorCode)
(requires tag_invocable<_fn, ErrorCode>)
std::exception_ptr operator()(ErrorCode&& error) const noexcept {
static_assert(nothrow_tag_invocable<_fn, ErrorCode>,
"as_exception_ptr() customisations must be declared noexcept");
return tag_invoke(*this, (ErrorCode &&) error);
}
Garcia6l20 marked this conversation as resolved.
Show resolved Hide resolved
} as_exception_ptr{};
} // namespace _as_exception_ptr

using _as_exception_ptr::as_exception_ptr;
Garcia6l20 marked this conversation as resolved.
Show resolved Hide resolved
} // namespace unifex

#include <unifex/detail/epilogue.hpp>
38 changes: 32 additions & 6 deletions include/unifex/receiver_concepts.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <unifex/type_traits.hpp>
#include <unifex/std_concepts.hpp>
#include <unifex/detail/unifex_fwd.hpp>
#include <unifex/as_exception_ptr.hpp>

#include <exception>
#include <type_traits>
Expand Down Expand Up @@ -96,12 +97,20 @@ namespace _rec_cpo {
using set_error_member_result_t =
decltype(UNIFEX_DECLVAL(Receiver).set_error(UNIFEX_DECLVAL(Error)));
template <typename Receiver, typename Error>
using _result_t =
typename conditional_t<
tag_invocable<_set_error_fn, Receiver, Error>,
meta_tag_invoke_result<_set_error_fn>,
meta_quote2<set_error_member_result_t>>::template apply<Receiver, Error>;
static auto _select() {
if constexpr (tag_invocable<_set_error_fn, Receiver, Error>) {
return meta_tag_invoke_result<_set_error_fn>{};
} else if constexpr (tag_invocable<_set_error_fn, Receiver, std::exception_ptr>) {
return meta_tag_invoke_result<_set_error_fn>{};
} else {
return meta_quote2<set_error_member_result_t>{};
}
}
template <typename Receiver, typename Error>
using _result_t = typename decltype(_set_error_fn::_select<Receiver, Error>())
::template apply<Receiver, Error>;
public:
// call through tag_invoke
template(typename Receiver, typename Error)
(requires tag_invocable<_set_error_fn, Receiver, Error>)
auto operator()(Receiver&& r, Error&& error) const noexcept
Expand All @@ -115,15 +124,32 @@ namespace _rec_cpo {
return unifex::tag_invoke(
_set_error_fn{}, (Receiver &&) r, (Error&&) error);
}

// direct call
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>)
Comment on lines +130 to +131
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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):

  1. tag_invoke(set_error, r, err) if that is well-formed; otherwise
  2. r.set_error(err) if that is well-formed; otherwise
  3. r.set_error(as_exception_ptr(err)) if that is well-formed; otherwise
  4. 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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));
   }
};

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

auto operator()(Receiver&& r, Error&& error) const noexcept
-> _result_t<Receiver, Error> {
static_assert(
noexcept(static_cast<Receiver&&>(r).set_error((Error &&) error)),
"receiver.set_error() method must be nothrow invocable");
return static_cast<Receiver&&>(r).set_error((Error&&) error);
}

// call through as_exception_ptr
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

std::invocable<decltype(as_exception_ptr), Error>)
auto operator()(Receiver&& r, Error&& error) const noexcept
-> _result_t<Receiver, std::exception_ptr> {
static_assert(
noexcept(static_cast<Receiver&&>(r).set_error(as_exception_ptr((Error&&) error))),
"receiver.set_error() method must be nothrow invocable");
return static_cast<Receiver&&>(r).set_error(as_exception_ptr((Error&&) error));
}
} set_error{};

inline const struct _set_done_fn {
Expand Down
16 changes: 4 additions & 12 deletions include/unifex/sync_wait.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
*/
#pragma once

#include <unifex/as_exception_ptr.hpp>
#include <unifex/bind_back.hpp>
#include <unifex/blocking.hpp>
#include <unifex/exception.hpp>
#include <unifex/manual_event_loop.hpp>
#include <unifex/manual_lifetime.hpp>
#include <unifex/scheduler_concepts.hpp>
#include <unifex/sender_concepts.hpp>
#include <unifex/blocking.hpp>
#include <unifex/with_query_value.hpp>
#include <unifex/bind_back.hpp>
#include <unifex/exception.hpp>

#include <condition_variable>
#include <exception>
Expand Down Expand Up @@ -83,15 +84,6 @@ struct _receiver {
signal_complete();
}

void set_error(std::error_code ec) && noexcept {
std::move(*this).set_error(make_exception_ptr(std::system_error{ec, "sync_wait"}));
}

template <typename Error>
void set_error(Error&& e) && noexcept {
std::move(*this).set_error(make_exception_ptr((Error&&)e));
}

void set_done() && noexcept {
promise_.state_ = promise<T>::state::done;
signal_complete();
Expand Down
7 changes: 5 additions & 2 deletions include/unifex/tag_invoke.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,11 @@ namespace unifex {

template <typename CPO, typename... Args>
UNIFEX_CONCEPT tag_invocable =
(sizeof(_tag_invoke::try_tag_invoke<CPO, Args...>(0)) ==
sizeof(_tag_invoke::yes_type));
(is_tag_invocable_v<CPO, Args...>);

template <typename CPO, typename... Args>
UNIFEX_CONCEPT nothrow_tag_invocable =
(is_nothrow_tag_invocable_v<CPO, Args...>);
Comment on lines +107 to +108
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


template <typename Fn>
using meta_tag_invoke_result =
Expand Down
87 changes: 87 additions & 0 deletions test/as_exception_ptr_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <unifex/as_exception_ptr.hpp>
#include <unifex/receiver_concepts.hpp>

#include <gtest/gtest.h>

using namespace unifex;

TEST(as_exception_ptr, error_code) {
try {
std::rethrow_exception(
as_exception_ptr(std::make_error_code(std::errc::not_supported)));
} catch (std::system_error& ex) {
EXPECT_EQ(ex.code(), std::errc::not_supported);
}
}

struct test_error {
int error_code;

friend std::exception_ptr
tag_invoke(tag_t<as_exception_ptr>, test_error&& error) noexcept {
return std::make_exception_ptr(std::runtime_error(
std::to_string(std::forward<test_error>(error).error_code)));
}
};

TEST(as_exception_ptr, custom_error) {
try {
std::rethrow_exception(as_exception_ptr(test_error{42}));
} catch (std::runtime_error& ex) {
EXPECT_EQ(ex.what(), std::string_view{"42"});
}
}

struct error_code_receiver {
std::optional<std::error_code>& ec_;
void set_error(std::error_code ec) && noexcept {
ec_ = (std::error_code &&) ec;
}
};

struct exception_ptr_receiver {
std::optional<std::exception_ptr>& ex_;
void set_error(std::exception_ptr ex) && noexcept {
ex_ = (std::exception_ptr &&) ex;
}
};

TEST(as_exception_ptr, set_error) {
{ // direct call with error_code
std::optional<std::error_code> ec{};
unifex::set_error(
error_code_receiver{ec},
std::make_error_code(std::errc::not_supported));
EXPECT_TRUE(ec.has_value());
}
{ // direct call with exception_ptr
std::optional<std::exception_ptr> ex{};
auto eptr = std::make_exception_ptr(
std::system_error{std::make_error_code(std::errc::not_supported)});
unifex::set_error(exception_ptr_receiver{ex}, std::move(eptr));
EXPECT_TRUE(ex.has_value());
}
{ // call through as_exception_ptr CPO
std::optional<std::exception_ptr> ex{};
unifex::set_error(
exception_ptr_receiver{ex},
std::make_error_code(std::errc::not_supported));
EXPECT_TRUE(ex.has_value());
}
}