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
65 changes: 65 additions & 0 deletions include/unifex/as_exception_ptr.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* 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
}

// convert std::exception based types to std::exception_ptr
template(typename Exception)
(requires std::is_base_of_v<std::exception, Exception>)
Garcia6l20 marked this conversation as resolved.
Show resolved Hide resolved
std::exception_ptr operator()(Exception&& ex) const noexcept {
return make_exception_ptr(std::forward<Exception>(ex));
}

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

// default std::error_code -> std::exception_ptr conversion
inline std::exception_ptr
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)});
Garcia6l20 marked this conversation as resolved.
Show resolved Hide resolved
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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


} // 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>
14 changes: 13 additions & 1 deletion 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 @@ -116,14 +117,25 @@ namespace _rec_cpo {
_set_error_fn{}, (Receiver &&) r, (Error&&) error);
}
template(typename Receiver, typename Error)
(requires (!tag_invocable<_set_error_fn, Receiver, Error>))
(requires (!tag_invocable<_set_error_fn, Receiver, Error>) &&
Garcia6l20 marked this conversation as resolved.
Show resolved Hide resolved
(!tag_invocable<decltype(as_exception_ptr), Error>))
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);
}
template(typename Receiver, typename Error)
(requires (!tag_invocable<_set_error_fn, Receiver, Error>) &&
Garcia6l20 marked this conversation as resolved.
Show resolved Hide resolved
tag_invocable<decltype(as_exception_ptr), Error>)
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

auto operator()(Receiver&& r, Error&& error) const noexcept
-> _result_t<Receiver, std::exception_ptr> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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

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
47 changes: 47 additions & 0 deletions test/as_exception_ptr_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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 <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"});
}
}