From d16772a31ec8b4429996c0f6fb4871b5162ec6fd Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sun, 18 Mar 2018 19:04:08 -0700 Subject: [PATCH] Sealable: fixed semantic; atomic type for the flag variable Summary: * Fixed semantic: all kinds of derivative instances lose `sealed` flag (which is expected); * Using atomic for `sealed_` ivar. Reviewed By: fkgozali Differential Revision: D7230674 fbshipit-source-id: abe786610c20a45a0fabb9068120e24adeeeac7f --- .../fabric/core/primitives/Sealable.cpp | 30 +++++++++++++++++++ ReactCommon/fabric/core/primitives/Sealable.h | 25 ++++++++++++---- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/ReactCommon/fabric/core/primitives/Sealable.cpp b/ReactCommon/fabric/core/primitives/Sealable.cpp index fc4bb8c25..a93f640db 100644 --- a/ReactCommon/fabric/core/primitives/Sealable.cpp +++ b/ReactCommon/fabric/core/primitives/Sealable.cpp @@ -12,6 +12,36 @@ namespace facebook { namespace react { +/* + * Note: + * We must explictly implement all *the rule of five* methods because: + * 1. Using `std::atomic` behind `sealed_` implicitly deletes default + * constructors; + * 2. We have to establish behaviour where any new cloned or moved instances + * of the object lose `sealed` flag. + * + * See more about the rule of three/five/zero: + * http://en.cppreference.com/w/cpp/language/rule_of_three + */ + +Sealable::Sealable(): sealed_(false) {} + +Sealable::Sealable(const Sealable& other): sealed_(false) {}; + +Sealable::Sealable(Sealable&& other) noexcept: sealed_(false) {}; + +Sealable::~Sealable() noexcept {}; + +Sealable& Sealable::operator= (const Sealable& other) { + ensureUnsealed(); + return *this; +} + +Sealable& Sealable::operator= (Sealable&& other) noexcept { + ensureUnsealed(); + return *this; +}; + void Sealable::seal() const { sealed_ = true; } diff --git a/ReactCommon/fabric/core/primitives/Sealable.h b/ReactCommon/fabric/core/primitives/Sealable.h index 1f8fa0f09..e0d168e44 100644 --- a/ReactCommon/fabric/core/primitives/Sealable.h +++ b/ReactCommon/fabric/core/primitives/Sealable.h @@ -7,21 +7,27 @@ #pragma once +#import + namespace facebook { namespace react { /* - * Represents something which can be *sealed* (imperatively marked as immutable). + * Represents an object which can be *sealed* (imperatively marked as immutable). + * + * The `sealed` flag is tight to a particular instance of the class and resets + * to `false` for all newly created (by copy-constructor, assignment operator + * and so on) derivative objects. * * Why do we need this? In Fabric, some objects are semi-immutable - * even if they explicitly marked as `const`. It means that in some special - * cases those objects can be const-cast-away and then mutated. That comes from + * even if they are explicitly marked as `const`. It means that in some special + * cases those objects can be const-casted-away and then mutated. That comes from * the fact that we share some object's life-cycle responsibilities with React * and the immutability is guaranteed by some logic splitted between native and - * JavaScript worlds (which makes impossible to fully use immutability + * JavaScript worlds (which makes it impossible to fully use immutability * enforcement at a language level). * To detect possible errors as early as possible we additionally mark objects - * as *sealed* after some stage and then enforce this at run-time. + * as *sealed* after some stages and then enforce this at run-time. * * How to use: * 1. Inherit your class from `Sealable`. @@ -31,6 +37,13 @@ namespace react { */ class Sealable { public: + Sealable(); + Sealable(const Sealable& other); + Sealable(Sealable&& other) noexcept; + ~Sealable() noexcept; + Sealable& operator=(const Sealable& other); + Sealable& operator=(Sealable&& other) noexcept; + /* * Seals the object. This operation is irreversible; * the object cannot be "unsealed" after being sealing. @@ -50,7 +63,7 @@ protected: void ensureUnsealed() const; private: - mutable bool sealed_ {false}; + mutable std::atomic sealed_ {false}; }; } // namespace react