Skip to content

Fixes and code cleanup for include/pfn/expected.hpp #189

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.

Sign up for GitHub

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

Open
Bronek wants to merge 13 commits into
base: main
Choose a base branch
from
+569 −63
Open

Fixes and code cleanup for include/pfn/expected.hpp #189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b248c1d
Remove dead code from pfn/expected.hpp
Bronek May 7, 2026
dee58a1
Add is_nothrow_swappable, fix typos, test cleanup
Bronek May 7, 2026
bfda5e6
Add comments to pfn/include.hpp
Bronek May 7, 2026
6b9b9f1
Refine noexcept on pfn::expected converting assignment operators
Bronek May 7, 2026
b8eed46
Improve unit test coverage for throwing assignment
Bronek May 7, 2026
cff135e
Improve unit test coverage for expected<void, E>
Bronek May 7, 2026
e37edba
Add noexcept to unexpected::operator== with tests
Bronek May 7, 2026
46e797b
Fix heterogeneous comparison in unexpected::operator== and extend tests
Bronek May 7, 2026
a853d19
Fix uses of std::is_assignable with tests
Bronek May 7, 2026
cd8c609
Add .scratch* to .gitignore
Bronek May 8, 2026
11521b6
Implement reinit workaround for clang in aarch64
Bronek May 8, 2026
0a78507
Workaround for heterogeneous comparisons in std::unexpected in libc++…
Bronek May 8, 2026
553f03c
MSVC workaround for buggy std::is_nothrow_* implementation
Bronek May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.idea/
.idea_modules/
.build*/
.scratch*/
dist*/
result*
.cache/
Expand Down
81 changes: 55 additions & 26 deletions include/pfn/expected.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <cassert>
#include <concepts>
#include <cstring>
#include <exception>
#include <functional>
#include <initializer_list>
Expand Down Expand Up @@ -37,6 +38,13 @@
#define ASSERT(...) assert((__VA_ARGS__) == true)
#endif

// This header is a polyfill for std::expected, tracking the C++ working draft
// and changes planned for the future standard revisions. As a deliberate extension,
// member functions whose `noexcept` specification is left unspecified by the
// standard are given a `noexcept` clause derived from the properties of the
// underlying types T, E and (where applicable) invocable arguments. Each such
// clause is marked inline with a "// extension" trailing comment.

namespace pfn {

template <class E> class bad_expected_access;
Expand Down Expand Up @@ -137,9 +145,11 @@
swap(e_, other.e_);
}

template <class E2> constexpr friend bool operator==(unexpected const &x, unexpected<E2> const &y)
template <class E2>
constexpr friend bool operator==(unexpected const &x, unexpected<E2> const &y) //
noexcept(noexcept(static_cast<bool>(x.error() == y.error()))) // extension
Copy link
Copy Markdown

@augmentcode augmentcode Bot May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

include/pfn/expected.hpp:149 — The noexcept computation uses static_cast<bool>(x.error() == y.error()); this can succeed even when the comparison result is only explicitly convertible to bool, while the return statement still requires an implicit conversion to bool. Consider aligning the noexcept/participation check with implicit convertibility to avoid surprising hard errors in some generic contexts.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

{
return x.e_ == y.e_;
return x.error() == y.error();
}

