From dd1706748ed4da84424d21c861f9b27fb714bc85 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Sun, 10 Sep 2023 03:20:50 -0700 Subject: [PATCH] Breaking: size_t indices Summary: 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). Differential Revision: D49130914 fbshipit-source-id: b2c3cb6384a54904c670ccba3dc768260ca595dd --- java/jni/YGJNIVanilla.cpp | 2 +- tests/{generated => }/YGConfigTest.cpp | 8 ++--- tests/YGTreeMutationTest.cpp | 4 +-- yoga/Yoga.cpp | 30 +++++++++--------- yoga/Yoga.h | 16 +++++----- yoga/algorithm/Baseline.cpp | 4 +-- yoga/algorithm/CalculateLayout.cpp | 36 ++++++++++------------ yoga/algorithm/CollectFlexItemsRowValues.h | 4 +-- yoga/algorithm/PixelGrid.cpp | 4 +-- yoga/config/Config.cpp | 2 +- yoga/config/Config.h | 4 +-- yoga/debug/NodeToString.cpp | 4 +-- yoga/node/Node.cpp | 2 +- yoga/node/Node.h | 6 ++-- 14 files changed, 60 insertions(+), 66 deletions(-) rename tests/{generated => }/YGConfigTest.cpp (85%) diff --git a/java/jni/YGJNIVanilla.cpp b/java/jni/YGJNIVanilla.cpp index af06c4a396..a95a00340d 100644 --- a/java/jni/YGJNIVanilla.cpp +++ b/java/jni/YGJNIVanilla.cpp @@ -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); } diff --git a/tests/generated/YGConfigTest.cpp b/tests/YGConfigTest.cpp similarity index 85% rename from tests/generated/YGConfigTest.cpp rename to tests/YGConfigTest.cpp index 4cfd90eff2..0a3e128a25 100644 --- a/tests/generated/YGConfigTest.cpp +++ b/tests/YGConfigTest.cpp @@ -5,8 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -// @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html - #include #include #include @@ -23,10 +21,10 @@ struct ConfigCloningTest : public ::testing::Test { void TearDown() override; static yoga::Node clonedNode; - static YGNodeRef cloneNode(YGNodeConstRef, YGNodeConstRef, int) { + static YGNodeRef cloneNode(YGNodeConstRef, YGNodeConstRef, size_t) { return &clonedNode; } - static YGNodeRef doNotClone(YGNodeConstRef, YGNodeConstRef, int) { + static YGNodeRef doNotClone(YGNodeConstRef, YGNodeConstRef, size_t) { return nullptr; } }; @@ -54,7 +52,7 @@ TEST_F( TEST_F(ConfigCloningTest, can_clone_with_context) { config->setCloneNodeCallback( - [](YGNodeConstRef, YGNodeConstRef, int, void* context) { + [](YGNodeConstRef, YGNodeConstRef, size_t, void* context) { return (YGNodeRef) context; }); diff --git a/tests/YGTreeMutationTest.cpp b/tests/YGTreeMutationTest.cpp index 5c0cc51859..301d0989f7 100644 --- a/tests/YGTreeMutationTest.cpp +++ b/tests/YGTreeMutationTest.cpp @@ -9,10 +9,10 @@ #include static std::vector getChildren(YGNodeRef const node) { - const uint32_t count = YGNodeGetChildCount(node); + const auto count = YGNodeGetChildCount(node); std::vector children; children.reserve(count); - for (uint32_t i = 0; i < count; i++) { + for (size_t i = 0; i < count; i++) { children.push_back(YGNodeGetChild(node, i)); } return children; diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index 0e89c06d8a..9a91e032bc 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -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); } @@ -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. @@ -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); @@ -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); @@ -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; } @@ -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; @@ -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); @@ -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(childrenRefs); @@ -391,7 +391,7 @@ YOGA_EXPORT void YGNodeSetChildren( const std::vector 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); @@ -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. @@ -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()) { @@ -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(resolveRef(node)->getChildren().size()); +YOGA_EXPORT size_t YGNodeGetChildCount(const YGNodeConstRef node) { + return resolveRef(node)->getChildren().size(); } YOGA_EXPORT YGNodeRef YGNodeGetOwner(const YGNodeRef node) { diff --git a/yoga/Yoga.h b/yoga/Yoga.h index fa5b9834ec..41464e1ee5 100644 --- a/yoga/Yoga.h +++ b/yoga/Yoga.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -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); @@ -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, diff --git a/yoga/algorithm/Baseline.cpp b/yoga/algorithm/Baseline.cpp index 6fb256c7bd..5042074a1e 100644 --- a/yoga/algorithm/Baseline.cpp +++ b/yoga/algorithm/Baseline.cpp @@ -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; diff --git a/yoga/algorithm/CalculateLayout.cpp b/yoga/algorithm/CalculateLayout.cpp index d94de53021..7d6c931cc3 100644 --- a/yoga/algorithm/CalculateLayout.cpp +++ b/yoga/algorithm/CalculateLayout.cpp @@ -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()); @@ -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 || @@ -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, @@ -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) { @@ -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(); @@ -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; @@ -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; @@ -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; @@ -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( @@ -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; @@ -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, diff --git a/yoga/algorithm/CollectFlexItemsRowValues.h b/yoga/algorithm/CollectFlexItemsRowValues.h index 58a3916ac3..f3ba7e2947 100644 --- a/yoga/algorithm/CollectFlexItemsRowValues.h +++ b/yoga/algorithm/CollectFlexItemsRowValues.h @@ -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 relativeChildren; float remainingFreeSpace; // The size of the mainDim for the row after considering size, padding, margin diff --git a/yoga/algorithm/PixelGrid.cpp b/yoga/algorithm/PixelGrid.cpp index 105e57a315..0a4bda2213 100644 --- a/yoga/algorithm/PixelGrid.cpp +++ b/yoga/algorithm/PixelGrid.cpp @@ -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); } diff --git a/yoga/config/Config.cpp b/yoga/config/Config.cpp index deb7b5fe4d..f4dea8433c 100644 --- a/yoga/config/Config.cpp +++ b/yoga/config/Config.cpp @@ -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) { diff --git a/yoga/config/Config.h b/yoga/config/Config.h index 75bde993b6..d06b1bacf7 100644 --- a/yoga/config/Config.h +++ b/yoga/config/Config.h @@ -34,7 +34,7 @@ using LogWithContextFn = int (*)( using CloneWithContextFn = YGNodeRef (*)( YGNodeConstRef node, YGNodeConstRef owner, - int childIndex, + size_t childIndex, void* cloneContext); #pragma pack(push) @@ -92,7 +92,7 @@ class YOGA_EXPORT Config : public ::YGConfig { YGNodeRef cloneNode( YGNodeConstRef node, YGNodeConstRef owner, - int childIndex, + size_t childIndex, void* cloneContext) const; private: diff --git a/yoga/debug/NodeToString.cpp b/yoga/debug/NodeToString.cpp index 30697dd6ed..787ec976e4 100644 --- a/yoga/debug/NodeToString.cpp +++ b/yoga/debug/NodeToString.cpp @@ -227,9 +227,9 @@ void nodeToString( } appendFormattedString(str, ">"); - const uint32_t childCount = static_cast(node->getChildren().size()); + const size_t childCount = node->getChildCount(); if (options & YGPrintOptionsChildren && childCount > 0) { - for (uint32_t i = 0; i < childCount; i++) { + for (size_t i = 0; i < childCount; i++) { appendFormattedString(str, "\n"); nodeToString(str, node->getChild(i), options, level + 1); } diff --git a/yoga/node/Node.cpp b/yoga/node/Node.cpp index 295f4eb9c6..005542dae1 100644 --- a/yoga/node/Node.cpp +++ b/yoga/node/Node.cpp @@ -484,7 +484,7 @@ YOGA_EXPORT void Node::clearChildren() { // Other Methods void Node::cloneChildrenIfNeeded(void* cloneContext) { - int i = 0; + size_t i = 0; for (Node*& child : children_) { if (child->getOwner() != this) { child = resolveRef(config_->cloneNode(child, this, i, cloneContext)); diff --git a/yoga/node/Node.h b/yoga/node/Node.h index 4e8ef24ede..e99a376425 100644 --- a/yoga/node/Node.h +++ b/yoga/node/Node.h @@ -68,7 +68,7 @@ class YOGA_EXPORT Node : public ::YGNode { YGDirtiedFunc dirtied_ = nullptr; Style style_ = {}; LayoutResults layout_ = {}; - uint32_t lineIndex_ = 0; + size_t lineIndex_ = 0; Node* owner_ = nullptr; std::vector children_ = {}; Config* config_; @@ -146,7 +146,7 @@ class YOGA_EXPORT Node : public ::YGNode { const LayoutResults& getLayout() const { return layout_; } - uint32_t getLineIndex() const { return lineIndex_; } + size_t getLineIndex() const { return lineIndex_; } bool isReferenceBaseline() const { return flags_.isReferenceBaseline; } @@ -276,7 +276,7 @@ class YOGA_EXPORT Node : public ::YGNode { void setLayout(const LayoutResults& layout) { layout_ = layout; } - void setLineIndex(uint32_t lineIndex) { lineIndex_ = lineIndex; } + void setLineIndex(size_t lineIndex) { lineIndex_ = lineIndex; } void setIsReferenceBaseline(bool isReferenceBaseline) { flags_.isReferenceBaseline = isReferenceBaseline;