Add a `try_unwrap(...) or_return` construct, similar to Rust's `?` operator #6554

daira · 2023-04-14T19:47:57Z

After #include "util/unwrap.h",

try_unwrap(x) or_return is roughly equivalent to (using the GCC/clang statement expression extension):

({
    auto m = (x);
    if (!m.has_value()) return adapt_error(m);
    m.value(); // if m has a non-void value_type
})

where adapt_error(m) preserves the error (or null) status of m but may adapt it to the correct value type.

try_unwrap(x) or_return_unexpected(err) is roughly equivalent to:

({
    auto m = (x);
    if (!m.has_value()) return tl::make_unexpected(err);
    m.value(); // if m has a non-void value_type
})

try_unwrap(x) or_assert is roughly equivalent to:

({
    auto m = (x);
    assert(m.has_value());
    m.value(); // if m has a non-void value_type
})

These constructs work correctly for any type T that has:

a value_type member;
a bool has_value() method;
a T::value_type value() method that can be called when has_value() returns true. This need not exist if T::value_type is void.

These conditions are met for tl::expected<T, E>, the proposed std::expected<T, E> [*], std::optional<T>, and potentially your custom type.

See the tests in src/gtest/test_unwrap.cpp for what is possible in terms of conversion between types.

[*] Caveats: type adaption is not enabled for std::expected (but could easily be added), and try_unwrap(x) or_return_unexpected(err) will always return tl::expected in the error case.

fixes #6552

daira · 2023-04-23T02:34:58Z

src/wallet/wallet_tx_builder.cpp

+            maxSaplingAvailable,
+            maxOrchardAvailable,
+            orchardOutputs)
+    ) or_return;


Notice that the map_errors (two of them in this function) go away because ResolvePayment returns a tl::expected<ResolvedPayment, AddressResolutionError>, and AddressResolutionError is a variant of InputSelectionError. So this just works, because there is an implicit conversion from each arm of a variant to the variant type.

I'm not wedded to the convention I've adopted here of putting the ) or return at the start of the line, but it does make it easier to see that there is a potential return. I looked at whether it was possible to get Visual Studio to highlight these as keywords, but couldn't find the highlighting config file for C++.

daira · 2023-04-23T02:36:16Z

src/wallet/wallet_tx_builder.cpp

-                });
-        });
+    try_unwrap(ValidateAmount(spendable, initialFee)
+    ) or_return;


It just works with void types (this was the most complicated part).

daira · 2023-04-23T02:38:27Z

src/wallet/wallet_tx_builder.cpp

+            orchardOutputs)
+    ) or_return;
+
+    return {std::make_pair(actualPayment, finalFee)};


The addition of braces here isn't essential; just a matter of style because we're constructing a tl::expected from this pair (as we were before).

daira · 2023-04-23T02:43:25Z

src/wallet/wallet_tx_builder.cpp

-        });
+    try_unwrap(ValidateAmount(spendable, initialFee)
+    ) or_return;
+    const ResolvedPayment rpayment = try_unwrap(ResolvePayment(


const auto rpayment would also work here; the try_unwrap doesn't interfere with type inference. I just prefer it this way. (You do sometimes have to add explicit return types to lambdas when refactoring to use try_unwrap.)

daira · 2023-04-23T02:55:11Z

src/gtest/test_unwrap.cpp

+        int v = try_unwrap(ex) or_assert;
+        return {v+1};
+    };
+    ASSERT_DEATH(fn(err), "has_value()");


I'm not certain these death tests will work on all platforms (in particular, I don't know if Windows will preserve the assertion text that we check for here). The test assumes NDEBUG is not defined, which is always the case for zcashd.

src/util/unwrap.h

`try_unwrap(x) or_return` is roughly equivalent to (using the GCC/clang statement expression extension): ```c++ ({ auto m = (x); if (!m.has_value()) return adapt_error(m); m.value(); // if m has a non-void value_type }) ``` where `adapt_error(m)` preserves the error (or null) status of `m` but may adapt it to the correct value type. `try_unwrap(x) or_return_unexpected(err)` is roughly equivalent to: ```c++ ({ auto m = (x); if (!m.has_value()) return tl::make_unexpected(err); m.value(); // if m has a non-void value_type }) ``` `try_unwrap(x) or_assert` is roughly equivalent to: ```c++ ({ auto m = (x); assert(m.has_value()); m.value(); // if m has a non-void value_type }) ``` These constructs work correctly for any type T that has: * a `value_type` member; * a `bool has_value()` method; * a `T::value_type value()` method that can be called when `has_value()` returns true. This need not exist if `T::value_type` is void. These conditions are met for `tl::expected<T, E>`, the proposed `std::expected<T, E>`, `std::optional<T>`, and potentially your custom type. See the tests in `src/gtest/test_unwrap.cpp` for what is possible in terms of conversion between types. Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>

Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>

daira · 2023-10-02T16:24:24Z

