Skip to content

Commit

Permalink
Fabric: Fixing const correctness in ShadowNodeFragment (and some pret…
Browse files Browse the repository at this point in the history
…tification)

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
  • Loading branch information
shergin authored and facebook-github-bot committed Apr 6, 2019
1 parent e34761f commit e5feb0c
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 32 deletions.
2 changes: 2 additions & 0 deletions ReactCommon/fabric/core/events/EventEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class EventEmitter {
using Tag = int32_t;

public:
using Shared = std::shared_ptr<EventEmitter const>;

static std::mutex &DispatchMutex();

static ValueFactory defaultPayloadFactory();
Expand Down
2 changes: 2 additions & 0 deletions ReactCommon/fabric/core/shadownode/LocalData.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ using SharedLocalData = std::shared_ptr<const LocalData>;
*/
class LocalData : public Sealable, public DebugStringConvertible {
public:
using Shared = std::shared_ptr<LocalData const>;

virtual ~LocalData() = default;

virtual folly::dynamic getDynamic() const {
Expand Down
12 changes: 7 additions & 5 deletions ReactCommon/fabric/core/shadownode/Props.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,20 @@ namespace react {

class Props;

using SharedProps = std::shared_ptr<const Props>;
using SharedProps = std::shared_ptr<Props const>;

/*
* Represents the most generic props object.
*/
class Props : public virtual Sealable, public virtual DebugStringConvertible {
public:
using Shared = std::shared_ptr<Props const>;

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
Expand All @@ -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
};

Expand Down
10 changes: 8 additions & 2 deletions ReactCommon/fabric/core/shadownode/ShadowNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,14 @@ class ShadowNode : public virtual Sealable,
public virtual DebugStringConvertible,
public std::enable_shared_from_this<ShadowNode> {
public:
using Shared = std::shared_ptr<const ShadowNode>;
using Weak = std::weak_ptr<const ShadowNode>;
using Shared = std::shared_ptr<ShadowNode const>;
using Weak = std::weak_ptr<ShadowNode const>;
using Unshared = std::shared_ptr<ShadowNode>;
using ListOfShared =
better::small_vector<Shared, kShadowNodeChildrenSmallVectorSize>;
using SharedListOfShared = std::shared_ptr<ListOfShared const>;
using UnsharedListOfShared = std::shared_ptr<ListOfShared>;

using AncestorList = better::small_vector<
std::pair<
std::reference_wrapper<ShadowNode const> /* parentNode */,
Expand Down
23 changes: 12 additions & 11 deletions ReactCommon/fabric/core/shadownode/ShadowNodeFragment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
33 changes: 19 additions & 14 deletions ReactCommon/fabric/core/shadownode/ShadowNodeFragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e5feb0c

Please sign in to comment.