Skip to content

Commit

Permalink
Delete traitCast and identifier traits (#42748)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #42748

React Native has globally enabled RTTI within `rn_xplat_cxx_library`, ahead of RTTI being forced on (regardless of flag) in Android apps (it was previously enabled everywhere but Android, which has caused us no small share of headaches, with public JSI APIs designed around clients using RTTI).

This diff:
1. Mechanically replaces usages of `traitCast` with equivalent calls to `dynamic_cast` or `dynamic_pointer_cast`
    1. These have similar semantics as current iteration of `traitCast`, where we return `nullptr` for pointer form, or throw on invalid cast for reference form.
2. Removes `IdentifierTrait` as a requirement to cast to a ShadowNode
3. Removes the ShadowNode traits used solely as cast identities

This enables consistent usage of `dynamic_cast` (including for user defined ShadowNodes), and also exposes some places where `traitCast` allowed implicit const conversion.

The OSS builds should already have RTTI on, and will be able to use `dynamic_cast` on RN provided types (`traitCast` is not extendable).

Changelog:
[General][Breaking] - Delete traitCast and identifier traits

Reviewed By: sammy-SC

Differential Revision: D53215009

fbshipit-source-id: d20cbf66b725f5565fa5d03332010d87f2b08b61
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Feb 1, 2024
1 parent 810c205 commit 1170a68
Show file tree
Hide file tree
Showing 26 changed files with 75 additions and 371 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ add_executable(reactnative_unittest
${REACT_COMMON_DIR}/react/renderer/core/tests/PrimitivesTest.cpp
${REACT_COMMON_DIR}/react/renderer/core/tests/RawPropsTest.cpp
${REACT_COMMON_DIR}/react/renderer/core/tests/ShadowNodeFamilyTest.cpp
${REACT_COMMON_DIR}/react/renderer/core/tests/traitCastTest.cpp
${REACT_COMMON_DIR}/react/renderer/debug/tests/DebugStringConvertibleTest.cpp
${REACT_COMMON_DIR}/react/renderer/element/tests/ElementTest.cpp
${REACT_COMMON_DIR}/react/renderer/graphics/tests/GraphicsTest.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include <react/renderer/components/text/RawTextShadowNode.h>
#include <react/renderer/components/text/TextProps.h>
#include <react/renderer/components/text/TextShadowNode.h>
#include <react/renderer/core/TraitCast.h>
#include <react/renderer/mounting/ShadowView.h>

namespace facebook::react {
Expand All @@ -33,7 +32,7 @@ void BaseTextShadowNode::buildAttributedString(
for (const auto& childNode : parentNode.getChildren()) {
// RawShadowNode
auto rawTextShadowNode =
traitCast<const RawTextShadowNode*>(childNode.get());
dynamic_cast<const RawTextShadowNode*>(childNode.get());
if (rawTextShadowNode != nullptr) {
auto fragment = AttributedString::Fragment{};
fragment.string = rawTextShadowNode->getConcreteProps().text;
Expand All @@ -49,7 +48,7 @@ void BaseTextShadowNode::buildAttributedString(
}

// TextShadowNode
auto textShadowNode = traitCast<const TextShadowNode*>(childNode.get());
auto textShadowNode = dynamic_cast<const TextShadowNode*>(childNode.get());
if (textShadowNode != nullptr) {
auto localTextAttributes = baseTextAttributes;
localTextAttributes.apply(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include <react/renderer/attributedstring/AttributedStringBox.h>
#include <react/renderer/components/view/ViewShadowNode.h>
#include <react/renderer/components/view/conversions.h>
#include <react/renderer/core/TraitCast.h>
#include <react/renderer/graphics/rounding.h>
#include <react/renderer/telemetry/TransactionTelemetry.h>
#include <react/renderer/textlayoutmanager/TextLayoutContext.h>
Expand All @@ -31,7 +30,7 @@ ParagraphShadowNode::ParagraphShadowNode(
const ShadowNodeFragment& fragment)
: ConcreteViewShadowNode(sourceShadowNode, fragment) {
auto& sourceParagraphShadowNode =
traitCast<ParagraphShadowNode const&>(sourceShadowNode);
dynamic_cast<const ParagraphShadowNode&>(sourceShadowNode);
if (!fragment.children && !fragment.props &&
sourceParagraphShadowNode.getIsLayoutClean()) {
// This ParagraphShadowNode was cloned but did not change
Expand Down Expand Up @@ -84,7 +83,7 @@ Content ParagraphShadowNode::getContentWithMeasuredAttachments(

for (const auto& attachment : content.attachments) {
auto laytableShadowNode =
traitCast<const LayoutableShadowNode*>(attachment.shadowNode);
dynamic_cast<const LayoutableShadowNode*>(attachment.shadowNode);

if (laytableShadowNode == nullptr) {
continue;
Expand Down Expand Up @@ -213,7 +212,7 @@ void ParagraphShadowNode::layout(LayoutContext layoutContext) {
for (size_t i = 0; i < content.attachments.size(); i++) {
auto& attachment = content.attachments.at(i);

if (traitCast<const LayoutableShadowNode*>(attachment.shadowNode) ==
if (dynamic_cast<const LayoutableShadowNode*>(attachment.shadowNode) ==
nullptr) {
// Not a layoutable `ShadowNode`, no need to lay it out.
continue;
Expand All @@ -231,7 +230,7 @@ void ParagraphShadowNode::layout(LayoutContext layoutContext) {
static_cast<ParagraphShadowNode*>(paragraphOwningShadowNode.get());

auto& layoutableShadowNode =
traitCast<LayoutableShadowNode&>(*clonedShadowNode);
dynamic_cast<LayoutableShadowNode&>(*clonedShadowNode);

auto attachmentFrame = measurement.attachments[i].frame;
auto attachmentSize = roundToPixel<&ceil>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class ParagraphShadowNode final : public ConcreteViewShadowNode<
static ShadowNodeTraits BaseTraits() {
auto traits = ConcreteViewShadowNode::BaseTraits();
traits.set(ShadowNodeTraits::Trait::LeafYogaNode);
traits.set(ShadowNodeTraits::Trait::TextKind);
traits.set(ShadowNodeTraits::Trait::MeasurableYogaNode);

#ifdef ANDROID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@ class RawTextShadowNode : public ConcreteShadowNode<
RawTextProps> {
public:
using ConcreteShadowNode::ConcreteShadowNode;
static ShadowNodeTraits BaseTraits() {
auto traits = ConcreteShadowNode::BaseTraits();
traits.set(IdentifierTrait());
return traits;
}
static ShadowNodeTraits::Trait IdentifierTrait() {
return ShadowNodeTraits::Trait::RawText;
}
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,12 @@ class TextShadowNode : public ConcreteShadowNode<
public:
static ShadowNodeTraits BaseTraits() {
auto traits = ConcreteShadowNode::BaseTraits();

#ifdef ANDROID
traits.set(ShadowNodeTraits::Trait::FormsView);
#endif
traits.set(IdentifierTrait());

return traits;
}

static ShadowNodeTraits::Trait IdentifierTrait() {
return ShadowNodeTraits::Trait::Text;
}

using ConcreteShadowNode::ConcreteShadowNode;

#ifdef ANDROID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class AndroidTextInputShadowNode final : public ConcreteViewShadowNode<
public:
static ShadowNodeTraits BaseTraits() {
auto traits = ConcreteViewShadowNode::BaseTraits();
traits.set(ShadowNodeTraits::Trait::TextKind);
traits.set(ShadowNodeTraits::Trait::LeafYogaNode);
return traits;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class TextInputShadowNode final : public ConcreteViewShadowNode<

static ShadowNodeTraits BaseTraits() {
auto traits = ConcreteViewShadowNode::BaseTraits();
traits.set(ShadowNodeTraits::Trait::TextKind);
traits.set(ShadowNodeTraits::Trait::LeafYogaNode);
traits.set(ShadowNodeTraits::Trait::MeasurableYogaNode);
return traits;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ class ViewShadowNode final : public ConcreteViewShadowNode<
ViewShadowNodeProps,
ViewEventEmitter> {
public:
static ShadowNodeTraits BaseTraits() {
auto traits = ConcreteViewShadowNode::BaseTraits();
traits.set(ShadowNodeTraits::Trait::View);
return traits;
}

ViewShadowNode(
const ShadowNodeFragment& fragment,
const ShadowNodeFamily::Shared& family,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <react/renderer/components/view/conversions.h>
#include <react/renderer/core/LayoutConstraints.h>
#include <react/renderer/core/LayoutContext.h>
#include <react/renderer/core/TraitCast.h>
#include <react/renderer/debug/DebugStringConvertibleItem.h>
#include <react/renderer/debug/SystraceSection.h>
#include <react/utils/CoreFeatures.h>
Expand All @@ -25,6 +24,8 @@

namespace facebook::react {

static_assert(RawPropsFilterable<YogaLayoutableShadowNode>);

static int FabricDefaultYogaLog(
const YGConfigConstRef /*unused*/,
const YGNodeConstRef /*unused*/,
Expand Down Expand Up @@ -62,16 +63,6 @@ static int FabricDefaultYogaLog(

thread_local LayoutContext threadLocalLayoutContext;

ShadowNodeTraits YogaLayoutableShadowNode::BaseTraits() {
auto traits = LayoutableShadowNode::BaseTraits();
traits.set(IdentifierTrait());
return traits;
}

ShadowNodeTraits::Trait YogaLayoutableShadowNode::IdentifierTrait() {
return ShadowNodeTraits::Trait::YogaLayoutableKind;
}

YogaLayoutableShadowNode::YogaLayoutableShadowNode(
const ShadowNodeFragment& fragment,
const ShadowNodeFamily::Shared& family,
Expand Down Expand Up @@ -123,8 +114,10 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode(

if (!getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)) {
for (auto& child : getChildren()) {
if (auto layoutableChild = traitCast<YogaLayoutableShadowNode>(child)) {
yogaLayoutableChildren_.push_back(layoutableChild);
if (auto layoutableChild =
std::dynamic_pointer_cast<const YogaLayoutableShadowNode>(
child)) {
yogaLayoutableChildren_.push_back(std::move(layoutableChild));
}
}
}
Expand Down Expand Up @@ -210,7 +203,7 @@ void YogaLayoutableShadowNode::adoptYogaChild(size_t index) {
!getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode));

auto& childNode =
traitCast<const YogaLayoutableShadowNode&>(*getChildren().at(index));
dynamic_cast<const YogaLayoutableShadowNode&>(*getChildren().at(index));

if (childNode.yogaNode_.getOwner() == nullptr) {
// The child node is not owned.
Expand Down Expand Up @@ -243,7 +236,8 @@ void YogaLayoutableShadowNode::appendChild(
}

if (auto yogaLayoutableChild =
traitCast<YogaLayoutableShadowNode>(childNode)) {
std::dynamic_pointer_cast<const YogaLayoutableShadowNode>(
childNode)) {
// Here we don't have information about the previous structure of the node
// (if it that existed before), so we don't have anything to compare the
// Yoga node with (like a previous version of this node). Therefore we must
Expand Down Expand Up @@ -273,8 +267,9 @@ void YogaLayoutableShadowNode::replaceChild(
ensureYogaChildrenLookFine();

auto layoutableOldChild =
traitCast<const YogaLayoutableShadowNode*>(&oldChild);
auto layoutableNewChild = traitCast<YogaLayoutableShadowNode>(newChild);
dynamic_cast<const YogaLayoutableShadowNode*>(&oldChild);
auto layoutableNewChild =
std::dynamic_pointer_cast<const YogaLayoutableShadowNode>(newChild);

if (layoutableOldChild == nullptr && layoutableNewChild == nullptr) {
// No need to mutate yogaLayoutableChildren_
Expand Down Expand Up @@ -349,7 +344,8 @@ void YogaLayoutableShadowNode::updateYogaChildren() {

for (size_t i = 0; i < getChildren().size(); i++) {
if (auto yogaLayoutableChild =
traitCast<YogaLayoutableShadowNode>(getChildren()[i])) {
std::dynamic_pointer_cast<const YogaLayoutableShadowNode>(
getChildren()[i])) {
appendYogaChild(yogaLayoutableChild);
adoptYogaChild(i);

Expand Down Expand Up @@ -496,7 +492,7 @@ void YogaLayoutableShadowNode::configureYogaTree(
}

YGErrata YogaLayoutableShadowNode::resolveErrata(YGErrata defaultErrata) const {
if (auto viewShadowNode = traitCast<const ViewShadowNode*>(this)) {
if (auto viewShadowNode = dynamic_cast<const ViewShadowNode*>(this)) {
const auto& props = viewShadowNode->getConcreteProps();
switch (props.experimental_layoutConformance) {
case LayoutConformance::Classic:
Expand Down Expand Up @@ -745,7 +741,7 @@ Rect YogaLayoutableShadowNode::getContentBounds() const {

auto layoutMetricsWithOverflowInset = childNode.getLayoutMetrics();
if (layoutMetricsWithOverflowInset.displayType != DisplayType::None) {
auto viewChildNode = traitCast<const ViewShadowNode*>(&childNode);
auto viewChildNode = dynamic_cast<const ViewShadowNode*>(&childNode);
auto hitSlop = viewChildNode != nullptr
? viewChildNode->getConcreteProps().hitSlop
: EdgeInsets{};
Expand Down Expand Up @@ -776,6 +772,13 @@ Rect YogaLayoutableShadowNode::getContentBounds() const {
return contentBounds;
}

/*static*/ void YogaLayoutableShadowNode::filterRawProps(RawProps& rawProps) {
if (CoreFeatures::excludeYogaFromRawProps) {
// TODO: this shouldn't live in RawProps
rawProps.filterYogaStylePropsInDynamicConversion();
}
}

#pragma mark - Yoga Connectors

YGNodeRef YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector(
Expand Down Expand Up @@ -837,7 +840,7 @@ YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector(

YogaLayoutableShadowNode& YogaLayoutableShadowNode::shadowNodeFromContext(
YGNodeConstRef yogaNode) {
return traitCast<YogaLayoutableShadowNode&>(
return dynamic_cast<YogaLayoutableShadowNode&>(
*static_cast<ShadowNode*>(YGNodeGetContext(yogaNode)));
}

Expand Down Expand Up @@ -1014,7 +1017,7 @@ void YogaLayoutableShadowNode::ensureYogaChildrenAlignment() const {
auto& child = children.at(i);
react_native_assert(
yogaChild->getContext() ==
traitCast<const YogaLayoutableShadowNode*>(child.get()));
dynamic_cast<const YogaLayoutableShadowNode*>(child.get()));
}
#endif
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode {
using Shared = std::shared_ptr<const YogaLayoutableShadowNode>;
using ListOfShared = std::vector<Shared>;

static ShadowNodeTraits BaseTraits();
static ShadowNodeTraits::Trait IdentifierTrait();

#pragma mark - Constructors

YogaLayoutableShadowNode(
Expand Down Expand Up @@ -88,6 +85,8 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode {

Rect getContentBounds() const;

static void filterRawProps(RawProps& rawProps);

protected:
/*
* Yoga config associated (only) with this particular node.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,8 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
return ShadowNodeT::defaultSharedProps();
}

if (CoreFeatures::excludeYogaFromRawProps) {
if (ShadowNodeT::IdentifierTrait() ==
ShadowNodeTraits::Trait::YogaLayoutableKind) {
rawProps.filterYogaStylePropsInDynamicConversion();
}
if constexpr (RawPropsFilterable<ShadowNodeT>) {
ShadowNodeT::filterRawProps(rawProps);
}

rawProps.parse(rawPropsParser_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ class ConcreteShadowNode : public BaseShadowNodeT {
return BaseShadowNodeT::BaseTraits();
}

static ShadowNodeTraits::Trait IdentifierTrait() {
return BaseShadowNodeT::IdentifierTrait();
}

static UnsharedConcreteProps Props(
const PropsParserContext& context,
const RawProps& rawProps,
Expand Down
Loading

0 comments on commit 1170a68

Please sign in to comment.