Skip to content

Commit

Permalink
Breaking: Fix callback const-correctness (facebook#1369)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#1369

X-link: facebook/react-native#39370

This fixes const-correctness of callbacks (e.g. not letting a logger function modify nodes during layout). This helps us to continue to fix const-correctness issues inside of Yoga.

This change is breaking to the public API, since it requires a change in signature passed to Yoga.

Changelog: [Internal]

Differential Revision: https://internalfb.com/D49130714

fbshipit-source-id: 5aa0d28ee7ba7987bb0a59f3c3bc4e1234c118a8
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Sep 12, 2023
1 parent 11ebdf3 commit 9d0348b
Show file tree
Hide file tree
Showing 22 changed files with 111 additions and 111 deletions.
2 changes: 1 addition & 1 deletion benchmark/YGBenchmark.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ static void __printBenchmarkResult(
}

static YGSize _measure(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand Down
12 changes: 6 additions & 6 deletions java/jni/YGJNIVanilla.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ using namespace facebook::yoga;
using namespace facebook::yoga::vanillajni;

static inline ScopedLocalRef<jobject> YGNodeJobject(
YGNodeRef node,
YGNodeConstRef node,
void* layoutContext) {
return reinterpret_cast<PtrJNodeMapVanilla*>(layoutContext)->ref(node);
}
Expand Down Expand Up @@ -138,8 +138,8 @@ static jlong jni_YGNodeNewWithConfigJNI(
}

static int YGJNILogFunc(
const YGConfigRef config,
const YGNodeRef /*node*/,
const YGConfigConstRef config,
const YGNodeConstRef /*node*/,
YGLogLevel level,
void* /*layoutContext*/,
const char* format,
Expand Down Expand Up @@ -639,7 +639,7 @@ static void jni_YGNodeStyleSetBorderJNI(
yogaNodeRef, static_cast<YGEdge>(edge), static_cast<float>(border));
}

static void YGTransferLayoutDirection(YGNodeRef node, jobject javaNode) {
static void YGTransferLayoutDirection(YGNodeConstRef node, jobject javaNode) {
// Don't change this field name without changing the name of the field in
// Database.java
JNIEnv* env = getCurrentEnv();
Expand All @@ -655,7 +655,7 @@ static void YGTransferLayoutDirection(YGNodeRef node, jobject javaNode) {
}

static YGSize YGJNIMeasureFunc(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand Down Expand Up @@ -700,7 +700,7 @@ static void jni_YGNodeSetHasMeasureFuncJNI(
}

static float YGJNIBaselineFunc(
YGNodeRef node,
YGNodeConstRef node,
float width,
float height,
void* layoutContext) {
Expand Down
4 changes: 2 additions & 2 deletions javascript/src/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "./Config.h"

static YGSize globalMeasureFunc(
YGNodeRef nodeRef,
YGNodeConstRef nodeRef,
float width,
YGMeasureMode widthMode,
float height,
Expand All @@ -29,7 +29,7 @@ static YGSize globalMeasureFunc(
return ygSize;
}

static void globalDirtiedFunc(YGNodeRef nodeRef) {
static void globalDirtiedFunc(YGNodeConstRef nodeRef) {
Node const& node = *reinterpret_cast<Node const*>(YGNodeGetContext(nodeRef));

node.callDirtiedFunc();
Expand Down
5 changes: 3 additions & 2 deletions tests/EventsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ TEST_F(EventTest, layout_events_has_max_measure_cache) {
TEST_F(EventTest, measure_functions_get_wrapped) {
auto root = YGNodeNew();
YGNodeSetMeasureFunc(
root, [](YGNodeRef, float, YGMeasureMode, float, YGMeasureMode) {
root, [](YGNodeConstRef, float, YGMeasureMode, float, YGMeasureMode) {
return YGSize{};
});

Expand All @@ -267,7 +267,8 @@ TEST_F(EventTest, baseline_functions_get_wrapped) {
auto child = YGNodeNew();
YGNodeInsertChild(root, child, 0);

YGNodeSetBaselineFunc(child, [](YGNodeRef, float, float) { return 0.0f; });
YGNodeSetBaselineFunc(
child, [](YGNodeConstRef, float, float) { return 0.0f; });
YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
YGNodeStyleSetAlignItems(root, YGAlignBaseline);

Expand Down
6 changes: 3 additions & 3 deletions tests/YGAlignBaselineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
#include <yoga/Yoga.h>

static float _baselineFunc(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
const float /*width*/,
const float height) {
return height / 2;
}

static YGSize _measure1(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float /*width*/,
YGMeasureMode /*widthMode*/,
float /*height*/,
Expand All @@ -25,7 +25,7 @@ static YGSize _measure1(
}

static YGSize _measure2(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float /*width*/,
YGMeasureMode /*widthMode*/,
float /*height*/,
Expand Down
2 changes: 1 addition & 1 deletion tests/YGAspectRatioTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <yoga/Yoga.h>

static YGSize _measure(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float width,
YGMeasureMode widthMode,
float height,
Expand Down
2 changes: 1 addition & 1 deletion tests/YGBaselineFuncTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <yoga/Yoga.h>

static float _baseline(
YGNodeRef node,
YGNodeConstRef node,
const float /*width*/,
const float /*height*/) {
float* baseline = (float*) YGNodeGetContext(node);
Expand Down
2 changes: 1 addition & 1 deletion tests/YGDirtiedTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

using namespace facebook;

static void _dirtied(YGNodeRef node) {
static void _dirtied(YGNodeConstRef node) {
int* dirtiedCount = (int*) YGNodeGetContext(node);
(*dirtiedCount)++;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/YGDirtyMarkingTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ TEST(YogaTest, dirty_propagation_changing_benign_config) {
YGConfigRef newConfig = YGConfigNew();
YGConfigSetLogger(
newConfig,
[](const YGConfigRef, const YGNodeRef, YGLogLevel, const char*, va_list) {
[](YGConfigConstRef, YGNodeConstRef, YGLogLevel, const char*, va_list) {
return 0;
});
YGNodeSetConfig(root_child0, newConfig);
Expand Down
4 changes: 2 additions & 2 deletions tests/YGLoggerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
namespace {
char writeBuffer[4096];
int _unmanagedLogger(
const YGConfigRef /*config*/,
const YGNodeRef /*node*/,
const YGConfigConstRef /*config*/,
const YGNodeConstRef /*node*/,
YGLogLevel /*level*/,
const char* format,
va_list args) {
Expand Down
6 changes: 3 additions & 3 deletions tests/YGMeasureCacheTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <yoga/Yoga.h>

static YGSize _measureMax(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand All @@ -24,7 +24,7 @@ static YGSize _measureMax(
}

static YGSize _measureMin(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand All @@ -44,7 +44,7 @@ static YGSize _measureMin(
}

static YGSize _measure_84_49(
YGNodeRef node,
YGNodeConstRef node,
float /*width*/,
YGMeasureMode /*widthMode*/,
float /*height*/,
Expand Down
2 changes: 1 addition & 1 deletion tests/YGMeasureModeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ struct _MeasureConstraintList {
};

static YGSize _measure(
YGNodeRef node,
YGNodeConstRef node,
float width,
YGMeasureMode widthMode,
float height,
Expand Down
10 changes: 5 additions & 5 deletions tests/YGMeasureTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <yoga/Yoga.h>

static YGSize _measure(
YGNodeRef node,
YGNodeConstRef node,
float /*width*/,
YGMeasureMode /*widthMode*/,
float /*height*/,
Expand All @@ -23,7 +23,7 @@ static YGSize _measure(
}

static YGSize _simulate_wrapping_text(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float width,
YGMeasureMode widthMode,
float /*height*/,
Expand All @@ -36,7 +36,7 @@ static YGSize _simulate_wrapping_text(
}

static YGSize _measure_assert_negative(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float width,
YGMeasureMode /*widthMode*/,
float height,
Expand Down Expand Up @@ -640,7 +640,7 @@ TEST(YogaTest, cant_call_negative_measure_horizontal) {
}

static YGSize _measure_90_10(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float /*width*/,
YGMeasureMode /*widthMode*/,
float /*height*/,
Expand All @@ -650,7 +650,7 @@ static YGSize _measure_90_10(
}

static YGSize _measure_100_100(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float /*width*/,
YGMeasureMode /*widthMode*/,
float /*height*/,
Expand Down
46 changes: 25 additions & 21 deletions tests/YGNodeCallbackTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,18 @@ TEST(Node, hasMeasureFunc_initial) {

TEST(Node, hasMeasureFunc_with_measure_fn) {
auto n = Node{};
n.setMeasureFunc([](YGNodeRef, float, YGMeasureMode, float, YGMeasureMode) {
return YGSize{};
});
n.setMeasureFunc(
[](YGNodeConstRef, float, YGMeasureMode, float, YGMeasureMode) {
return YGSize{};
});
ASSERT_TRUE(n.hasMeasureFunc());
}

TEST(Node, measure_with_measure_fn) {
auto n = Node{};

n.setMeasureFunc(
[](YGNodeRef, float w, YGMeasureMode wm, float h, YGMeasureMode hm) {
[](YGNodeConstRef, float w, YGMeasureMode wm, float h, YGMeasureMode hm) {
return YGSize{w * static_cast<float>(wm), h / static_cast<float>(hm)};
});

Expand All @@ -43,10 +44,12 @@ TEST(Node, measure_with_measure_fn) {

TEST(Node, measure_with_context_measure_fn) {
auto n = Node{};
n.setMeasureFunc(
[](YGNodeRef, float, YGMeasureMode, float, YGMeasureMode, void* ctx) {
return *(YGSize*) ctx;
});
n.setMeasureFunc([](YGNodeConstRef,
float,
YGMeasureMode,
float,
YGMeasureMode,
void* ctx) { return *(YGSize*) ctx; });

auto result = YGSize{123.4f, -56.7f};
ASSERT_EQ(
Expand All @@ -57,11 +60,11 @@ TEST(Node, measure_with_context_measure_fn) {
TEST(Node, switching_measure_fn_types) {
auto n = Node{};
n.setMeasureFunc(
[](YGNodeRef, float, YGMeasureMode, float, YGMeasureMode, void*) {
[](YGNodeConstRef, float, YGMeasureMode, float, YGMeasureMode, void*) {
return YGSize{};
});
n.setMeasureFunc(
[](YGNodeRef, float w, YGMeasureMode wm, float h, YGMeasureMode hm) {
[](YGNodeConstRef, float w, YGMeasureMode wm, float h, YGMeasureMode hm) {
return YGSize{w * static_cast<float>(wm), h / static_cast<float>(hm)};
});

Expand All @@ -72,9 +75,10 @@ TEST(Node, switching_measure_fn_types) {

TEST(Node, hasMeasureFunc_after_unset) {
auto n = Node{};
n.setMeasureFunc([](YGNodeRef, float, YGMeasureMode, float, YGMeasureMode) {
return YGSize{};
});
n.setMeasureFunc(
[](YGNodeConstRef, float, YGMeasureMode, float, YGMeasureMode) {
return YGSize{};
});

n.setMeasureFunc(nullptr);
ASSERT_FALSE(n.hasMeasureFunc());
Expand All @@ -83,7 +87,7 @@ TEST(Node, hasMeasureFunc_after_unset) {
TEST(Node, hasMeasureFunc_after_unset_context) {
auto n = Node{};
n.setMeasureFunc(
[](YGNodeRef, float, YGMeasureMode, float, YGMeasureMode, void*) {
[](YGNodeConstRef, float, YGMeasureMode, float, YGMeasureMode, void*) {
return YGSize{};
});

Expand All @@ -98,20 +102,20 @@ TEST(Node, hasBaselineFunc_initial) {

TEST(Node, hasBaselineFunc_with_baseline_fn) {
auto n = Node{};
n.setBaselineFunc([](YGNodeRef, float, float) { return 0.0f; });
n.setBaselineFunc([](YGNodeConstRef, float, float) { return 0.0f; });
ASSERT_TRUE(n.hasBaselineFunc());
}

TEST(Node, baseline_with_baseline_fn) {
auto n = Node{};
n.setBaselineFunc([](YGNodeRef, float w, float h) { return w + h; });
n.setBaselineFunc([](YGNodeConstRef, float w, float h) { return w + h; });

ASSERT_EQ(n.baseline(1.25f, 2.5f, nullptr), 3.75f);
}

TEST(Node, baseline_with_context_baseline_fn) {
auto n = Node{};
n.setBaselineFunc([](YGNodeRef, float w, float h, void* ctx) {
n.setBaselineFunc([](YGNodeConstRef, float w, float h, void* ctx) {
return w + h + *(float*) ctx;
});

Expand All @@ -121,23 +125,23 @@ TEST(Node, baseline_with_context_baseline_fn) {

TEST(Node, hasBaselineFunc_after_unset) {
auto n = Node{};
n.setBaselineFunc([](YGNodeRef, float, float) { return 0.0f; });
n.setBaselineFunc([](YGNodeConstRef, float, float) { return 0.0f; });

n.setBaselineFunc(nullptr);
ASSERT_FALSE(n.hasBaselineFunc());
}

TEST(Node, hasBaselineFunc_after_unset_context) {
auto n = Node{};
n.setBaselineFunc([](YGNodeRef, float, float, void*) { return 0.0f; });
n.setBaselineFunc([](YGNodeConstRef, float, float, void*) { return 0.0f; });

n.setMeasureFunc(nullptr);
ASSERT_FALSE(n.hasMeasureFunc());
}

TEST(Node, switching_baseline_fn_types) {
auto n = Node{};
n.setBaselineFunc([](YGNodeRef, float, float, void*) { return 0.0f; });
n.setBaselineFunc([](YGNodeRef, float, float) { return 1.0f; });
n.setBaselineFunc([](YGNodeConstRef, float, float, void*) { return 0.0f; });
n.setBaselineFunc([](YGNodeConstRef, float, float) { return 1.0f; });
ASSERT_EQ(n.baseline(1, 2, nullptr), 1.0f);
}
2 changes: 1 addition & 1 deletion tests/YGRoundingFunctionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ TEST(YogaTest, rounding_value) {
}

static YGSize measureText(
YGNodeRef /*node*/,
YGNodeConstRef /*node*/,
float /*width*/,
YGMeasureMode /*widthMode*/,
float /*height*/,
Expand Down
Loading

0 comments on commit 9d0348b

Please sign in to comment.