Eliminate YGFloatOptional::getValue()

Summary:
@public

`YGFloatOptional::getValue()` has the unfortunate property of calling `std::exit` if the wrapped value is undefined.
That forces `x.isUndefined() ? fallback : x.getValue()` as access pattern.

Here, we replace that by introducing `YGFloatOptional::orElse(float)` which encapsulates that pattern. Other additions are `orElseGet([] { … })` and some extra operators.

Reviewed By: SidharthGuglani

Differential Revision: D13209152

fbshipit-source-id: 4e5deceaaaaf8eaed44846a8c152cc8b235e815c
This commit is contained in:
David Aurelio
2018-12-06 07:35:10 -08:00
committed by Facebook Github Bot
parent ed3b54b603
commit 50ec35575f
8 changed files with 141 additions and 134 deletions

View File

@@ -6,7 +6,8 @@
*/
#include <yoga/YGFloatOptional.h>
#include <gtest/gtest.h>
#include <yoga/YGValue.h>
#include <yoga/YGFloatOptional.h>
#include <yoga/Yoga.h>
constexpr auto empty = YGFloatOptional{};
constexpr auto zero = YGFloatOptional{0.0f};
@@ -15,9 +16,10 @@ constexpr auto positive = YGFloatOptional{1234.5f};
constexpr auto negative = YGFloatOptional{-9876.5f};
TEST(YGFloatOptional, value) {
ASSERT_EQ(zero.getValue(), 0.0f);
ASSERT_EQ(positive.getValue(), 1234.5f);
ASSERT_EQ(negative.getValue(), -9876.5f);
ASSERT_EQ(zero.unwrap(), 0.0f);
ASSERT_EQ(positive.unwrap(), 1234.5f);
ASSERT_EQ(negative.unwrap(), -9876.5f);
ASSERT_TRUE(YGFloatIsUndefined(empty.unwrap()));
ASSERT_TRUE(empty.isUndefined());
ASSERT_FALSE(zero.isUndefined());
@@ -41,7 +43,7 @@ TEST(YGFloatOptional, equality) {
ASSERT_FALSE(one == positive);
ASSERT_TRUE(negative == negative);
ASSERT_TRUE(negative == negative.getValue());
ASSERT_TRUE(negative == negative.unwrap());
ASSERT_FALSE(negative == zero);
}
@@ -62,7 +64,7 @@ TEST(YGFloatOptional, inequality) {
ASSERT_TRUE(one != positive);
ASSERT_FALSE(negative != negative);
ASSERT_FALSE(negative != negative.getValue());
ASSERT_FALSE(negative != negative.unwrap());
ASSERT_TRUE(negative != zero);
}
@@ -72,6 +74,10 @@ TEST(YGFloatOptional, greater) {
ASSERT_FALSE(empty > one);
ASSERT_FALSE(empty > positive);
ASSERT_FALSE(empty > negative);
ASSERT_FALSE(zero > empty);
ASSERT_FALSE(one > empty);
ASSERT_FALSE(positive > empty);
ASSERT_FALSE(negative > empty);
ASSERT_TRUE(zero > negative);
ASSERT_FALSE(zero > zero);
@@ -93,6 +99,10 @@ TEST(YGFloatOptional, lower) {
ASSERT_FALSE(empty < one);
ASSERT_FALSE(empty < positive);
ASSERT_FALSE(empty < negative);
ASSERT_FALSE(zero < empty);
ASSERT_FALSE(one < empty);
ASSERT_FALSE(positive < empty);
ASSERT_FALSE(negative < empty);
ASSERT_TRUE(negative < zero);
ASSERT_FALSE(zero < zero);
@@ -156,8 +166,40 @@ TEST(YGFloatOptional, addition) {
ASSERT_EQ(zero + one, one);
ASSERT_EQ(
negative + positive,
YGFloatOptional{negative.getValue() + positive.getValue()});
YGFloatOptional{negative.unwrap() + positive.unwrap()});
ASSERT_EQ(empty + zero, empty);
ASSERT_EQ(empty + empty, empty);
ASSERT_EQ(negative + empty, empty);
}
TEST(YGFloatOptional, subtraction) {
ASSERT_EQ(zero - one, YGFloatOptional{-1.0f});
ASSERT_EQ(
negative - positive,
YGFloatOptional{negative.unwrap() - positive.unwrap()});
ASSERT_EQ(empty - zero, empty);
ASSERT_EQ(empty - empty, empty);
ASSERT_EQ(negative - empty, empty);
}
TEST(YGFloatOptional, unary_minus) {
ASSERT_EQ(-zero, zero);
ASSERT_EQ(-negative, YGFloatOptional{-negative.unwrap()});
ASSERT_EQ(-positive, YGFloatOptional{-positive.unwrap()});
ASSERT_EQ(-empty, empty);
}
TEST(YGFloatOptional, orElse) {
ASSERT_EQ(empty.orElse(1.23f), 1.23f);
ASSERT_TRUE(YGFloatIsUndefined(empty.orElse(YGUndefined)));
ASSERT_EQ(one.orElse(1.23f), 1.0f);
}
TEST(YGFloatOptional, orElseGet) {
auto x = empty.orElseGet([] { return 1.23f; });
ASSERT_EQ(x, 1.23f);
ASSERT_TRUE(YGFloatIsUndefined(empty.orElseGet([] { return YGUndefined; })));
auto y = one.orElseGet([] { return 1.23f; });
ASSERT_EQ(y, 1.0f);
}

View File

@@ -56,14 +56,17 @@ float YGFloatSanitize(const float val) {
}
float YGUnwrapFloatOptional(const YGFloatOptional& op) {
return op.isUndefined() ? YGUndefined : op.getValue();
return op.unwrap();
}
YGFloatOptional YGFloatOptionalMax(
const YGFloatOptional& op1,
const YGFloatOptional& op2) {
if (!op1.isUndefined() && !op2.isUndefined()) {
return op1.getValue() > op2.getValue() ? op1 : op2;
if (op1 > op2) {
return op1;
}
if (op2 > op1) {
return op2;
}
return op1.isUndefined() ? op2 : op1;
}

View File

@@ -12,15 +12,6 @@
using namespace facebook;
float YGFloatOptional::getValue() const {
if (isUndefined()) {
// Abort, accessing a value of an undefined float optional
std::cerr << "Tried to get value of an undefined YGFloatOptional\n";
std::exit(EXIT_FAILURE);
}
return value_;
}
bool YGFloatOptional::operator==(YGFloatOptional op) const {
return value_ == op.value_ || (isUndefined() && op.isUndefined());
}
@@ -37,10 +28,18 @@ bool YGFloatOptional::operator!=(float val) const {
return !(*this == val);
}
YGFloatOptional YGFloatOptional::operator-() const {
return YGFloatOptional{-value_};
}
YGFloatOptional YGFloatOptional::operator+(YGFloatOptional op) const {
return YGFloatOptional{value_ + op.value_};
}
YGFloatOptional YGFloatOptional::operator-(YGFloatOptional op) const {
return YGFloatOptional{value_ - op.value_};
}
bool YGFloatOptional::operator>(YGFloatOptional op) const {
return value_ > op.value_;
}

View File

@@ -6,7 +6,6 @@
*/
#pragma once
#include <cmath>
#include <limits>
struct YGFloatOptional {
@@ -17,16 +16,28 @@ struct YGFloatOptional {
explicit constexpr YGFloatOptional(float value) : value_(value) {}
constexpr YGFloatOptional() = default;
// Program will terminate if the value of an undefined is accessed. Please
// make sure to check if the optional is defined before calling this function.
// To check if float optional is defined, use `isUndefined()`.
float getValue() const;
bool isUndefined() const {
return std::isnan(value_);
// returns the wrapped value, or a value x with YGIsUndefined(x) == true
float unwrap() const {
return value_;
}
constexpr bool isUndefined() const {
// std::isnan is not constexpr
return !(value_ == value_);
}
constexpr float orElse(float other) const {
return isUndefined() ? other : value_;
}
template <typename Factory>
constexpr float orElseGet(Factory&& f) const {
return isUndefined() ? f() : value_;
}
YGFloatOptional operator-() const;
YGFloatOptional operator+(YGFloatOptional op) const;
YGFloatOptional operator-(YGFloatOptional op) const;
bool operator>(YGFloatOptional op) const;
bool operator<(YGFloatOptional op) const;
bool operator>=(YGFloatOptional op) const;

View File

@@ -209,11 +209,7 @@ YGFloatOptional YGNode::relativePosition(
return getLeadingPosition(axis, axisSize);
}
YGFloatOptional trailingPosition = getTrailingPosition(axis, axisSize);
if (!trailingPosition.isUndefined()) {
trailingPosition = YGFloatOptional{-1 * trailingPosition.getValue()};
}
return trailingPosition;
return -getTrailingPosition(axis, axisSize);
}
void YGNode::setPosition(
@@ -304,7 +300,7 @@ YGValue YGNode::resolveFlexBasisPtr() const {
if (flexBasis.unit != YGUnitAuto && flexBasis.unit != YGUnitUndefined) {
return flexBasis;
}
if (!style_.flex.isUndefined() && style_.flex.getValue() > 0.0f) {
if (style_.flex > YGFloatOptional{0.0f}) {
return config_->useWebDefaults ? YGValueAuto : YGValueZero;
}
return YGValueAuto;
@@ -394,27 +390,23 @@ float YGNode::resolveFlexGrow() {
if (owner_ == nullptr) {
return 0.0;
}
if (!style_.flexGrow.isUndefined()) {
return style_.flexGrow.getValue();
}
if (!style_.flex.isUndefined() && style_.flex.getValue() > 0.0f) {
return style_.flex.getValue();
}
return kDefaultFlexGrow;
return style_.flexGrow.orElseGet(
[this] { return style_.flex.orElse(kDefaultFlexGrow); });
}
float YGNode::resolveFlexShrink() {
if (owner_ == nullptr) {
return 0.0;
}
if (!style_.flexShrink.isUndefined()) {
return style_.flexShrink.getValue();
}
if (!config_->useWebDefaults && !style_.flex.isUndefined() &&
style_.flex.getValue() < 0.0f) {
return -style_.flex.getValue();
}
return config_->useWebDefaults ? kWebDefaultFlexShrink : kDefaultFlexShrink;
return style_.flexShrink.orElseGet([this] {
if (style_.flex < YGFloatOptional{0.0f} && !config_->useWebDefaults) {
return -style_.flex.unwrap();
} else {
return config_->useWebDefaults ? kWebDefaultFlexShrink
: kDefaultFlexShrink;
}
});
}
bool YGNode::isNodeFlexible() {
@@ -455,9 +447,7 @@ YGFloatOptional YGNode::getLeadingPadding(
const float widthSize) const {
const YGFloatOptional& paddingEdgeStart =
YGResolveValue(style_.padding[YGEdgeStart], widthSize);
if (YGFlexDirectionIsRow(axis) &&
style_.padding[YGEdgeStart].unit != YGUnitUndefined &&
!paddingEdgeStart.isUndefined() && paddingEdgeStart.getValue() >= 0.0f) {
if (YGFlexDirectionIsRow(axis) && paddingEdgeStart >= YGFloatOptional{0.0f}) {
return paddingEdgeStart;
}
@@ -470,11 +460,10 @@ YGFloatOptional YGNode::getLeadingPadding(
YGFloatOptional YGNode::getTrailingPadding(
const YGFlexDirection axis,
const float widthSize) const {
if (YGFlexDirectionIsRow(axis) &&
style_.padding[YGEdgeEnd].unit != YGUnitUndefined &&
!YGResolveValue(style_.padding[YGEdgeEnd], widthSize).isUndefined() &&
YGResolveValue(style_.padding[YGEdgeEnd], widthSize).getValue() >= 0.0f) {
return YGResolveValue(style_.padding[YGEdgeEnd], widthSize);
const YGFloatOptional& paddingEdgeEnd =
YGResolveValue(style_.padding[YGEdgeEnd], widthSize);
if (YGFlexDirectionIsRow(axis) && paddingEdgeEnd >= YGFloatOptional{0.0f}) {
return paddingEdgeEnd;
}
YGFloatOptional resolvedValue = YGResolveValue(

View File

@@ -43,7 +43,7 @@ static void appendFloatOptionalIfDefined(
const string key,
const YGFloatOptional num) {
if (!num.isUndefined()) {
appendFormatedString(base, "%s: %g; ", key.c_str(), num.getValue());
appendFormatedString(base, "%s: %g; ", key.c_str(), num.unwrap());
}
}
@@ -71,7 +71,6 @@ appendNumberIfNotAuto(string* base, const string& key, const YGValue number) {
static void
appendNumberIfNotZero(string* base, const string& str, const YGValue number) {
if (number.unit == YGUnitAuto) {
base->append(str + ": auto; ");
} else if (!YGFloatsEqual(number.value, 0)) {

View File

@@ -8,8 +8,8 @@
// Yoga specific properties, not compatible with flexbox specification
bool YGStyle::operator==(const YGStyle& style) {
bool areNonFloatValuesEqual = direction == style.direction &&
flexDirection == style.flexDirection &&
return (
direction == style.direction && flexDirection == style.flexDirection &&
justifyContent == style.justifyContent &&
alignContent == style.alignContent && alignItems == style.alignItems &&
alignSelf == style.alignSelf && positionType == style.positionType &&
@@ -21,34 +21,7 @@ bool YGStyle::operator==(const YGStyle& style) {
YGValueArrayEqual(border, style.border) &&
YGValueArrayEqual(dimensions, style.dimensions) &&
YGValueArrayEqual(minDimensions, style.minDimensions) &&
YGValueArrayEqual(maxDimensions, style.maxDimensions);
areNonFloatValuesEqual =
areNonFloatValuesEqual && flex.isUndefined() == style.flex.isUndefined();
if (areNonFloatValuesEqual && !flex.isUndefined() &&
!style.flex.isUndefined()) {
areNonFloatValuesEqual =
areNonFloatValuesEqual && flex.getValue() == style.flex.getValue();
}
areNonFloatValuesEqual = areNonFloatValuesEqual &&
flexGrow.isUndefined() == style.flexGrow.isUndefined();
if (areNonFloatValuesEqual && !flexGrow.isUndefined()) {
areNonFloatValuesEqual = areNonFloatValuesEqual &&
flexGrow.getValue() == style.flexGrow.getValue();
}
areNonFloatValuesEqual = areNonFloatValuesEqual &&
flexShrink.isUndefined() == style.flexShrink.isUndefined();
if (areNonFloatValuesEqual && !style.flexShrink.isUndefined()) {
areNonFloatValuesEqual = areNonFloatValuesEqual &&
flexShrink.getValue() == style.flexShrink.getValue();
}
if (!(aspectRatio.isUndefined() && style.aspectRatio.isUndefined())) {
areNonFloatValuesEqual = areNonFloatValuesEqual &&
aspectRatio.getValue() == style.aspectRatio.getValue();
}
return areNonFloatValuesEqual;
YGValueArrayEqual(maxDimensions, style.maxDimensions) &&
flex == style.flex && flexGrow == style.flexGrow &&
flexShrink == style.flexShrink && aspectRatio == style.aspectRatio);
}

View File

@@ -579,16 +579,14 @@ void YGNodeCopyStyle(const YGNodeRef dstNode, const YGNodeRef srcNode) {
}
float YGNodeStyleGetFlexGrow(const YGNodeRef node) {
return node->getStyle().flexGrow.isUndefined()
? kDefaultFlexGrow
: node->getStyle().flexGrow.getValue();
return node->getStyle().flexGrow.orElse(kDefaultFlexGrow);
}
float YGNodeStyleGetFlexShrink(const YGNodeRef node) {
return node->getStyle().flexShrink.isUndefined()
? (node->getConfig()->useWebDefaults ? kWebDefaultFlexShrink
: kDefaultFlexShrink)
: node->getStyle().flexShrink.getValue();
return node->getStyle().flexShrink.orElseGet([node] {
return node->getConfig()->useWebDefaults ? kWebDefaultFlexShrink
: kDefaultFlexShrink;
});
}
namespace {
@@ -856,8 +854,7 @@ void YGNodeStyleSetFlex(const YGNodeRef node, const float flex) {
// TODO(T26792433): Change the API to accept YGFloatOptional.
float YGNodeStyleGetFlex(const YGNodeRef node) {
return node->getStyle().flex.isUndefined() ? YGUndefined
: node->getStyle().flex.getValue();
return node->getStyle().flex.orElse(YGUndefined);
}
// TODO(T26792433): Change the API to accept YGFloatOptional.
@@ -959,7 +956,7 @@ float YGNodeStyleGetBorder(const YGNodeRef node, const YGEdge edge) {
// TODO(T26792433): Change the API to accept YGFloatOptional.
float YGNodeStyleGetAspectRatio(const YGNodeRef node) {
const YGFloatOptional op = node->getStyle().aspectRatio;
return op.isUndefined() ? YGUndefined : op.getValue();
return op.orElse(YGUndefined);
}
// TODO(T26792433): Change the API to accept YGFloatOptional.
@@ -1229,15 +1226,15 @@ static YGFloatOptional YGNodeBoundAxisWithinMinAndMax(
node->getStyle().maxDimensions[YGDimensionWidth], axisSize);
}
if (!max.isUndefined() && max.getValue() >= 0 && value > max.getValue()) {
if (max >= YGFloatOptional{0} && YGFloatOptional{value} > max) {
return max;
}
if (!min.isUndefined() && min.getValue() >= 0 && value < min.getValue()) {
if (min >= YGFloatOptional{0} && YGFloatOptional{value} < min) {
return min;
}
return YGFloatOptional(value);
return YGFloatOptional{value};
}
// Like YGNodeBoundAxisWithinMinAndMax but also ensures that the value doesn't
@@ -1278,14 +1275,14 @@ static void YGConstrainMaxSizeForMode(
switch (*mode) {
case YGMeasureModeExactly:
case YGMeasureModeAtMost:
*size = (maxSize.isUndefined() || *size < maxSize.getValue())
? *size
: maxSize.getValue();
if (YGFloatOptional{*size} > maxSize) {
*size = maxSize.unwrap();
}
break;
case YGMeasureModeUndefined:
if (!maxSize.isUndefined()) {
*mode = YGMeasureModeAtMost;
*size = maxSize.getValue();
*size = maxSize.unwrap();
}
break;
}
@@ -1395,16 +1392,15 @@ static void YGNodeComputeFlexBasisForChild(
}
}
if (!child->getStyle().aspectRatio.isUndefined()) {
auto hasAspectRatio = !child->getStyle().aspectRatio.isUndefined();
auto aspectRatio = child->getStyle().aspectRatio.unwrap();
if (hasAspectRatio) {
if (!isMainAxisRow && childWidthMeasureMode == YGMeasureModeExactly) {
childHeight = marginColumn +
(childWidth - marginRow) / child->getStyle().aspectRatio.getValue();
childHeight = marginColumn + (childWidth - marginRow) / aspectRatio;
childHeightMeasureMode = YGMeasureModeExactly;
} else if (
isMainAxisRow && childHeightMeasureMode == YGMeasureModeExactly) {
childWidth = marginRow +
(childHeight - marginColumn) *
child->getStyle().aspectRatio.getValue();
childWidth = marginRow + (childHeight - marginColumn) * aspectRatio;
childWidthMeasureMode = YGMeasureModeExactly;
}
}
@@ -1422,9 +1418,8 @@ static void YGNodeComputeFlexBasisForChild(
childWidthStretch) {
childWidth = width;
childWidthMeasureMode = YGMeasureModeExactly;
if (!child->getStyle().aspectRatio.isUndefined()) {
childHeight =
(childWidth - marginRow) / child->getStyle().aspectRatio.getValue();
if (hasAspectRatio) {
childHeight = (childWidth - marginRow) / aspectRatio;
childHeightMeasureMode = YGMeasureModeExactly;
}
}
@@ -1439,9 +1434,8 @@ static void YGNodeComputeFlexBasisForChild(
childHeight = height;
childHeightMeasureMode = YGMeasureModeExactly;
if (!child->getStyle().aspectRatio.isUndefined()) {
childWidth = (childHeight - marginColumn) *
child->getStyle().aspectRatio.getValue();
if (hasAspectRatio) {
childWidth = (childHeight - marginColumn) * aspectRatio;
childWidthMeasureMode = YGMeasureModeExactly;
}
}
@@ -1553,13 +1547,11 @@ static void YGNodeAbsoluteLayoutChild(
// flexible.
if (YGFloatIsUndefined(childWidth) ^ YGFloatIsUndefined(childHeight)) {
if (!child->getStyle().aspectRatio.isUndefined()) {
auto aspectRatio = child->getStyle().aspectRatio.unwrap();
if (YGFloatIsUndefined(childWidth)) {
childWidth = marginRow +
(childHeight - marginColumn) *
child->getStyle().aspectRatio.getValue();
childWidth = marginRow + (childHeight - marginColumn) * aspectRatio;
} else if (YGFloatIsUndefined(childHeight)) {
childHeight = marginColumn +
(childWidth - marginRow) / child->getStyle().aspectRatio.getValue();
childHeight = marginColumn + (childWidth - marginRow) / aspectRatio;
}
}
}
@@ -1885,16 +1877,15 @@ static float YGNodeCalculateAvailableInnerDim(
// constraints
const YGFloatOptional minDimensionOptional =
YGResolveValue(node->getStyle().minDimensions[dimension], ownerDim);
const float minInnerDim = minDimensionOptional.isUndefined()
? 0.0f
: minDimensionOptional.getValue() - paddingAndBorder;
const float minInnerDim =
(minDimensionOptional - YGFloatOptional{paddingAndBorder}).orElse(0.0f);
const YGFloatOptional maxDimensionOptional =
YGResolveValue(node->getStyle().maxDimensions[dimension], ownerDim);
const float maxInnerDim = maxDimensionOptional.isUndefined()
? FLT_MAX
: maxDimensionOptional.getValue() - paddingAndBorder;
const float maxInnerDim =
(maxDimensionOptional - YGFloatOptional{paddingAndBorder})
.orElse(FLT_MAX);
availableInnerDim =
YGFloatMax(YGFloatMin(availableInnerDim, maxInnerDim), minInnerDim);
}
@@ -2162,9 +2153,9 @@ static float YGDistributeFreeSpaceSecondPass(
if (!currentRelativeChild->getStyle().aspectRatio.isUndefined()) {
childCrossSize = isMainAxisRow ? (childMainSize - marginMain) /
currentRelativeChild->getStyle().aspectRatio.getValue()
currentRelativeChild->getStyle().aspectRatio.unwrap()
: (childMainSize - marginMain) *
currentRelativeChild->getStyle().aspectRatio.getValue();
currentRelativeChild->getStyle().aspectRatio.unwrap();
childCrossMeasureMode = YGMeasureModeExactly;
childCrossSize += marginCross;
@@ -3131,9 +3122,9 @@ static void YGNodelayoutImpl(
? ((YGUnwrapFloatOptional(child->getMarginForAxis(
crossAxis, availableInnerWidth)) +
(isMainAxisRow ? childMainSize /
child->getStyle().aspectRatio.getValue()
child->getStyle().aspectRatio.unwrap()
: childMainSize *
child->getStyle().aspectRatio.getValue())))
child->getStyle().aspectRatio.unwrap())))
: collectedFlexItemsValues.crossDim;
childMainSize += YGUnwrapFloatOptional(