diff --git a/Yoga.podspec b/Yoga.podspec index 4ebb5adff3..847b37735f 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/cmake/project-defaults.cmake b/cmake/project-defaults.cmake index d02a2836df..8868ce93d9 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/tests/NumericBitfieldTest.cpp b/tests/NumericBitfieldTest.cpp index 9b487cee21..4c25d56f84 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,7 +189,7 @@ 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; diff --git a/yoga/algorithm/CalculateLayout.cpp b/yoga/algorithm/CalculateLayout.cpp index e6a3dd5aaa..d2bc46b6cc 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 af3fc6927d..358c4ab7d5 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)) | - ((newValue << index) & (details::mask(minimumBitCount(), index))); -} - -template -void setEnumData(uint8_t& flags, size_t index, int newValue) { +void setEnumData(uint32_t& flags, uint8_t index, uint32_t newValue) { flags = (flags & - ~static_cast(details::mask(minimumBitCount(), index))) | - ((newValue << index) & - (static_cast(details::mask(minimumBitCount(), index)))); + ~static_cast(details::mask(minimumBitCount(), index))) | + ((newValue << index) & (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 756f480098..449fbc7012 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 3798e4663e..fca12aca63 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 95b4a21bf8..7d25e33c09 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 { + uint8_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 @@ struct LayoutResults { 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); } - 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 eb674a1dfe..70a81bd182 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 4116e5a6dc..ebedd863a5 100644 --- a/yoga/node/Node.h +++ b/yoga/node/Node.h @@ -181,8 +181,8 @@ class YOGA_EXPORT Node : public ::YGNode { return resolvedDimensions_; } - YGValue getResolvedDimension(int index) const { - return resolvedDimensions_[index]; + YGValue getResolvedDimension(YGDimension dimension) const { + return resolvedDimensions_[static_cast(dimension)]; } static CompactValue computeEdgeValueForColumn( @@ -303,14 +303,16 @@ class YOGA_EXPORT Node : public ::YGNode { 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 @@ class YOGA_EXPORT Node : public ::YGNode { 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 7b3ca9dd2d..5b78fa31e5 100644 --- a/yoga/style/Style.h +++ b/yoga/style/Style.h @@ -36,7 +36,7 @@ class YOGA_EXPORT Style { 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 @@ class YOGA_EXPORT Style { ~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;