constexpr friend void swap(unexpected &x, unexpected &y) noexcept(noexcept(x.swap(y)))
Expand Down Expand Up @@ -216,7 +226,7 @@
not ::std::is_same_v<expected, ::std::remove_cvref_t<U>> //
&& not detail::_is_some_unexpected<::std::remove_cvref_t<U>> //
&& ::std::is_constructible_v<T, U> //
&& ::std::is_assignable_v<T, U> //
&& ::std::is_assignable_v<T &, U> //
&& (::std::is_nothrow_constructible_v<T, U> //
|| ::std::is_nothrow_move_constructible_v<T> //
|| ::std::is_nothrow_move_constructible_v<E>)>;
Expand All @@ -233,6 +243,37 @@
New tmp(::std::forward<Args>(args)...);
::std::destroy_at(::std::addressof(oldval));
::std::construct_at(::std::addressof(newval), std::move(tmp));
} else if constexpr (::std::is_trivially_copyable_v<Old>) {
// Workaround for https://github.com/llvm/llvm-project/issues/196520:
// clang on aarch64 sinks the snapshot load past the store-through-newval
// when Old's TBAA tag differs from New's, corrupting the strong-EG
// restoration on catch. A byte-buffer snapshot via std::memcpy is opaque
// to TBAA, and preserving *oldval in place across the try (no destroy_at)
// makes the catch a plain byte restore -- which for trivially-copyable
// (and therefore trivially-destructible) Old is observationally identical
// to the destroy-and-recreate branch below.
// (Old is also trivially-destructible -- implied by is_trivially_copyable_v
// per [class.prop]/1 -- so skipping destroy_at is observationally identical
// to the destroy-and-recreate branch below.)
if (not ::std::is_constant_evaluated()) {
alignas(Old) unsigned char _bytes[sizeof(Old)];
::std::memcpy(_bytes, ::std::addressof(oldval), sizeof(Old));
try {
::std::construct_at(::std::addressof(newval), ::std::forward<Args>(args)...);
} catch (...) {
::std::memcpy(::std::addressof(oldval), _bytes, sizeof(Old));
throw;
}
} else {
Old tmp(std::move(oldval));
::std::destroy_at(::std::addressof(oldval));

Check warning on line 269 in include/pfn/expected.hpp

View check run for this annotation

Codecov / codecov/patch

include/pfn/expected.hpp#L268-L269 

Added lines #L268 - L269 were not covered by tests
try {
::std::construct_at(::std::addressof(newval), ::std::forward<Args>(args)...);
} catch (...) {
::std::construct_at(::std::addressof(oldval), std::move(tmp));
throw;

Check warning on line 274 in include/pfn/expected.hpp

View check run for this annotation

Codecov / codecov/patch

include/pfn/expected.hpp#L271-L274 

Added lines #L271 - L274 were not covered by tests
}
}
} else {
Old tmp(std::move(oldval));
::std::destroy_at(::std::addressof(oldval));
Expand Down Expand Up @@ -575,9 +616,8 @@
}

