Use variant as payload for Result<T, E>
Use std::variant<std::monostate, T, std::unique_ptr<E>> as the payload
for Result allows T to be non-default-constructible and also moves some
bookkeeping from Result into std::variant.
Bug: dawn:2344
Change-Id: I3f7d1aa4b7776de0174052c4379e5abb1ef107c3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/169361
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/common/Result.h b/src/dawn/common/Result.h
index 7bcccfc..8000a0f 100644
--- a/src/dawn/common/Result.h
+++ b/src/dawn/common/Result.h
@@ -33,6 +33,7 @@
#include <memory>
#include <type_traits>
#include <utility>
+#include <variant>
#include "dawn/common/Assert.h"
#include "dawn/common/Compiler.h"
@@ -224,7 +225,7 @@
template <typename T, typename E>
class [[nodiscard]] Result {
public:
- Result(T&& success);
+ Result(T success);
Result(std::unique_ptr<E> error);
Result(Result<T, E>&& other);
@@ -235,19 +236,11 @@
bool IsError() const;
bool IsSuccess() const;
- T&& AcquireSuccess();
+ T AcquireSuccess();
std::unique_ptr<E> AcquireError();
private:
- enum PayloadType {
- Success = 0,
- Error = 1,
- Acquired = 2,
- };
- PayloadType mType;
-
- std::unique_ptr<E> mError;
- T mSuccess;
+ std::variant<std::monostate, T, std::unique_ptr<E>> mPayload;
};
// Implementation of Result<void, E>
@@ -478,52 +471,52 @@
// Implementation of Result<T, E>
template <typename T, typename E>
-Result<T, E>::Result(T&& success) : mType(Success), mSuccess(std::move(success)) {}
+Result<T, E>::Result(T success) : mPayload(std::move(success)) {}
template <typename T, typename E>
-Result<T, E>::Result(std::unique_ptr<E> error) : mType(Error), mError(std::move(error)) {}
+Result<T, E>::Result(std::unique_ptr<E> error) : mPayload(std::move(error)) {}
template <typename T, typename E>
Result<T, E>::~Result() {
- DAWN_ASSERT(mType == Acquired);
+ DAWN_ASSERT(std::holds_alternative<std::monostate>(mPayload));
}
template <typename T, typename E>
-Result<T, E>::Result(Result<T, E>&& other)
- : mType(other.mType), mError(std::move(other.mError)), mSuccess(std::move(other.mSuccess)) {
- other.mType = Acquired;
+Result<T, E>::Result(Result<T, E>&& other) {
+ *this = std::move(other);
}
+
template <typename T, typename E>
Result<T, E>& Result<T, E>::operator=(Result<T, E>&& other) {
- mType = other.mType;
- mError = std::move(other.mError);
- mSuccess = std::move(other.mSuccess);
- other.mType = Acquired;
+ DAWN_ASSERT(std::holds_alternative<std::monostate>(mPayload));
+ std::swap(mPayload, other.mPayload);
return *this;
}
template <typename T, typename E>
bool Result<T, E>::IsError() const {
- return mType == Error;
+ return std::holds_alternative<std::unique_ptr<E>>(mPayload);
}
template <typename T, typename E>
bool Result<T, E>::IsSuccess() const {
- return mType == Success;
+ return std::holds_alternative<T>(mPayload);
}
template <typename T, typename E>
-T&& Result<T, E>::AcquireSuccess() {
- DAWN_ASSERT(mType == Success);
- mType = Acquired;
- return std::move(mSuccess);
+T Result<T, E>::AcquireSuccess() {
+ DAWN_ASSERT(IsSuccess());
+ auto payload = std::move(mPayload);
+ mPayload = {};
+ return std::move(std::get<T>(payload));
}
template <typename T, typename E>
std::unique_ptr<E> Result<T, E>::AcquireError() {
- DAWN_ASSERT(mType == Error);
- mType = Acquired;
- return std::move(mError);
+ DAWN_ASSERT(IsError());
+ auto payload = std::move(mPayload);
+ mPayload = {};
+ return std::move(std::get<std::unique_ptr<E>>(payload));
}
} // namespace dawn
diff --git a/src/dawn/tests/unittests/ResultTests.cpp b/src/dawn/tests/unittests/ResultTests.cpp
index 8b5064d..44274e1 100644
--- a/src/dawn/tests/unittests/ResultTests.cpp
+++ b/src/dawn/tests/unittests/ResultTests.cpp
@@ -398,5 +398,58 @@
TestSuccess(&result, {1.0f});
}
+class NonDefaultConstructible {
+ public:
+ explicit NonDefaultConstructible(float v) : v(v) {}
+ bool operator==(const NonDefaultConstructible& other) const { return v == other.v; }
+ float v;
+};
+
+// Test constructing an error Result<T, E> for non-default-constructible T
+TEST(ResultNonDefaultConstructible, ConstructingError) {
+ Result<NonDefaultConstructible, int> result(std::make_unique<int>(placeholderError));
+ TestError(&result, placeholderError);
+}
+
+// Test moving an error Result<T, E> for non-default-constructible T
+TEST(ResultNonDefaultConstructible, MovingError) {
+ Result<NonDefaultConstructible, int> result(std::make_unique<int>(placeholderError));
+ Result<NonDefaultConstructible, int> movedResult(std::move(result));
+ TestError(&movedResult, placeholderError);
+}
+
+// Test returning an error Result<T, E> for non-default-constructible T
+TEST(ResultNonDefaultConstructible, ReturningError) {
+ auto CreateError = []() -> Result<NonDefaultConstructible, int> {
+ return {std::make_unique<int>(placeholderError)};
+ };
+
+ Result<NonDefaultConstructible, int> result = CreateError();
+ TestError(&result, placeholderError);
+}
+
+// Test constructing a success Result<T, E> for non-default-constructible T
+TEST(ResultNonDefaultConstructible, ConstructingSuccess) {
+ Result<NonDefaultConstructible, int> result(NonDefaultConstructible(1.0f));
+ TestSuccess(&result, NonDefaultConstructible(1.0f));
+}
+
+// Test moving a success Result<T, E> for non-default-constructible T
+TEST(ResultNonDefaultConstructible, MovingSuccess) {
+ Result<NonDefaultConstructible, int> result(NonDefaultConstructible(1.0f));
+ Result<NonDefaultConstructible, int> movedResult(std::move(result));
+ TestSuccess(&movedResult, NonDefaultConstructible(1.0f));
+}
+
+// Test returning a success Result<T, E> for non-default-constructible T
+TEST(ResultNonDefaultConstructible, ReturningSuccess) {
+ auto CreateSuccess = []() -> Result<NonDefaultConstructible, int> {
+ return {NonDefaultConstructible(1.0f)};
+ };
+
+ Result<NonDefaultConstructible, int> result = CreateSuccess();
+ TestSuccess(&result, NonDefaultConstructible(1.0f));
+}
+
} // anonymous namespace
} // namespace dawn
