From aee43a53bcfa78fe3d90b9df30feb0e4a36bb754 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Wed, 6 Sep 2023 08:16:42 -0700 Subject: [PATCH] Enable `-Wconversion` (#1359) Summary: X-link: https://github.com/facebook/react-native/pull/39291 Pull Request resolved: https://github.com/facebook/yoga/pull/1359 This enables clang warnings around potentially unsafe conversions, such as those with mismatched signedness, or ones which may lead to truncation. This should catch issues in local development which create errors for MSVC (e.g. Dash), who's default `/W3` includes warnings akin to `-Wshorten-64-to-32`. This full set of warnings here is a tad spammy, but probably more useful than not. Changelog: [Internal] Reviewed By: yungsters Differential Revision: D48954777 fbshipit-source-id: 1ccc07b99d09d1c2d428158149698ffd04025605 --- Yoga.podspec | 1 + benchmark/YGBenchmark.c | 3 +- cmake/project-defaults.cmake | 1 + java/jni/YGJNI.h | 4 +-- java/jni/YGJNIVanilla.cpp | 19 +++++++----- java/jni/YGJTypesVanilla.h | 10 +++---- java/jni/common.cpp | 3 +- tests/NumericBitfieldTest.cpp | 37 ++++++++++++------------ tests/YGNodeCallbackTest.cpp | 4 +-- yoga/algorithm/CalculateLayout.cpp | 38 +++++++++++++----------- yoga/bits/NumericBitfield.h | 29 +++++++------------ yoga/debug/NodeToString.cpp | 4 +-- yoga/event/event.h | 2 +- yoga/node/LayoutResults.h | 23 ++++++++------- yoga/node/Node.cpp | 46 +++++++++++++++++++----------- yoga/node/Node.h | 28 +++++++++--------- yoga/style/Style.h | 22 +++++++------- 17 files changed, 149 insertions(+), 125 deletions(-) diff --git a/Yoga.podspec b/Yoga.podspec index 4ebb5adf..847b3773 100644 --- a/Yoga.podspec +++ b/Yoga.podspec @@ -32,6 +32,7 @@ Pod::Spec.new do |spec| '-Wall', '-Werror', '-Wextra', + '-Wconversion', '-std=c++17', '-fPIC' ] diff --git a/benchmark/YGBenchmark.c b/benchmark/YGBenchmark.c index ff1f6646..443ed7e4 100644 --- a/benchmark/YGBenchmark.c +++ b/benchmark/YGBenchmark.c @@ -55,7 +55,8 @@ static void __printBenchmarkResult( double mean = 0; clock_t lastEnd = start; for (uint32_t i = 0; i < NUM_REPETITIONS; i++) { - timesInMs[i] = (endTimes[i] - lastEnd) / (double) CLOCKS_PER_SEC * 1000; + timesInMs[i] = + ((double) (endTimes[i] - lastEnd)) / (double) CLOCKS_PER_SEC * 1000; lastEnd = endTimes[i]; mean += timesInMs[i]; } diff --git a/cmake/project-defaults.cmake b/cmake/project-defaults.cmake index d02a2836..8868ce93 100644 --- a/cmake/project-defaults.cmake +++ b/cmake/project-defaults.cmake @@ -35,6 +35,7 @@ add_compile_options( -Wall -Wextra -Werror + -Wconversion # Disable RTTI $<$:-fno-rtti> # Use -O2 (prioritize speed) diff --git a/java/jni/YGJNI.h b/java/jni/YGJNI.h index 13950445..6c6d7d5b 100644 --- a/java/jni/YGJNI.h +++ b/java/jni/YGJNI.h @@ -22,12 +22,12 @@ namespace { const int HAS_NEW_LAYOUT = 16; union YGNodeContext { - uintptr_t edgesSet = 0; + int32_t edgesSet = 0; void* asVoidPtr; }; class YGNodeEdges { - uintptr_t edges_; + int32_t edges_; public: enum Edge { diff --git a/java/jni/YGJNIVanilla.cpp b/java/jni/YGJNIVanilla.cpp index 5a712b0f..46d2248a 100644 --- a/java/jni/YGJNIVanilla.cpp +++ b/java/jni/YGJNIVanilla.cpp @@ -146,7 +146,7 @@ static int YGJNILogFunc( va_list argsCopy; va_copy(argsCopy, args); int result = vsnprintf(nullptr, 0, format, argsCopy); - std::vector buffer(1 + result); + std::vector buffer(1 + static_cast(result)); vsnprintf(buffer.data(), buffer.size(), format, args); auto jloggerPtr = @@ -236,7 +236,9 @@ static void jni_YGNodeInsertChildJNI( jlong childPointer, jint index) { YGNodeInsertChild( - _jlong2YGNodeRef(nativePointer), _jlong2YGNodeRef(childPointer), index); + _jlong2YGNodeRef(nativePointer), + _jlong2YGNodeRef(childPointer), + static_cast(index)); } static void jni_YGNodeSwapChildJNI( @@ -246,7 +248,9 @@ static void jni_YGNodeSwapChildJNI( jlong childPointer, jint index) { YGNodeSwapChild( - _jlong2YGNodeRef(nativePointer), _jlong2YGNodeRef(childPointer), index); + _jlong2YGNodeRef(nativePointer), + _jlong2YGNodeRef(childPointer), + static_cast(index)); } static void jni_YGNodeSetIsReferenceBaselineJNI( @@ -307,13 +311,13 @@ static void YGTransferLayoutOutputsRecursive( const int arrSize = 6 + (marginFieldSet ? 4 : 0) + (paddingFieldSet ? 4 : 0) + (borderFieldSet ? 4 : 0); float arr[18]; - arr[LAYOUT_EDGE_SET_FLAG_INDEX] = fieldFlags; + arr[LAYOUT_EDGE_SET_FLAG_INDEX] = static_cast(fieldFlags); arr[LAYOUT_WIDTH_INDEX] = YGNodeLayoutGetWidth(root); arr[LAYOUT_HEIGHT_INDEX] = YGNodeLayoutGetHeight(root); arr[LAYOUT_LEFT_INDEX] = YGNodeLayoutGetLeft(root); arr[LAYOUT_TOP_INDEX] = YGNodeLayoutGetTop(root); arr[LAYOUT_DIRECTION_INDEX] = - static_cast(YGNodeLayoutGetDirection(root)); + static_cast(YGNodeLayoutGetDirection(root)); if (marginFieldSet) { arr[LAYOUT_MARGIN_START_INDEX] = YGNodeLayoutGetMargin(root, YGEdgeLeft); arr[LAYOUT_MARGIN_START_INDEX + 1] = YGNodeLayoutGetMargin(root, YGEdgeTop); @@ -670,9 +674,10 @@ static YGSize YGJNIMeasureFunc( sizeof(measureResult) == 8, "Expected measureResult to be 8 bytes, or two 32 bit ints"); - int32_t wBits = 0xFFFFFFFF & (measureResult >> 32); - int32_t hBits = 0xFFFFFFFF & measureResult; + uint32_t wBits = 0xFFFFFFFF & (measureResult >> 32); + uint32_t hBits = 0xFFFFFFFF & measureResult; + // TODO: this is unsafe under strict aliasing and should use bit_cast const float* measuredWidth = reinterpret_cast(&wBits); const float* measuredHeight = reinterpret_cast(&hBits); diff --git a/java/jni/YGJTypesVanilla.h b/java/jni/YGJTypesVanilla.h index c1534fea..e5c44c0c 100644 --- a/java/jni/YGJTypesVanilla.h +++ b/java/jni/YGJTypesVanilla.h @@ -14,7 +14,7 @@ #include "jni.h" class PtrJNodeMapVanilla { - std::map ptrsToIdxs_{}; + std::map ptrsToIdxs_{}; jobjectArray javaNodes_{}; public: @@ -25,13 +25,13 @@ public: using namespace facebook::yoga::vanillajni; JNIEnv* env = getCurrentEnv(); - size_t nativePointersSize = env->GetArrayLength(javaNativePointers); - std::vector nativePointers(nativePointersSize); + jsize nativePointersSize = env->GetArrayLength(javaNativePointers); + std::vector nativePointers(static_cast(nativePointersSize)); env->GetLongArrayRegion( javaNativePointers, 0, nativePointersSize, nativePointers.data()); - for (size_t i = 0; i < nativePointersSize; ++i) { - ptrsToIdxs_[(YGNodeConstRef) nativePointers[i]] = i; + for (jsize i = 0; i < nativePointersSize; ++i) { + ptrsToIdxs_[(YGNodeConstRef) nativePointers[static_cast(i)]] = i; } } diff --git a/java/jni/common.cpp b/java/jni/common.cpp index f7a43175..317d4046 100644 --- a/java/jni/common.cpp +++ b/java/jni/common.cpp @@ -18,7 +18,8 @@ void registerNatives( assertNoPendingJniExceptionIf(env, !clazz); - auto result = env->RegisterNatives(clazz, methods, numMethods); + auto result = + env->RegisterNatives(clazz, methods, static_cast(numMethods)); assertNoPendingJniExceptionIf(env, result != JNI_OK); } diff --git a/tests/NumericBitfieldTest.cpp b/tests/NumericBitfieldTest.cpp index 9b487cee..7b4e2abe 100644 --- a/tests/NumericBitfieldTest.cpp +++ b/tests/NumericBitfieldTest.cpp @@ -13,7 +13,7 @@ namespace facebook::yoga { TEST(NumericBitfield, one_boolean_defaults_to_false) { - constexpr uint8_t flags = 0; + constexpr uint32_t flags = 0; ASSERT_EQ(getBooleanData(flags, 0), false); static_assert( @@ -22,7 +22,7 @@ TEST(NumericBitfield, one_boolean_defaults_to_false) { } TEST(NumericBitfield, one_boolean_can_be_initialized_to_true) { - constexpr uint8_t flags = 1; + constexpr uint32_t flags = 1; ASSERT_EQ(getBooleanData(flags, 0), true); static_assert( @@ -31,14 +31,14 @@ TEST(NumericBitfield, one_boolean_can_be_initialized_to_true) { } TEST(NumericBitfield, one_boolean_can_be_set_to_true) { - uint8_t flags = 0; + uint32_t flags = 0; setBooleanData(flags, 0, true); ASSERT_EQ(getBooleanData(flags, 0), true); } TEST(NumericBitfield, second_boolean_defaults_to_false) { - constexpr uint8_t flags = 0; + constexpr uint32_t flags = 0; ASSERT_EQ(getBooleanData(flags, 1), false); static_assert( @@ -47,7 +47,7 @@ TEST(NumericBitfield, second_boolean_defaults_to_false) { } TEST(NumericBitfield, second_boolean_can_be_initialized_to_true) { - constexpr uint8_t flags = 2; + constexpr uint32_t flags = 2; ASSERT_EQ(getBooleanData(flags, 0), false); ASSERT_EQ(getBooleanData(flags, 1), true); @@ -60,7 +60,7 @@ TEST(NumericBitfield, second_boolean_can_be_initialized_to_true) { } TEST(NumericBitfield, second_boolean_can_be_set_to_true) { - uint8_t flags = 0; + uint32_t flags = 0; setBooleanData(flags, 1, true); ASSERT_EQ(getBooleanData(flags, 0), false); @@ -68,7 +68,7 @@ TEST(NumericBitfield, second_boolean_can_be_set_to_true) { } TEST(NumericBitfield, third_boolean_defaults_to_false) { - constexpr uint8_t flags = 0; + constexpr uint32_t flags = 0; ASSERT_EQ(getBooleanData(flags, 2), false); static_assert( @@ -77,7 +77,7 @@ TEST(NumericBitfield, third_boolean_defaults_to_false) { } TEST(NumericBitfield, third_boolean_can_be_initialized_to_true) { - constexpr uint8_t flags = 4; + constexpr uint32_t flags = 4; ASSERT_EQ(getBooleanData(flags, 0), false); ASSERT_EQ(getBooleanData(flags, 1), false); @@ -94,7 +94,7 @@ TEST(NumericBitfield, third_boolean_can_be_initialized_to_true) { } TEST(NumericBitfield, third_boolean_can_be_set_to_true) { - uint8_t flags = 0; + uint32_t flags = 0; setBooleanData(flags, 2, true); ASSERT_EQ(getBooleanData(flags, 0), false); @@ -103,7 +103,7 @@ TEST(NumericBitfield, third_boolean_can_be_set_to_true) { } TEST(NumericBitfield, setting_boolean_values_does_not_spill_over) { - uint8_t flags = 0; + uint32_t flags = 0; setBooleanData(flags, 1, (bool) 7); @@ -113,7 +113,7 @@ TEST(NumericBitfield, setting_boolean_values_does_not_spill_over) { } TEST(NumericBitfield, first_enum_defaults_to_0) { - constexpr uint8_t flags = 0; + constexpr uint32_t flags = 0; ASSERT_EQ(getEnumData(flags, 0), YGAlignAuto); static_assert( @@ -122,7 +122,7 @@ TEST(NumericBitfield, first_enum_defaults_to_0) { } TEST(NumericBitfield, first_enum_can_be_set) { - uint8_t flags = 0; + uint32_t flags = 0; setEnumData(flags, 0, YGAlignSpaceBetween); @@ -130,7 +130,7 @@ TEST(NumericBitfield, first_enum_can_be_set) { } TEST(NumericBitfield, second_enum_defaults_to_0) { - constexpr uint8_t flags = 0; + constexpr uint32_t flags = 0; static constexpr size_t alignOffset = 0; static constexpr size_t edgeOffset = 3; @@ -145,7 +145,7 @@ TEST(NumericBitfield, second_enum_defaults_to_0) { } TEST(NumericBitfield, second_enum_can_be_set) { - uint8_t flags = 0; + uint32_t flags = 0; static constexpr size_t alignOffset = 0; static constexpr size_t edgeOffset = 3; @@ -156,7 +156,7 @@ TEST(NumericBitfield, second_enum_can_be_set) { } TEST(NumericBitfield, third_enum_defaults_to_0) { - constexpr uint8_t flags = 0; + constexpr uint32_t flags = 0; static constexpr size_t alignOffset = 0; static constexpr size_t boolOffset = 3; static constexpr size_t edgesOffset = 4; @@ -176,7 +176,7 @@ TEST(NumericBitfield, third_enum_defaults_to_0) { } TEST(NumericBitfield, third_enum_can_be_set) { - uint8_t flags = 0; + uint32_t flags = 0; static constexpr size_t alignOffset = 0; static constexpr size_t boolOffset = 3; static constexpr size_t edgesOffset = 4; @@ -189,12 +189,13 @@ TEST(NumericBitfield, third_enum_can_be_set) { } TEST(NumericBitfield, setting_values_does_not_spill_over) { - uint8_t flags = 0; + uint32_t flags = 0; static constexpr size_t alignOffset = 0; static constexpr size_t edgesOffset = 3; static constexpr size_t boolOffset = 7; - setEnumData(flags, edgesOffset, (YGEdge) 0xffffff); + uint32_t edge = 0xffffff; + setEnumData(flags, edgesOffset, (YGEdge) edge); ASSERT_EQ(getEnumData(flags, alignOffset), 0); ASSERT_EQ(getBooleanData(flags, boolOffset), false); diff --git a/tests/YGNodeCallbackTest.cpp b/tests/YGNodeCallbackTest.cpp index 9a594975..ec41bb43 100644 --- a/tests/YGNodeCallbackTest.cpp +++ b/tests/YGNodeCallbackTest.cpp @@ -33,7 +33,7 @@ TEST(Node, measure_with_measure_fn) { n.setMeasureFunc( [](YGNodeRef, float w, YGMeasureMode wm, float h, YGMeasureMode hm) { - return YGSize{w * static_cast(wm), h / static_cast(hm)}; + return YGSize{w * static_cast(wm), h / static_cast(hm)}; }); ASSERT_EQ( @@ -62,7 +62,7 @@ TEST(Node, switching_measure_fn_types) { }); n.setMeasureFunc( [](YGNodeRef, float w, YGMeasureMode wm, float h, YGMeasureMode hm) { - return YGSize{w * static_cast(wm), h / static_cast(hm)}; + return YGSize{w * static_cast(wm), h / static_cast(hm)}; }); ASSERT_EQ( diff --git a/yoga/algorithm/CalculateLayout.cpp b/yoga/algorithm/CalculateLayout.cpp index e6a3dd5a..d2bc46b6 100644 --- a/yoga/algorithm/CalculateLayout.cpp +++ b/yoga/algorithm/CalculateLayout.cpp @@ -823,8 +823,8 @@ static void zeroOutLayoutRecursively( yoga::Node* const node, void* layoutContext) { node->getLayout() = {}; - node->setLayoutDimension(0, 0); - node->setLayoutDimension(0, 1); + node->setLayoutDimension(0, YGDimensionWidth); + node->setLayoutDimension(0, YGDimensionHeight); node->setHasNewLayout(true); node->iterChildrenAfterCloningIfNeeded( @@ -1488,19 +1488,19 @@ static void YGJustifyMainAxis( betweenMainDim += yoga::maxOrDefined( collectedFlexItemsValues.remainingFreeSpace, 0) / - (collectedFlexItemsValues.itemsOnLine - 1); + static_cast(collectedFlexItemsValues.itemsOnLine - 1); } break; case YGJustifySpaceEvenly: // Space is distributed evenly across all elements leadingMainDim = collectedFlexItemsValues.remainingFreeSpace / - (collectedFlexItemsValues.itemsOnLine + 1); + static_cast(collectedFlexItemsValues.itemsOnLine + 1); betweenMainDim += leadingMainDim; break; case YGJustifySpaceAround: // Space on the edges is half of the space between elements leadingMainDim = 0.5f * collectedFlexItemsValues.remainingFreeSpace / - collectedFlexItemsValues.itemsOnLine; + static_cast(collectedFlexItemsValues.itemsOnLine); betweenMainDim += leadingMainDim * 2; break; case YGJustifyFlexStart: @@ -1550,7 +1550,7 @@ static void YGJustifyMainAxis( if (child->marginLeadingValue(mainAxis).unit == YGUnitAuto) { collectedFlexItemsValues.mainDim += collectedFlexItemsValues.remainingFreeSpace / - numberOfAutoMarginsOnCurrentLine; + static_cast(numberOfAutoMarginsOnCurrentLine); } if (performLayout) { @@ -1563,7 +1563,7 @@ static void YGJustifyMainAxis( if (child->marginTrailingValue(mainAxis).unit == YGUnitAuto) { collectedFlexItemsValues.mainDim += collectedFlexItemsValues.remainingFreeSpace / - numberOfAutoMarginsOnCurrentLine; + static_cast(numberOfAutoMarginsOnCurrentLine); } bool canSkipFlex = !performLayout && measureModeCrossDim == YGMeasureModeExactly; @@ -1890,7 +1890,7 @@ static void calculateLayoutImpl( if (childCount > 1) { totalMainDim += node->getGapForAxis(mainAxis, availableInnerCrossDim).unwrap() * - (childCount - 1); + static_cast(childCount - 1); } const bool mainAxisOverflows = @@ -2261,14 +2261,17 @@ static void calculateLayoutImpl( break; case YGAlignStretch: if (availableInnerCrossDim > totalLineCrossDim) { - crossDimLead = remainingAlignContentDim / lineCount; + crossDimLead = + remainingAlignContentDim / static_cast(lineCount); } break; case YGAlignSpaceAround: if (availableInnerCrossDim > totalLineCrossDim) { - currentLead += remainingAlignContentDim / (2 * lineCount); + currentLead += + remainingAlignContentDim / (2 * static_cast(lineCount)); if (lineCount > 1) { - crossDimLead = remainingAlignContentDim / lineCount; + crossDimLead = + remainingAlignContentDim / static_cast(lineCount); } } else { currentLead += remainingAlignContentDim / 2; @@ -2276,7 +2279,8 @@ static void calculateLayoutImpl( break; case YGAlignSpaceBetween: if (availableInnerCrossDim > totalLineCrossDim && lineCount > 1) { - crossDimLead = remainingAlignContentDim / (lineCount - 1); + crossDimLead = + remainingAlignContentDim / static_cast(lineCount - 1); } break; case YGAlignAuto: @@ -2843,11 +2847,11 @@ bool calculateLayoutInternal( layout->lastOwnerDirection = ownerDirection; if (cachedResults == nullptr) { - if (layout->nextCachedMeasurementsIndex + 1 > - (uint32_t) layoutMarkerData.maxMeasureCache) { - layoutMarkerData.maxMeasureCache = - layout->nextCachedMeasurementsIndex + 1; - } + + layoutMarkerData.maxMeasureCache = std::max( + layoutMarkerData.maxMeasureCache, + layout->nextCachedMeasurementsIndex + 1u); + if (layout->nextCachedMeasurementsIndex == LayoutResults::MaxCachedMeasurements) { if (gPrintChanges) { diff --git a/yoga/bits/NumericBitfield.h b/yoga/bits/NumericBitfield.h index af3fc692..358c4ab7 100644 --- a/yoga/bits/NumericBitfield.h +++ b/yoga/bits/NumericBitfield.h @@ -15,12 +15,12 @@ namespace facebook::yoga::details { -constexpr size_t log2ceilFn(size_t n) { +constexpr uint8_t log2ceilFn(uint8_t n) { return n < 1 ? 0 : (1 + log2ceilFn(n / 2)); } -constexpr int mask(size_t bitWidth, size_t index) { - return ((1 << bitWidth) - 1) << index; +constexpr uint32_t mask(uint8_t bitWidth, uint8_t index) { + return ((1u << bitWidth) - 1u) << index; } } // namespace facebook::yoga::details @@ -29,38 +29,31 @@ namespace facebook::yoga { // The number of bits necessary to represent enums defined with YG_ENUM_SEQ_DECL template -constexpr size_t minimumBitCount() { +constexpr uint8_t minimumBitCount() { static_assert( enums::count() > 0, "Enums must have at least one entries"); return details::log2ceilFn(enums::count() - 1); } template -constexpr Enum getEnumData(int flags, size_t index) { +constexpr Enum getEnumData(uint32_t flags, uint8_t index) { return static_cast( (flags & details::mask(minimumBitCount(), index)) >> index); } template -void setEnumData(uint32_t& flags, size_t index, int newValue) { - flags = (flags & ~details::mask(minimumBitCount(), index)) | +void setEnumData(uint32_t& flags, uint8_t index, uint32_t newValue) { + flags = + (flags & + ~static_cast(details::mask(minimumBitCount(), index))) | ((newValue << index) & (details::mask(minimumBitCount(), index))); } -template -void setEnumData(uint8_t& flags, size_t index, int newValue) { - flags = - (flags & - ~static_cast(details::mask(minimumBitCount(), index))) | - ((newValue << index) & - (static_cast(details::mask(minimumBitCount(), index)))); -} - -constexpr bool getBooleanData(int flags, size_t index) { +constexpr bool getBooleanData(uint32_t flags, uint8_t index) { return (flags >> index) & 1; } -inline void setBooleanData(uint8_t& flags, size_t index, bool value) { +inline void setBooleanData(uint32_t& flags, uint8_t index, bool value) { if (value) { flags |= 1 << index; } else { diff --git a/yoga/debug/NodeToString.cpp b/yoga/debug/NodeToString.cpp index 756f4800..449fbc70 100644 --- a/yoga/debug/NodeToString.cpp +++ b/yoga/debug/NodeToString.cpp @@ -33,7 +33,7 @@ static void appendFormattedString(std::string& str, const char* fmt, ...) { va_start(args, fmt); va_list argsCopy; va_copy(argsCopy, args); - std::vector buf(1 + vsnprintf(NULL, 0, fmt, args)); + std::vector buf(1 + static_cast(vsnprintf(NULL, 0, fmt, args))); va_end(args); vsnprintf(buf.data(), buf.size(), fmt, argsCopy); va_end(argsCopy); @@ -96,7 +96,7 @@ static void appendEdges( } else { for (int edge = YGEdgeLeft; edge != YGEdgeAll; ++edge) { std::string str = key + "-" + YGEdgeToString(static_cast(edge)); - appendNumberIfNotZero(base, str, edges[edge]); + appendNumberIfNotZero(base, str, edges[static_cast(edge)]); } } } diff --git a/yoga/event/event.h b/yoga/event/event.h index 3798e466..fca12aca 100644 --- a/yoga/event/event.h +++ b/yoga/event/event.h @@ -38,7 +38,7 @@ enum struct LayoutPassReason : int { struct LayoutData { int layouts; int measures; - int maxMeasureCache; + uint32_t maxMeasureCache; int cachedLayouts; int cachedMeasures; int measureCallbacks; diff --git a/yoga/node/LayoutResults.h b/yoga/node/LayoutResults.h index 95b4a21b..2e1fb38f 100644 --- a/yoga/node/LayoutResults.h +++ b/yoga/node/LayoutResults.h @@ -15,6 +15,14 @@ namespace facebook::yoga { +#pragma pack(push) +#pragma pack(1) +struct LayoutResultFlags { + uint32_t direction : 2; + bool hadOverflow : 1; +}; +#pragma pack(pop) + struct LayoutResults { // This value was chosen based on empirical data: // 98% of analyzed layouts require less than 8 entries. @@ -27,10 +35,7 @@ struct LayoutResults { std::array padding = {}; private: - static constexpr size_t directionOffset = 0; - static constexpr size_t hadOverflowOffset = - directionOffset + minimumBitCount(); - uint8_t flags = 0; + LayoutResultFlags flags_{}; public: uint32_t computedFlexBasisGeneration = 0; @@ -48,17 +53,15 @@ public: CachedMeasurement cachedLayout{}; YGDirection direction() const { - return getEnumData(flags, directionOffset); + return static_cast(flags_.direction); } void setDirection(YGDirection direction) { - setEnumData(flags, directionOffset, direction); + flags_.direction = static_cast(direction) & 0x03; } - bool hadOverflow() const { return getBooleanData(flags, hadOverflowOffset); } - void setHadOverflow(bool hadOverflow) { - setBooleanData(flags, hadOverflowOffset, hadOverflow); - } + bool hadOverflow() const { return flags_.hadOverflow; } + void setHadOverflow(bool hadOverflow) { flags_.hadOverflow = hadOverflow; } bool operator==(LayoutResults layout) const; bool operator!=(LayoutResults layout) const { return !(*this == layout); } diff --git a/yoga/node/Node.cpp b/yoga/node/Node.cpp index eb674a1d..70a81bd1 100644 --- a/yoga/node/Node.cpp +++ b/yoga/node/Node.cpp @@ -6,6 +6,7 @@ */ #include +#include #include #include @@ -264,7 +265,7 @@ YOGA_EXPORT void Node::setMeasureFunc(MeasureWithContextFn measureFunc) { setMeasureFunc(m); } -void Node::replaceChild(Node* child, uint32_t index) { +void Node::replaceChild(Node* child, size_t index) { children_[index] = child; } @@ -272,8 +273,8 @@ void Node::replaceChild(Node* oldChild, Node* newChild) { std::replace(children_.begin(), children_.end(), oldChild, newChild); } -void Node::insertChild(Node* child, uint32_t index) { - children_.insert(children_.begin() + index, child); +void Node::insertChild(Node* child, size_t index) { + children_.insert(children_.begin() + static_cast(index), child); } void Node::setConfig(yoga::Config* config) { @@ -311,24 +312,30 @@ bool Node::removeChild(Node* child) { return false; } -void Node::removeChild(uint32_t index) { - children_.erase(children_.begin() + index); +void Node::removeChild(size_t index) { + children_.erase(children_.begin() + static_cast(index)); } void Node::setLayoutDirection(YGDirection direction) { layout_.setDirection(direction); } -void Node::setLayoutMargin(float margin, int index) { - layout_.margin[index] = margin; +void Node::setLayoutMargin(float margin, YGEdge edge) { + assertFatal( + edge < layout_.margin.size(), "Edge must be top/left/bottom/right"); + layout_.margin[edge] = margin; } -void Node::setLayoutBorder(float border, int index) { - layout_.border[index] = border; +void Node::setLayoutBorder(float border, YGEdge edge) { + assertFatal( + edge < layout_.border.size(), "Edge must be top/left/bottom/right"); + layout_.border[edge] = border; } -void Node::setLayoutPadding(float padding, int index) { - layout_.padding[index] = padding; +void Node::setLayoutPadding(float padding, YGEdge edge) { + assertFatal( + edge < layout_.padding.size(), "Edge must be top/left/bottom/right"); + layout_.padding[edge] = padding; } void Node::setLayoutLastOwnerDirection(YGDirection direction) { @@ -339,8 +346,10 @@ void Node::setLayoutComputedFlexBasis(const FloatOptional computedFlexBasis) { layout_.computedFlexBasis = computedFlexBasis; } -void Node::setLayoutPosition(float position, int index) { - layout_.position[index] = position; +void Node::setLayoutPosition(float position, YGEdge edge) { + assertFatal( + edge < layout_.position.size(), "Edge must be top/left/bottom/right"); + layout_.position[edge] = position; } void Node::setLayoutComputedFlexBasisGeneration( @@ -348,16 +357,19 @@ void Node::setLayoutComputedFlexBasisGeneration( layout_.computedFlexBasisGeneration = computedFlexBasisGeneration; } -void Node::setLayoutMeasuredDimension(float measuredDimension, int index) { - layout_.measuredDimensions[index] = measuredDimension; +void Node::setLayoutMeasuredDimension( + float measuredDimension, + YGDimension dimension) { + layout_.measuredDimensions[static_cast(dimension)] = + measuredDimension; } void Node::setLayoutHadOverflow(bool hadOverflow) { layout_.setHadOverflow(hadOverflow); } -void Node::setLayoutDimension(float dimension, int index) { - layout_.dimensions[index] = dimension; +void Node::setLayoutDimension(float dimensionValue, YGDimension dimension) { + layout_.dimensions[static_cast(dimension)] = dimensionValue; } // If both left and right are defined, then use left. Otherwise return +left or diff --git a/yoga/node/Node.h b/yoga/node/Node.h index 4116e5a6..c7d69267 100644 --- a/yoga/node/Node.h +++ b/yoga/node/Node.h @@ -27,7 +27,7 @@ struct NodeFlags { bool hasNewLayout : 1; bool isReferenceBaseline : 1; bool isDirty : 1; - uint8_t nodeType : 1; + uint32_t nodeType : 1; bool measureUsesContext : 1; bool baselineUsesContext : 1; bool printUsesContext : 1; @@ -181,8 +181,8 @@ public: return resolvedDimensions_; } - YGValue getResolvedDimension(int index) const { - return resolvedDimensions_[index]; + YGValue getResolvedDimension(YGDimension dimension) const { + return resolvedDimensions_[static_cast(dimension)]; } static CompactValue computeEdgeValueForColumn( @@ -257,7 +257,7 @@ public: } void setNodeType(YGNodeType nodeType) { - flags_.nodeType = static_cast(nodeType); + flags_.nodeType = static_cast(nodeType) & 0x01; } void setMeasureFunc(YGMeasureFunc measureFunc); @@ -303,14 +303,16 @@ public: void setLayoutComputedFlexBasis(const FloatOptional computedFlexBasis); void setLayoutComputedFlexBasisGeneration( uint32_t computedFlexBasisGeneration); - void setLayoutMeasuredDimension(float measuredDimension, int index); + void setLayoutMeasuredDimension( + float measuredDimension, + YGDimension dimension); void setLayoutHadOverflow(bool hadOverflow); - void setLayoutDimension(float dimension, int index); + void setLayoutDimension(float dimensionValue, YGDimension dimension); void setLayoutDirection(YGDirection direction); - void setLayoutMargin(float margin, int index); - void setLayoutBorder(float border, int index); - void setLayoutPadding(float padding, int index); - void setLayoutPosition(float position, int index); + void setLayoutMargin(float margin, YGEdge edge); + void setLayoutBorder(float border, YGEdge edge); + void setLayoutPadding(float padding, YGEdge edge); + void setLayoutPosition(float position, YGEdge edge); void setPosition( const YGDirection direction, const float mainSize, @@ -327,11 +329,11 @@ public: void clearChildren(); /// Replaces the occurrences of oldChild with newChild void replaceChild(Node* oldChild, Node* newChild); - void replaceChild(Node* child, uint32_t index); - void insertChild(Node* child, uint32_t index); + void replaceChild(Node* child, size_t index); + void insertChild(Node* child, size_t index); /// Removes the first occurrence of child bool removeChild(Node* child); - void removeChild(uint32_t index); + void removeChild(size_t index); void cloneChildrenIfNeeded(void*); void markDirtyAndPropagate(); diff --git a/yoga/style/Style.h b/yoga/style/Style.h index 7b3ca9dd..5b78fa31 100644 --- a/yoga/style/Style.h +++ b/yoga/style/Style.h @@ -36,7 +36,7 @@ public: template struct BitfieldRef { Style& style; - size_t offset; + uint8_t offset; operator T() const { return getEnumData(style.flags, offset); } BitfieldRef& operator=(T x) { setEnumData(style.flags, offset, x); @@ -84,24 +84,24 @@ public: ~Style() = default; private: - static constexpr size_t directionOffset = 0; - static constexpr size_t flexdirectionOffset = + static constexpr uint8_t directionOffset = 0; + static constexpr uint8_t flexdirectionOffset = directionOffset + minimumBitCount(); - static constexpr size_t justifyContentOffset = + static constexpr uint8_t justifyContentOffset = flexdirectionOffset + minimumBitCount(); - static constexpr size_t alignContentOffset = + static constexpr uint8_t alignContentOffset = justifyContentOffset + minimumBitCount(); - static constexpr size_t alignItemsOffset = + static constexpr uint8_t alignItemsOffset = alignContentOffset + minimumBitCount(); - static constexpr size_t alignSelfOffset = + static constexpr uint8_t alignSelfOffset = alignItemsOffset + minimumBitCount(); - static constexpr size_t positionTypeOffset = + static constexpr uint8_t positionTypeOffset = alignSelfOffset + minimumBitCount(); - static constexpr size_t flexWrapOffset = + static constexpr uint8_t flexWrapOffset = positionTypeOffset + minimumBitCount(); - static constexpr size_t overflowOffset = + static constexpr uint8_t overflowOffset = flexWrapOffset + minimumBitCount(); - static constexpr size_t displayOffset = + static constexpr uint8_t displayOffset = overflowOffset + minimumBitCount(); uint32_t flags = 0;