From dce1863bf9173495251e6a0de97bdaee8b4d9046 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Sun, 31 Jan 2021 15:07:41 -0800 Subject: [PATCH] Put moved from AttributedStringBox into consistent state Summary: Changelog: [internal] Fixes an inconsistency that `AttributedStringBox` can get into. Example of inconsitency: After `AttributedStringBox` is moved (move constructor or move assignment operator), moved from `AttributedStringBox` needs to be set into blank state. Its mode needs to be `Value`, `opaquePointer_` should be nullptr and `value_` empty AttributedString. This was not the case before as the default move constructor and operator would leave `mode_` as `OpaquePointer` but ivar representing opaquePointer would be nullptr. Reviewed By: JoshuaGross Differential Revision: D26168142 fbshipit-source-id: eed2a7c3a165ae5e1f269822c12042c6ccbd3388 --- .../attributedstring/AttributedStringBox.cpp | 24 +++++++++++++++++-- .../attributedstring/AttributedStringBox.h | 4 ++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/ReactCommon/react/renderer/attributedstring/AttributedStringBox.cpp b/ReactCommon/react/renderer/attributedstring/AttributedStringBox.cpp index 5983a7fc754..83d10b365bf 100644 --- a/ReactCommon/react/renderer/attributedstring/AttributedStringBox.cpp +++ b/ReactCommon/react/renderer/attributedstring/AttributedStringBox.cpp @@ -24,6 +24,14 @@ AttributedStringBox::AttributedStringBox( std::shared_ptr const &opaquePointer) : mode_(Mode::OpaquePointer), value_({}), opaquePointer_(opaquePointer) {} +AttributedStringBox::AttributedStringBox(AttributedStringBox &&other) noexcept + : mode_(other.mode_), + value_(std::move(other.value_)), + opaquePointer_(std::move(other.opaquePointer_)) { + other.mode_ = AttributedStringBox::Mode::Value; + other.value_ = std::make_shared(AttributedString{}); +} + AttributedStringBox::Mode AttributedStringBox::getMode() const { return mode_; } @@ -40,6 +48,18 @@ std::shared_ptr AttributedStringBox::getOpaquePointer() const { return opaquePointer_; } +AttributedStringBox &AttributedStringBox::operator=( + AttributedStringBox &&other) { + if (this != &other) { + mode_ = other.mode_; + value_ = std::move(other.value_); + opaquePointer_ = std::move(other.opaquePointer_); + other.mode_ = AttributedStringBox::Mode::Value; + other.value_ = std::make_shared(AttributedString{}); + } + return *this; +} + bool operator==( AttributedStringBox const &lhs, AttributedStringBox const &rhs) { @@ -48,9 +68,9 @@ bool operator==( } switch (lhs.getMode()) { - case facebook::react::AttributedStringBox::Mode::Value: + case AttributedStringBox::Mode::Value: return lhs.getValue() == rhs.getValue(); - case facebook::react::AttributedStringBox::Mode::OpaquePointer: + case AttributedStringBox::Mode::OpaquePointer: return lhs.getOpaquePointer() == rhs.getOpaquePointer(); } } diff --git a/ReactCommon/react/renderer/attributedstring/AttributedStringBox.h b/ReactCommon/react/renderer/attributedstring/AttributedStringBox.h index 7965a7d6d28..b8bd96c9cae 100644 --- a/ReactCommon/react/renderer/attributedstring/AttributedStringBox.h +++ b/ReactCommon/react/renderer/attributedstring/AttributedStringBox.h @@ -41,9 +41,9 @@ class AttributedStringBox final { * Movable, Copyable, Assignable. */ AttributedStringBox(AttributedStringBox const &other) = default; - AttributedStringBox(AttributedStringBox &&other) noexcept = default; + AttributedStringBox(AttributedStringBox &&other) noexcept; AttributedStringBox &operator=(AttributedStringBox const &other) = default; - AttributedStringBox &operator=(AttributedStringBox &&other) = default; + AttributedStringBox &operator=(AttributedStringBox &&other); /* * Getters.