template <class U = T>
constexpr expected &operator=(U &&s) //
noexcept(::std::is_nothrow_assignable_v<T, U> && ::std::is_nothrow_constructible_v<T, U>
&& (::std::is_nothrow_move_constructible_v<T> || ::std::is_nothrow_move_constructible_v<E>)) // extension
constexpr expected &operator=(U &&s) //
noexcept(::std::is_nothrow_assignable_v<T &, U> && ::std::is_nothrow_constructible_v<T, U>) // extension
requires(_can_convert_assign<U>::value)
{
if (set_) {
Expand All @@ -591,8 +631,8 @@

template <class G>
constexpr expected &operator=(unexpected<G> const &s) //
noexcept(::std::is_nothrow_assignable_v<E, G const &> && ::std::is_nothrow_constructible_v<E, G const &>
&& (::std::is_nothrow_move_constructible_v<E> || ::std::is_nothrow_move_constructible_v<T>)) // extension
noexcept(::std::is_nothrow_assignable_v<E &, G const &>
&& ::std::is_nothrow_constructible_v<E, G const &>) // extension
requires(::std::is_constructible_v<E, G const &> && ::std::is_assignable_v<E &, G const &>
&& (::std::is_nothrow_constructible_v<E, G const &> || ::std::is_nothrow_move_constructible_v<T>
|| ::std::is_nothrow_move_constructible_v<E>))
Expand All @@ -607,9 +647,8 @@
}

template <class G>
constexpr expected &operator=(unexpected<G> &&s) //
noexcept(::std::is_nothrow_assignable_v<E, G> && ::std::is_nothrow_constructible_v<E, G>
&& (::std::is_nothrow_move_constructible_v<E> || ::std::is_nothrow_move_constructible_v<T>)) // extension
constexpr expected &operator=(unexpected<G> &&s) //
noexcept(::std::is_nothrow_assignable_v<E &, G> && ::std::is_nothrow_constructible_v<E, G>) // extension
requires(::std::is_constructible_v<E, G> && ::std::is_assignable_v<E &, G>
&& (::std::is_nothrow_constructible_v<E, G> || ::std::is_nothrow_move_constructible_v<T>
|| ::std::is_nothrow_move_constructible_v<E>))
Expand Down Expand Up @@ -699,7 +738,7 @@
constexpr T &&operator*() && noexcept { return ::std::move(*(this->operator->())); }
constexpr explicit operator bool() const noexcept { return set_; }
constexpr bool has_value() const noexcept { return set_; }
constexpr bool has_error() const noexcept { return !set_; }
constexpr bool has_error() const noexcept { return !set_; } // P3798
constexpr T const &value() const &
{
static_assert(::std::is_copy_constructible_v<E>);
Expand Down Expand Up @@ -974,16 +1013,6 @@
template <class U, class G> using _can_move_convert = _can_convert_detail<U, G, U, G>;
template <class U, class G> friend class expected;

template <class U>
using _can_convert = ::std::bool_constant< //
not ::std::is_same_v<::std::remove_cvref_t<U>, ::std::in_place_t> //
&& not ::std::is_same_v<::std::remove_cvref_t<U>, unexpect_t> // LWG4222
&& not ::std::is_same_v<expected, ::std::remove_cvref_t<U>> //
&& not detail::_is_some_unexpected<::std::remove_cvref_t<U>> //
&& ::std::is_constructible_v<T, U> //
&& (not ::std::is_same_v<bool, ::std::remove_cv_t<T>> //
|| not detail::_is_some_expected<::std::remove_cvref_t<U>>)>;

template <typename Self, typename Fn>
static constexpr auto _and_then(Self &&self, Fn &&fn) //
noexcept(::std::is_nothrow_invocable_v<Fn> && ::std::is_nothrow_constructible_v<E, decltype(FWD(self).error())>)
Expand Down Expand Up @@ -1203,7 +1232,7 @@

template <class G>
constexpr expected &operator=(unexpected<G> const &s) //
noexcept(::std::is_nothrow_assignable_v<E, G const &>
noexcept(::std::is_nothrow_assignable_v<E &, G const &>
&& ::std::is_nothrow_constructible_v<E, G const &>) // extension
requires(::std::is_constructible_v<E, G const &> && ::std::is_assignable_v<E &, G const &>)
{
Expand All @@ -1218,8 +1247,8 @@
}

template <class G>
constexpr expected &operator=(unexpected<G> &&s) //
noexcept(::std::is_nothrow_assignable_v<E, G> && ::std::is_nothrow_constructible_v<E, G>) // extension
constexpr expected &operator=(unexpected<G> &&s) //
noexcept(::std::is_nothrow_assignable_v<E &, G> && ::std::is_nothrow_constructible_v<E, G>) // extension
requires(::std::is_constructible_v<E, G> && ::std::is_assignable_v<E &, G>)
{
if (set_) {
Expand Down Expand Up @@ -1276,7 +1305,7 @@
// [expected.void.obs], observers
constexpr explicit operator bool() const noexcept { return set_; }
constexpr bool has_value() const noexcept { return set_; }
constexpr bool has_error() const noexcept { return !set_; }
constexpr bool has_error() const noexcept { return !set_; } // P3798
constexpr void operator*() const noexcept { ASSERT(set_); }
constexpr void value() const &
{
Expand Down
Loading
Loading