I suggest reviewing with "Hide whitespace" initially. (One of the points of this is to avoid the tab slide to the right caused by nested error handling.)

daira force-pushed the unwrap_or_return branch 2 times, most recently from 1375714 to 0e4b5d3 Compare April 23, 2023 02:10

daira added I-error-handling Problems and improvements related to error handling safe-to-build Used to send PR to prod CI environment labels Apr 23, 2023

daira requested review from sellout, nuttycom and str4d April 23, 2023 02:12

daira marked this pull request as ready for review April 23, 2023 02:12

daira changed the title ~~Add an unwrap { ... } or_return construct, similar to Rust's ? operator~~ Add an try_unwrap { ... } or_return construct, similar to Rust's ? operator Apr 23, 2023

daira changed the title ~~Add an try_unwrap { ... } or_return construct, similar to Rust's ? operator~~ Add an try_unwrap(...) or_return construct, similar to Rust's ? operator Apr 23, 2023

daira changed the title ~~Add an try_unwrap(...) or_return construct, similar to Rust's ? operator~~ Add a try_unwrap(...) or_return construct, similar to Rust's ? operator Apr 23, 2023

daira force-pushed the unwrap_or_return branch from 0e4b5d3 to 4b2123a Compare April 23, 2023 02:21

daira mentioned this pull request Apr 23, 2023

Consider a macro corresponding to Rust's ? operator #6552

Open

daira commented Apr 23, 2023

View reviewed changes

ECC-CI removed the safe-to-build Used to send PR to prod CI environment label Apr 23, 2023

daira commented Apr 23, 2023

View reviewed changes

daira added the safe-to-build Used to send PR to prod CI environment label Apr 23, 2023

ECC-CI removed the safe-to-build Used to send PR to prod CI environment label Apr 23, 2023

daira force-pushed the unwrap_or_return branch from 4b2123a to e7d1319 Compare April 23, 2023 13:08

daira mentioned this pull request Apr 23, 2023

Implementation of try? TartanLlama/expected#37

Open

daira added the safe-to-build Used to send PR to prod CI environment label Apr 23, 2023

ECC-CI removed the safe-to-build Used to send PR to prod CI environment label Apr 23, 2023

daira commented Apr 28, 2023

View reviewed changes

src/util/unwrap.h Outdated Show resolved Hide resolved

daira commented Apr 28, 2023

View reviewed changes

src/util/unwrap.h Outdated Show resolved Hide resolved

daira commented Apr 28, 2023

View reviewed changes

src/util/unwrap.h Outdated Show resolved Hide resolved

daira force-pushed the unwrap_or_return branch from e7d1319 to 2af0ba3 Compare October 2, 2023 16:12

Make use of try_unwrap constructs.

100a0e1

Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>

daira force-pushed the unwrap_or_return branch from 2af0ba3 to 100a0e1 Compare October 2, 2023 16:12

daira added the safe-to-build Used to send PR to prod CI environment label Oct 2, 2023

daira removed the request for review from sellout October 2, 2023 16:24

daira added this to the Release 5.8.0 milestone Oct 2, 2023

ECC-CI removed the safe-to-build Used to send PR to prod CI environment label Oct 2, 2023

str4d modified the milestones: Release 5.8.0, Post 5.8.0 Dec 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a `try_unwrap(...) or_return` construct, similar to Rust's `?` operator #6554

Add a `try_unwrap(...) or_return` construct, similar to Rust's `?` operator #6554

daira commented Apr 14, 2023 •

edited

daira Apr 23, 2023 •

edited

daira Apr 23, 2023 •

edited

daira Apr 23, 2023

daira Apr 23, 2023 •

edited

daira Apr 23, 2023 •

edited

daira Apr 23, 2023 •

edited

daira commented Oct 2, 2023

Add a try_unwrap(...) or_return construct, similar to Rust's ? operator #6554

Are you sure you want to change the base?

Add a try_unwrap(...) or_return construct, similar to Rust's ? operator #6554

Conversation

daira commented Apr 14, 2023 • edited

daira Apr 23, 2023 • edited

Choose a reason for hiding this comment

daira Apr 23, 2023 • edited

Choose a reason for hiding this comment

daira Apr 23, 2023

Choose a reason for hiding this comment

daira Apr 23, 2023 • edited

Choose a reason for hiding this comment

daira Apr 23, 2023 • edited

Choose a reason for hiding this comment

daira Apr 23, 2023 • edited

Choose a reason for hiding this comment

daira commented Oct 2, 2023

Add a `try_unwrap(...) or_return` construct, similar to Rust's `?` operator #6554

Add a `try_unwrap(...) or_return` construct, similar to Rust's `?` operator #6554

daira commented Apr 14, 2023 •

edited

daira Apr 23, 2023 •

edited

daira Apr 23, 2023 •

edited

daira Apr 23, 2023 •

edited

daira Apr 23, 2023 •

edited

daira Apr 23, 2023 •

edited