From 8f73edeaf6558e8c14ad80679737b90c24d08e31 Mon Sep 17 00:00:00 2001 From: Ethan Nicholas Date: Tue, 1 Jun 2021 13:25:22 -0400 Subject: [PATCH] Added more SkTOptional APIs This adds copy constructors / assignment operator, reset(), value(), and has_value() to SkTOptional. Change-Id: I564552a75a4c612685cdaa8e80e4359b394b64a4 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/414338 Commit-Queue: Ethan Nicholas Reviewed-by: John Stiles --- include/private/SkTOptional.h | 101 ++++++++++++++++++++------ tests/SkTOptionalTest.cpp | 131 +++++++++++++++++++++++++++++++--- 2 files changed, 202 insertions(+), 30 deletions(-) diff --git a/include/private/SkTOptional.h b/include/private/SkTOptional.h index 87ef034c4e..c66eb8bf7f 100644 --- a/include/private/SkTOptional.h +++ b/include/private/SkTOptional.h @@ -21,66 +21,123 @@ namespace skstd { template class optional { public: - optional(const optional&) = delete; + optional(const T& value) + : fHasValue(true) { + new(&fPayload.fValue) T(value); + } optional(T&& value) : fHasValue(true) { new(&fPayload.fValue) T(std::move(value)); } - template - optional(Args&&... args) - : optional(T(std::forward(args...))) {} - optional() {} + optional(const optional& other) { + *this = other; + } + + // We need a non-const copy constructor because otherwise optional(nonConstSrc) isn't an exact + // match for the copy constructor, and we'd end up invoking the Args&&... template by mistake. + optional(optional& other) { + *this = other; + } + optional(optional&& other) { *this = std::move(other); } - ~optional() { - if (fHasValue) { - fPayload.fValue.~T(); - } + template + optional(Args&&... args) { + fHasValue = true; + new(&fPayload.fValue) T(std::forward(args...)); } - optional& operator=(const optional&) = delete; + ~optional() { + this->reset(); + } - optional& operator=(optional&& other) { + optional& operator=(const optional& other) { if (this != &other) { if (fHasValue) { - fPayload.fValue.~T(); - } - fHasValue = other.fHasValue; - if (fHasValue) { - new(&fPayload.fValue) T(std::move(other.fPayload.fValue)); + if (other.fHasValue) { + fPayload.fValue = other.fPayload.fValue; + } else { + this->reset(); + } + } else { + if (other.fHasValue) { + fHasValue = true; + new (&fPayload.fValue) T(other.fPayload.fValue); + } else { + // do nothing, no value on either side + } } } return *this; } - T& operator*() { + optional& operator=(optional&& other) { + if (this != &other) { + if (fHasValue) { + if (other.fHasValue) { + fPayload.fValue = std::move(other.fPayload.fValue); + } else { + this->reset(); + } + } else { + if (other.fHasValue) { + fHasValue = true; + new (&fPayload.fValue) T(std::move(other.fPayload.fValue)); + } else { + // do nothing, no value on either side + } + } + } + return *this; + } + + const T& value() const { SkASSERT(fHasValue); return fPayload.fValue; } + T& value() { + return fPayload.fValue; + } + + T& operator*() { + SkASSERT(fHasValue); + return this->value(); + } + T* operator->() { SkASSERT(fHasValue); - return &fPayload.fValue; + return &this->value(); } const T& operator*() const { - SkASSERT(fHasValue); - return fPayload.fValue; + return this->value(); } const T* operator->() const { SkASSERT(fHasValue); - return &fPayload.fValue; + return &this->value(); + } + + bool has_value() const { + return fHasValue; } explicit operator bool() const { - return fHasValue; + return this->has_value(); + } + + void reset() { + if (fHasValue) { + fPayload.fValue.~T(); + fHasValue = false; + } } private: diff --git a/tests/SkTOptionalTest.cpp b/tests/SkTOptionalTest.cpp index 228a00ebfd..232108043d 100644 --- a/tests/SkTOptionalTest.cpp +++ b/tests/SkTOptionalTest.cpp @@ -13,27 +13,137 @@ DEF_TEST(SkTOptionalEmpty, r) { skstd::optional o; REPORTER_ASSERT(r, !o); + REPORTER_ASSERT(r, !o.has_value()); } DEF_TEST(SkTOptionalValue, r) { skstd::optional o("test"); REPORTER_ASSERT(r, o); + REPORTER_ASSERT(r, o.has_value()); REPORTER_ASSERT(r, !strcmp(*o, "test")); + REPORTER_ASSERT(r, !strcmp(o.value(), "test")); + o.reset(); + REPORTER_ASSERT(r, !o); + REPORTER_ASSERT(r, !o.has_value()); } -DEF_TEST(SkTOptionalAssignment, r) { - skstd::optional> o; +class SkTOptionalTestPayload { +public: + enum State { + kConstructed, + kCopyConstructed, + kCopyAssigned, + kMoveConstructed, + kMoveAssigned, + kMovedFrom + }; + + SkTOptionalTestPayload(int payload) + : fState(kConstructed) + , fPayload(payload) {} + + SkTOptionalTestPayload(const SkTOptionalTestPayload& other) + : fState(kCopyConstructed) + , fPayload(other.fPayload) {} + + SkTOptionalTestPayload(SkTOptionalTestPayload&& other) + : fState(kMoveConstructed) + , fPayload(other.fPayload) { + other.fState = kMovedFrom; + } + + SkTOptionalTestPayload& operator=(const SkTOptionalTestPayload& other) { + fState = kCopyAssigned; + fPayload = other.fPayload; + return *this; + } + + SkTOptionalTestPayload& operator=(SkTOptionalTestPayload&& other) { + fState = kMoveAssigned; + fPayload = other.fPayload; + other.fState = kMovedFrom; + return *this; + } + State fState; + int fPayload; +}; + +DEF_TEST(SkTOptionalConstruction, r) { + skstd::optional o(1); + REPORTER_ASSERT(r, o); + REPORTER_ASSERT(r, o->fState == SkTOptionalTestPayload::kConstructed); + REPORTER_ASSERT(r, o->fPayload == 1); + + skstd::optional copy(o); + REPORTER_ASSERT(r, copy); + REPORTER_ASSERT(r, copy->fState == SkTOptionalTestPayload::kCopyConstructed); + REPORTER_ASSERT(r, copy->fPayload == 1); + REPORTER_ASSERT(r, o->fState == SkTOptionalTestPayload::kConstructed); + + skstd::optional move(std::move(o)); + REPORTER_ASSERT(r, move); + REPORTER_ASSERT(r, move->fState == SkTOptionalTestPayload::kMoveConstructed); + REPORTER_ASSERT(r, move->fPayload == 1); + // NOLINTNEXTLINE(bugprone-use-after-move) + REPORTER_ASSERT(r, o->fState == SkTOptionalTestPayload::kMovedFrom); +} + +DEF_TEST(SkTOptionalMoveAssignment, r) { + skstd::optional o; REPORTER_ASSERT(r, !o); - o = skstd::optional>(50); - REPORTER_ASSERT(r, o); - REPORTER_ASSERT(r, o->capacity() == 50); + // assign to an empty optional from an empty optional + o = skstd::optional(); + REPORTER_ASSERT(r, !o); - o = skstd::optional>(SkTArray(100)); + // assign to an empty optional from a full optional + skstd::optional full(1); + o = std::move(full); REPORTER_ASSERT(r, o); - REPORTER_ASSERT(r, o->capacity() == 100); + REPORTER_ASSERT(r, o->fState == SkTOptionalTestPayload::kMoveConstructed); + REPORTER_ASSERT(r, o->fPayload == 1); + // NOLINTNEXTLINE(bugprone-use-after-move) + REPORTER_ASSERT(r, full->fState == SkTOptionalTestPayload::kMovedFrom); - o = skstd::optional>(); + // assign to a full optional from a full optional + full = skstd::optional(2); + o = std::move(full); + REPORTER_ASSERT(r, o); + REPORTER_ASSERT(r, o->fState == SkTOptionalTestPayload::kMoveAssigned); + REPORTER_ASSERT(r, o->fPayload == 2); + // NOLINTNEXTLINE(bugprone-use-after-move) + REPORTER_ASSERT(r, full->fState == SkTOptionalTestPayload::kMovedFrom); + + // assign to a full optional from an empty optional + o = skstd::optional(); + REPORTER_ASSERT(r, !o); +} + +DEF_TEST(SkTOptionalCopyAssignment, r) { + skstd::optional o; + REPORTER_ASSERT(r, !o); + + skstd::optional empty; + skstd::optional full(1); + + // assign to an empty optional from an empty optional + o = empty; + REPORTER_ASSERT(r, !o); + + // assign to an empty optional from a full optional + o = full; + REPORTER_ASSERT(r, o); + REPORTER_ASSERT(r, o->fState == SkTOptionalTestPayload::kCopyConstructed); + REPORTER_ASSERT(r, o->fPayload == 1); + + // assign to a full optional from a full optional + o = full; + REPORTER_ASSERT(r, o); + REPORTER_ASSERT(r, o->fState == SkTOptionalTestPayload::kCopyAssigned); + REPORTER_ASSERT(r, o->fPayload == 1); + + // assign to a full optional from an empty optional + o = empty; REPORTER_ASSERT(r, !o); } @@ -59,11 +169,16 @@ DEF_TEST(SkTOptionalNoDefaultConstructor, r) { DEF_TEST(SkTOptionalSelfAssignment, r) { skstd::optional empty; skstd::optional& emptyRef = empty; + empty = emptyRef; + REPORTER_ASSERT(r, !empty); empty = std::move(emptyRef); REPORTER_ASSERT(r, !empty); skstd::optional full("full"); skstd::optional& fullRef = full; + full = fullRef; + REPORTER_ASSERT(r, full); + REPORTER_ASSERT(r, *full == SkString("full")); full = std::move(fullRef); REPORTER_ASSERT(r, full); REPORTER_ASSERT(r, *full == SkString("full"));