From e5feb0cee3f377258cb4901b41c60707e1b87c49 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sat, 6 Apr 2019 09:11:16 -0700 Subject: [PATCH] Fabric: Fixing const correctness in ShadowNodeFragment (and some prettification) Summary: Previously, all placeholders methods have return type `SomeType &` which is not correct because it allows the called to modify enclosed `static` value of the placeholders; the type should be `SomeType const &`. Besides that this diff migrates some type aliases to the new style (which makes evething much prettier and readable). Reviewed By: mdvacca Differential Revision: D14819076 fbshipit-source-id: 87e68d546f16d7a9ad1fb65e1b238859f9381eb7 --- ReactCommon/fabric/core/events/EventEmitter.h | 2 ++ .../fabric/core/shadownode/LocalData.h | 2 ++ ReactCommon/fabric/core/shadownode/Props.h | 12 ++++--- .../fabric/core/shadownode/ShadowNode.h | 10 ++++-- .../core/shadownode/ShadowNodeFragment.cpp | 23 ++++++------- .../core/shadownode/ShadowNodeFragment.h | 33 +++++++++++-------- 6 files changed, 50 insertions(+), 32 deletions(-) diff --git a/ReactCommon/fabric/core/events/EventEmitter.h b/ReactCommon/fabric/core/events/EventEmitter.h index f3ce76ca4f1..f819e56b024 100644 --- a/ReactCommon/fabric/core/events/EventEmitter.h +++ b/ReactCommon/fabric/core/events/EventEmitter.h @@ -35,6 +35,8 @@ class EventEmitter { using Tag = int32_t; public: + using Shared = std::shared_ptr; + static std::mutex &DispatchMutex(); static ValueFactory defaultPayloadFactory(); diff --git a/ReactCommon/fabric/core/shadownode/LocalData.h b/ReactCommon/fabric/core/shadownode/LocalData.h index d62b3d29128..83680c3853b 100644 --- a/ReactCommon/fabric/core/shadownode/LocalData.h +++ b/ReactCommon/fabric/core/shadownode/LocalData.h @@ -28,6 +28,8 @@ using SharedLocalData = std::shared_ptr; */ class LocalData : public Sealable, public DebugStringConvertible { public: + using Shared = std::shared_ptr; + virtual ~LocalData() = default; virtual folly::dynamic getDynamic() const { diff --git a/ReactCommon/fabric/core/shadownode/Props.h b/ReactCommon/fabric/core/shadownode/Props.h index 56362dddf87..3c07d2b4c9e 100644 --- a/ReactCommon/fabric/core/shadownode/Props.h +++ b/ReactCommon/fabric/core/shadownode/Props.h @@ -18,18 +18,20 @@ namespace react { class Props; -using SharedProps = std::shared_ptr; +using SharedProps = std::shared_ptr; /* * Represents the most generic props object. */ class Props : public virtual Sealable, public virtual DebugStringConvertible { public: + using Shared = std::shared_ptr; + Props() = default; - Props(const Props &sourceProps, const RawProps &rawProps); + Props(Props const &sourceProps, RawProps const &rawProps); virtual ~Props() = default; - const std::string nativeId; + std::string const nativeId; /* * Special value that represents generation number of `Props` object, which @@ -38,10 +40,10 @@ class Props : public virtual Sealable, public virtual DebugStringConvertible { * revision equals `0`. * The value might be used for optimization purposes. */ - const int revision{0}; + int const revision{0}; #ifdef ANDROID - const folly::dynamic rawProps = folly::dynamic::object(); + folly::dynamic const rawProps = folly::dynamic::object(); #endif }; diff --git a/ReactCommon/fabric/core/shadownode/ShadowNode.h b/ReactCommon/fabric/core/shadownode/ShadowNode.h index 2d34ba43f7f..773406b4df1 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNode.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNode.h @@ -43,8 +43,14 @@ class ShadowNode : public virtual Sealable, public virtual DebugStringConvertible, public std::enable_shared_from_this { public: - using Shared = std::shared_ptr; - using Weak = std::weak_ptr; + using Shared = std::shared_ptr; + using Weak = std::weak_ptr; + using Unshared = std::shared_ptr; + using ListOfShared = + better::small_vector; + using SharedListOfShared = std::shared_ptr; + using UnsharedListOfShared = std::shared_ptr; + using AncestorList = better::small_vector< std::pair< std::reference_wrapper /* parentNode */, diff --git a/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.cpp b/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.cpp index 18bce2d4558..4e31b42b2d2 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.cpp +++ b/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.cpp @@ -10,35 +10,36 @@ namespace facebook { namespace react { -Tag ShadowNodeFragment::tagPlaceholder() { +Tag const ShadowNodeFragment::tagPlaceholder() { return 0; } -Tag ShadowNodeFragment::surfaceIdPlaceholder() { +SurfaceId const ShadowNodeFragment::surfaceIdPlaceholder() { return 0; } -SharedProps &ShadowNodeFragment::propsPlaceholder() { - static auto &instance = *new SharedProps(); +Props::Shared const &ShadowNodeFragment::propsPlaceholder() { + static auto &instance = *new Props::Shared(); return instance; } -SharedEventEmitter &ShadowNodeFragment::eventEmitterPlaceholder() { - static auto &instance = *new SharedEventEmitter(); +EventEmitter::Shared const &ShadowNodeFragment::eventEmitterPlaceholder() { + static auto &instance = *new EventEmitter::Shared(); return instance; } -SharedShadowNodeSharedList &ShadowNodeFragment::childrenPlaceholder() { - static auto &instance = *new SharedShadowNodeSharedList(); +ShadowNode::SharedListOfShared const & +ShadowNodeFragment::childrenPlaceholder() { + static auto &instance = *new ShadowNode::SharedListOfShared(); return instance; } -SharedLocalData &ShadowNodeFragment::localDataPlaceholder() { - static auto &instance = *new SharedLocalData(); +LocalData::Shared const &ShadowNodeFragment::localDataPlaceholder() { + static auto &instance = *new LocalData::Shared(); return instance; } -State::Shared &ShadowNodeFragment::statePlaceholder() { +State::Shared const &ShadowNodeFragment::statePlaceholder() { static auto &instance = *new State::Shared(); return instance; } diff --git a/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.h b/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.h index 1c77b09572c..754e0553189 100644 --- a/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.h +++ b/ReactCommon/fabric/core/shadownode/ShadowNodeFragment.h @@ -25,21 +25,26 @@ namespace react { * retain ownership of them. */ struct ShadowNodeFragment { - Tag tag = 0; - Tag rootTag = 0; - const SharedProps &props = propsPlaceholder(); - const SharedEventEmitter &eventEmitter = eventEmitterPlaceholder(); - const SharedShadowNodeSharedList &children = childrenPlaceholder(); - const SharedLocalData &localData = localDataPlaceholder(); - const State::Shared &state = statePlaceholder(); + Tag const tag = tagPlaceholder(); + SurfaceId const rootTag = surfaceIdPlaceholder(); + Props::Shared const &props = propsPlaceholder(); + EventEmitter::Shared const &eventEmitter = eventEmitterPlaceholder(); + ShadowNode::SharedListOfShared const &children = childrenPlaceholder(); + LocalData::Shared const &localData = localDataPlaceholder(); + State::Shared const &state = statePlaceholder(); - static Tag tagPlaceholder(); - static Tag surfaceIdPlaceholder(); - static SharedProps &propsPlaceholder(); - static SharedEventEmitter &eventEmitterPlaceholder(); - static SharedShadowNodeSharedList &childrenPlaceholder(); - static SharedLocalData &localDataPlaceholder(); - static State::Shared &statePlaceholder(); + /* + * Placeholders. + * Use as default arguments as an indication that the field does not need to + * be changed. + */ + static Tag const tagPlaceholder(); + static SurfaceId const surfaceIdPlaceholder(); + static Props::Shared const &propsPlaceholder(); + static EventEmitter::Shared const &eventEmitterPlaceholder(); + static ShadowNode::SharedListOfShared const &childrenPlaceholder(); + static LocalData::Shared const &localDataPlaceholder(); + static State::Shared const &statePlaceholder(); }; } // namespace react