Skip to content

Commit

Permalink
Breaking: size_t indices (facebook#39371)
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/yoga#1366

Pull Request resolved: facebook#39371

Yoga's public API exposes indices most often as `uint32_t`, with exception of clone callbacks which are `int32_t`. Yoga internally represents these indices as `size_t` when dealing with the child vector, and this is the true index.

This changes the API to consistently be `size_t`. This should not be breaking for most users, but will cause breaks where:

1. Users set a clone node callback (I think this should be rare. RN uses it, but only because it relies on a separate private API).
2. Callers of `YGNodeGetChildCount()` are assigning to an int with less width than `size_t` and have strong warnings enabled.
3. Using a newer Yoga binary with older source, since we are not preserving ABI compatibility (Yoga in general does not aim to be ABI stable between major versions, only ABI safe for a given set of sources).

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D49130914

fbshipit-source-id: 6fd72245cd8de59159a254dd98690c9dc2d81af0
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Sep 12, 2023
1 parent 9ad0e7d commit 3cc9ce9
Show file tree
Hide file tree
Showing 14 changed files with 63 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ static void YGTransferLayoutOutputsRecursive(

YGNodeSetHasNewLayout(root, false);

for (uint32_t i = 0; i < YGNodeGetChildCount(root); i++) {
for (size_t i = 0; i < YGNodeGetChildCount(root); i++) {
YGTransferLayoutOutputsRecursive(
env, thiz, YGNodeGetChild(root, i), layoutContext);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ void YogaLayoutableShadowNode::appendYogaChild(
ensureYogaChildrenLookFine();

yogaLayoutableChildren_.push_back(childNode);
yogaNode_.insertChild(
&childNode->yogaNode_,
static_cast<uint32_t>(yogaNode_.getChildren().size()));
yogaNode_.insertChild(&childNode->yogaNode_, yogaNode_.getChildren().size());

ensureYogaChildrenLookFine();
}
Expand All @@ -219,7 +217,7 @@ void YogaLayoutableShadowNode::adoptYogaChild(size_t index) {
auto clonedChildNode = childNode.clone({});

// Replace the child node with a newly cloned one in the children list.
replaceChild(childNode, clonedChildNode, static_cast<int>(index));
replaceChild(childNode, clonedChildNode, static_cast<int32_t>(index));
}

ensureYogaChildrenLookFine();
Expand Down Expand Up @@ -513,7 +511,7 @@ YGErrata YogaLayoutableShadowNode::resolveErrata(YGErrata defaultErrata) const {
}

YogaLayoutableShadowNode& YogaLayoutableShadowNode::cloneChildInPlace(
int32_t layoutableChildIndex) {
size_t layoutableChildIndex) {
ensureUnsealed();

const auto& childNode = *yogaLayoutableChildren_[layoutableChildIndex];
Expand All @@ -525,7 +523,8 @@ YogaLayoutableShadowNode& YogaLayoutableShadowNode::cloneChildInPlace(
ShadowNodeFragment::childrenPlaceholder(),
childNode.getState()});

replaceChild(childNode, clonedChildNode, layoutableChildIndex);
replaceChild(
childNode, clonedChildNode, static_cast<int32_t>(layoutableChildIndex));
return static_cast<YogaLayoutableShadowNode&>(*clonedChildNode);
}

Expand Down Expand Up @@ -790,7 +789,7 @@ Rect YogaLayoutableShadowNode::getContentBounds() const {
YGNodeRef YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector(
YGNodeConstRef /*oldYogaNode*/,
YGNodeConstRef parentYogaNode,
int childIndex) {
size_t childIndex) {
SystraceSection s("YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector");

auto& parentNode = shadowNodeFromContext(parentYogaNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,15 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode {
/**
* Replcaes a child with a mutable clone of itself, returning the clone.
*/
YogaLayoutableShadowNode& cloneChildInPlace(int32_t layoutableChildIndex);
YogaLayoutableShadowNode& cloneChildInPlace(size_t layoutableChildIndex);

static yoga::Config& initializeYogaConfig(
yoga::Config& config,
YGConfigRef previousConfig = nullptr);
static YGNodeRef yogaNodeCloneCallbackConnector(
YGNodeConstRef oldYogaNode,
YGNodeConstRef parentYogaNode,
int childIndex);
size_t childIndex);
static YGSize yogaNodeMeasureCallbackConnector(
YGNodeConstRef yogaNode,
float width,
Expand Down
30 changes: 15 additions & 15 deletions packages/react-native/ReactCommon/yoga/yoga/Yoga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ YOGA_EXPORT void YGNodeFree(const YGNodeRef nodeRef) {
node->setOwner(nullptr);
}

const uint32_t childCount = YGNodeGetChildCount(node);
for (uint32_t i = 0; i < childCount; i++) {
const size_t childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
auto child = node->getChild(i);
child->setOwner(nullptr);
}
Expand All @@ -236,8 +236,8 @@ YOGA_EXPORT void YGNodeFreeRecursiveWithCleanupFunc(
YGNodeCleanupFunc cleanup) {
const auto root = resolveRef(rootRef);

uint32_t skipped = 0;
while (YGNodeGetChildCount(root) > skipped) {
size_t skipped = 0;
while (root->getChildCount() > skipped) {
const auto child = root->getChild(skipped);
if (child->getOwner() != root) {
// Don't free shared nodes that we don't own.
Expand Down Expand Up @@ -297,7 +297,7 @@ YOGA_EXPORT bool YGNodeIsReferenceBaseline(YGNodeConstRef node) {
YOGA_EXPORT void YGNodeInsertChild(
const YGNodeRef ownerRef,
const YGNodeRef childRef,
const uint32_t index) {
const size_t index) {
auto owner = resolveRef(ownerRef);
auto child = resolveRef(childRef);

Expand All @@ -319,7 +319,7 @@ YOGA_EXPORT void YGNodeInsertChild(
YOGA_EXPORT void YGNodeSwapChild(
const YGNodeRef ownerRef,
const YGNodeRef childRef,
const uint32_t index) {
const size_t index) {
auto owner = resolveRef(ownerRef);
auto child = resolveRef(childRef);

Expand All @@ -333,7 +333,7 @@ YOGA_EXPORT void YGNodeRemoveChild(
auto owner = resolveRef(ownerRef);
auto excludedChild = resolveRef(excludedChildRef);

if (YGNodeGetChildCount(owner) == 0) {
if (owner->getChildCount() == 0) {
// This is an empty set. Nothing to remove.
return;
}
Expand All @@ -354,7 +354,7 @@ YOGA_EXPORT void YGNodeRemoveChild(
YOGA_EXPORT void YGNodeRemoveAllChildren(const YGNodeRef ownerRef) {
auto owner = resolveRef(ownerRef);

const uint32_t childCount = YGNodeGetChildCount(owner);
const size_t childCount = owner->getChildCount();
if (childCount == 0) {
// This is an empty set already. Nothing to do.
return;
Expand All @@ -363,7 +363,7 @@ YOGA_EXPORT void YGNodeRemoveAllChildren(const YGNodeRef ownerRef) {
if (firstChild->getOwner() == owner) {
// If the first child has this node as its owner, we assume that this child
// set is unique.
for (uint32_t i = 0; i < childCount; i++) {
for (size_t i = 0; i < childCount; i++) {
yoga::Node* oldChild = owner->getChild(i);
oldChild->setLayout({}); // layout is no longer valid
oldChild->setOwner(nullptr);
Expand All @@ -381,7 +381,7 @@ YOGA_EXPORT void YGNodeRemoveAllChildren(const YGNodeRef ownerRef) {
YOGA_EXPORT void YGNodeSetChildren(
const YGNodeRef ownerRef,
const YGNodeRef* childrenRefs,
const uint32_t count) {
const size_t count) {
auto owner = resolveRef(ownerRef);
auto children = reinterpret_cast<yoga::Node* const*>(childrenRefs);

Expand All @@ -391,7 +391,7 @@ YOGA_EXPORT void YGNodeSetChildren(

const std::vector<yoga::Node*> childrenVector = {children, children + count};
if (childrenVector.size() == 0) {
if (YGNodeGetChildCount(owner) > 0) {
if (owner->getChildCount() > 0) {
for (auto* child : owner->getChildren()) {
child->setLayout({});
child->setOwner(nullptr);
Expand All @@ -400,7 +400,7 @@ YOGA_EXPORT void YGNodeSetChildren(
owner->markDirtyAndPropagate();
}
} else {
if (YGNodeGetChildCount(owner) > 0) {
if (owner->getChildCount() > 0) {
for (auto* oldChild : owner->getChildren()) {
// Our new children may have nodes in common with the old children. We
// don't reset these common nodes.
Expand All @@ -420,7 +420,7 @@ YOGA_EXPORT void YGNodeSetChildren(
}

YOGA_EXPORT YGNodeRef
YGNodeGetChild(const YGNodeRef nodeRef, const uint32_t index) {
YGNodeGetChild(const YGNodeRef nodeRef, const size_t index) {
const auto node = resolveRef(nodeRef);

if (index < node->getChildren().size()) {
Expand All @@ -429,8 +429,8 @@ YGNodeGetChild(const YGNodeRef nodeRef, const uint32_t index) {
return nullptr;
}

YOGA_EXPORT uint32_t YGNodeGetChildCount(const YGNodeConstRef node) {
return static_cast<uint32_t>(resolveRef(node)->getChildren().size());
YOGA_EXPORT size_t YGNodeGetChildCount(const YGNodeConstRef node) {
return resolveRef(node)->getChildren().size();
}

YOGA_EXPORT YGNodeRef YGNodeGetOwner(const YGNodeRef node) {
Expand Down
16 changes: 7 additions & 9 deletions packages/react-native/ReactCommon/yoga/yoga/Yoga.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

#include <yoga/YGEnums.h>
Expand Down Expand Up @@ -47,7 +48,7 @@ typedef int (*YGLogger)(
typedef YGNodeRef (*YGCloneNodeFunc)(
YGNodeConstRef oldNode,
YGNodeConstRef owner,
int childIndex);
size_t childIndex);

// YGNode
WIN_EXPORT YGNodeRef YGNodeNew(void);
Expand All @@ -63,23 +64,20 @@ WIN_EXPORT void YGNodeReset(YGNodeRef node);
WIN_EXPORT void YGNodeInsertChild(
YGNodeRef node,
YGNodeRef child,
uint32_t index);
size_t index);

WIN_EXPORT void YGNodeSwapChild(
YGNodeRef node,
YGNodeRef child,
uint32_t index);
WIN_EXPORT void YGNodeSwapChild(YGNodeRef node, YGNodeRef child, size_t index);

WIN_EXPORT void YGNodeRemoveChild(YGNodeRef node, YGNodeRef child);
WIN_EXPORT void YGNodeRemoveAllChildren(YGNodeRef node);
WIN_EXPORT YGNodeRef YGNodeGetChild(YGNodeRef node, uint32_t index);
WIN_EXPORT YGNodeRef YGNodeGetChild(YGNodeRef node, size_t index);
WIN_EXPORT YGNodeRef YGNodeGetOwner(YGNodeRef node);
WIN_EXPORT YGNodeRef YGNodeGetParent(YGNodeRef node);
WIN_EXPORT uint32_t YGNodeGetChildCount(YGNodeConstRef node);
WIN_EXPORT size_t YGNodeGetChildCount(YGNodeConstRef node);
WIN_EXPORT void YGNodeSetChildren(
YGNodeRef owner,
const YGNodeRef* children,
uint32_t count);
size_t count);

WIN_EXPORT void YGNodeSetIsReferenceBaseline(
YGNodeRef node,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ float calculateBaseline(const yoga::Node* node, void* layoutContext) {
}

yoga::Node* baselineChild = nullptr;
const uint32_t childCount = YGNodeGetChildCount(node);
for (uint32_t i = 0; i < childCount; i++) {
const size_t childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
auto child = node->getChild(i);
if (child->getLineIndex() > 0) {
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,8 @@ static CollectFlexItemsRowValues calculateCollectFlexItemsRowValues(
const float mainAxisownerSize,
const float availableInnerWidth,
const float availableInnerMainDim,
const uint32_t startOfLineIndex,
const uint32_t lineCount) {
const size_t startOfLineIndex,
const size_t lineCount) {
CollectFlexItemsRowValues flexAlgoRowMeasurement = {};
flexAlgoRowMeasurement.relativeChildren.reserve(node->getChildren().size());

Expand All @@ -977,7 +977,7 @@ static CollectFlexItemsRowValues calculateCollectFlexItemsRowValues(
const float gap = node->getGapForAxis(mainAxis, availableInnerWidth).unwrap();

// Add items to the current line until it's full or we run out of items.
uint32_t endOfLineIndex = startOfLineIndex;
size_t endOfLineIndex = startOfLineIndex;
for (; endOfLineIndex < node->getChildren().size(); endOfLineIndex++) {
auto child = node->getChild(endOfLineIndex);
if (child->getStyle().display() == YGDisplayNone ||
Expand Down Expand Up @@ -1408,7 +1408,7 @@ static void resolveFlexibleLength(
static void YGJustifyMainAxis(
yoga::Node* const node,
CollectFlexItemsRowValues& collectedFlexItemsValues,
const uint32_t startOfLineIndex,
const size_t startOfLineIndex,
const YGFlexDirection mainAxis,
const YGFlexDirection crossAxis,
const YGMeasureMode measureModeMainDim,
Expand Down Expand Up @@ -1456,8 +1456,7 @@ static void YGJustifyMainAxis(
}

int numberOfAutoMarginsOnCurrentLine = 0;
for (uint32_t i = startOfLineIndex;
i < collectedFlexItemsValues.endOfLineIndex;
for (size_t i = startOfLineIndex; i < collectedFlexItemsValues.endOfLineIndex;
i++) {
auto child = node->getChild(i);
if (child->getStyle().positionType() != YGPositionTypeAbsolute) {
Expand Down Expand Up @@ -1517,8 +1516,7 @@ static void YGJustifyMainAxis(
float maxAscentForCurrentLine = 0;
float maxDescentForCurrentLine = 0;
bool isNodeBaselineLayout = isBaselineLayout(node);
for (uint32_t i = startOfLineIndex;
i < collectedFlexItemsValues.endOfLineIndex;
for (size_t i = startOfLineIndex; i < collectedFlexItemsValues.endOfLineIndex;
i++) {
const auto child = node->getChild(i);
const Style& childStyle = child->getStyle();
Expand Down Expand Up @@ -1906,11 +1904,11 @@ static void calculateLayoutImpl(
// STEP 4: COLLECT FLEX ITEMS INTO FLEX LINES

// Indexes of children that represent the first and last items in the line.
uint32_t startOfLineIndex = 0;
uint32_t endOfLineIndex = 0;
size_t startOfLineIndex = 0;
size_t endOfLineIndex = 0;

// Number of lines.
uint32_t lineCount = 0;
size_t lineCount = 0;

// Accumulated cross dimensions of all lines so far.
float totalLineCrossDim = 0;
Expand Down Expand Up @@ -2093,7 +2091,7 @@ static void calculateLayoutImpl(
// STEP 7: CROSS-AXIS ALIGNMENT
// We can skip child alignment if we're just measuring the container.
if (performLayout) {
for (uint32_t i = startOfLineIndex; i < endOfLineIndex; i++) {
for (size_t i = startOfLineIndex; i < endOfLineIndex; i++) {
const auto child = node->getChild(i);
if (child->getStyle().display() == YGDisplayNone) {
continue;
Expand Down Expand Up @@ -2291,10 +2289,10 @@ static void calculateLayoutImpl(
break;
}
}
uint32_t endIndex = 0;
for (uint32_t i = 0; i < lineCount; i++) {
const uint32_t startIndex = endIndex;
uint32_t ii;
size_t endIndex = 0;
for (size_t i = 0; i < lineCount; i++) {
const size_t startIndex = endIndex;
size_t ii;

// compute the line's height and find the endIndex
float lineHeight = 0;
Expand Down Expand Up @@ -2537,7 +2535,7 @@ static void calculateLayoutImpl(
// As we only wrapped in normal direction yet, we need to reverse the
// positions on wrap-reverse.
if (performLayout && node->getStyle().flexWrap() == YGWrapWrapReverse) {
for (uint32_t i = 0; i < childCount; i++) {
for (size_t i = 0; i < childCount; i++) {
const auto child = node->getChild(i);
if (child->getStyle().positionType() != YGPositionTypeAbsolute) {
child->setLayoutPosition(
Expand Down Expand Up @@ -2586,7 +2584,7 @@ static void calculateLayoutImpl(

// Set trailing position if necessary.
if (needsMainTrailingPos || needsCrossTrailingPos) {
for (uint32_t i = 0; i < childCount; i++) {
for (size_t i = 0; i < childCount; i++) {
const auto child = node->getChild(i);
if (child->getStyle().display() == YGDisplayNone) {
continue;
Expand Down Expand Up @@ -2710,7 +2708,7 @@ bool calculateLayoutInternal(
cachedResults = &layout->cachedLayout;
} else {
// Try to use the measurement cache.
for (uint32_t i = 0; i < layout->nextCachedMeasurementsIndex; i++) {
for (size_t i = 0; i < layout->nextCachedMeasurementsIndex; i++) {
if (canUseCachedMeasurement(
widthMeasureMode,
availableWidth,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ namespace facebook::yoga {
// and/or grow.

struct CollectFlexItemsRowValues {
uint32_t itemsOnLine;
size_t itemsOnLine;
float sizeConsumedOnCurrentLine;
float totalFlexGrowFactors;
float totalFlexShrinkScaledFactors;
uint32_t endOfLineIndex;
size_t endOfLineIndex;
std::vector<yoga::Node*> relativeChildren;
float remainingFreeSpace;
// The size of the mainDim for the row after considering size, padding, margin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ void roundLayoutResultsToPixelGrid(
absoluteNodeTop, pointScaleFactor, false, textRounding),
YGDimensionHeight);

const uint32_t childCount = YGNodeGetChildCount(node);
for (uint32_t i = 0; i < childCount; i++) {
const size_t childCount = node->getChildCount();
for (size_t i = 0; i < childCount; i++) {
roundLayoutResultsToPixelGrid(
node->getChild(i), pointScaleFactor, absoluteNodeLeft, absoluteNodeTop);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void Config::setCloneNodeCallback(std::nullptr_t) {
YGNodeRef Config::cloneNode(
YGNodeConstRef node,
YGNodeConstRef owner,
int childIndex,
size_t childIndex,
void* cloneContext) const {
YGNodeRef clone = nullptr;
if (cloneNodeCallback_.noContext != nullptr) {
Expand Down
Loading

0 comments on commit 3cc9ce9

Please sign in to comment.