Support references in ErrorOr (#4889)

### Context & Motivation

The error handling utilities in `//base/error.h` are very useful for
writing code with strong safety guarantees. While hardening the `Dump`
debug utilities (from review in #4866), I encountered a rough edge with
references and pointers. After a [brief Discord discussion in
#contributing-help](https://discord.com/channels/655572317891461132/1052653651895779359/1334675462877610038),
it was suggested that adding support for references to `ErrorOr` would
be a good candidate to move forward.

Using a reference type with the `ErrorOr` class (e.g. `ErrorOr<Node&>`)
produces two errors:

<ol>
<li><strong><code>variant can not have a reference type as an
alternative</code></strong>
<ul><li>From private field: <code>std::variant&lt;Error, T&gt;
val_;</code></li></ul>
</li>
<li><strong><code>'operator-&gt;' declared as a pointer to a
reference</code></strong>
<ul><li>From member function: <code>auto operator-&gt;() -&gt;
T*</code></li></ul>
</li>
</ol>

### Changes

To support reference types, both errors are resolved:

1. `std::reference_wrapper` is conditionally used for storage when `T`
is a reference type
2. type trait aliases like `using ValueT = std::remove_reference_t<T>`
are used to produce compatible types for methods like `auto operator->()
-> ValueT*`
This commit is contained in:
Calvin
2025-02-04 21:08:43 +00:00
committed by GitHub
parent 6803127ab0
commit dcfccd3187
2 changed files with 31 additions and 15 deletions
+25 -15
View File
@@ -5,7 +5,9 @@
#ifndef CARBON_COMMON_ERROR_H_
#define CARBON_COMMON_ERROR_H_
#include <functional>
#include <string>
#include <type_traits>
#include <variant>
#include "common/check.h"
@@ -76,18 +78,29 @@ class [[nodiscard]] Error : public Printable<Error> {
template <typename T>
class [[nodiscard]] ErrorOr {
public:
using ValueT = std::remove_reference_t<T>;
// Constructs with an error; the error must not be Error::Success().
// Implicit for easy construction on returns.
// NOLINTNEXTLINE(google-explicit-constructor)
ErrorOr(Error err) : val_(std::move(err)) {}
// Constructs with a reference.
// Implicit for easy construction on returns.
// NOLINTNEXTLINE(google-explicit-constructor)
ErrorOr(T ref)
requires std::is_reference_v<T>
: val_(std::ref(ref)) {}
// Constructs with a value.
// Implicit for easy construction on returns.
// NOLINTNEXTLINE(google-explicit-constructor)
ErrorOr(T val) : val_(std::move(val)) {}
ErrorOr(T val)
requires(!std::is_reference_v<T>)
: val_(std::move(val)) {}
// Returns true for success.
auto ok() const -> bool { return std::holds_alternative<T>(val_); }
auto ok() const -> bool { return std::holds_alternative<StoredT>(val_); }
// Returns the contained error.
// REQUIRES: `ok()` is false.
@@ -102,35 +115,32 @@ class [[nodiscard]] ErrorOr {
// Returns the contained value.
// REQUIRES: `ok()` is true.
auto operator*() -> T& {
auto operator*() -> ValueT& {
CARBON_CHECK(ok());
return std::get<T>(val_);
return std::get<StoredT>(val_);
}
// Returns the contained value.
// REQUIRES: `ok()` is true.
auto operator*() const -> const T& {
auto operator*() const -> const ValueT& {
CARBON_CHECK(ok());
return std::get<T>(val_);
return std::get<StoredT>(val_);
}
// Returns the contained value.
// REQUIRES: `ok()` is true.
auto operator->() -> T* {
CARBON_CHECK(ok());
return &std::get<T>(val_);
}
auto operator->() -> ValueT* { return &**this; }
// Returns the contained value.
// REQUIRES: `ok()` is true.
auto operator->() const -> const T* {
CARBON_CHECK(ok());
return &std::get<T>(val_);
}
auto operator->() const -> const ValueT* { return &**this; }
private:
using StoredT = std::conditional_t<std::is_reference_v<T>,
std::reference_wrapper<ValueT>, T>;
// Either an error message or a value.
std::variant<Error, T> val_;
std::variant<Error, StoredT> val_;
};
// A helper class for accumulating error message and converting to
+6
View File
@@ -50,6 +50,12 @@ TEST(ErrorTest, ErrorOrArrowOp) {
EXPECT_EQ(err->val, 1);
}
TEST(ErrorTest, ErrorOrReference) {
Val val = {1};
ErrorOr<Val&> maybe_val(val);
EXPECT_EQ(maybe_val->val, 1);
}
auto IndirectErrorOrSuccessTest() -> ErrorOr<Success> { return Success(); }
TEST(ErrorTest, IndirectErrorOrSuccess) {