From 1c8e8d3aecc521642948e9f0676f57f56509d358 Mon Sep 17 00:00:00 2001 From: James Burnett Date: Thu, 4 Jul 2019 18:56:08 -0700 Subject: [PATCH] Compile Issues with Recent GCC (#895) Summary: GCC 8.3.0 (and possibly all gcc 7+) identified several warnings for signed unsigned integer comparison. With `-Werror` enabled this broke compiling tests. I suspect the warning is related to google/googletest#683. This diff updates those `ASSERT_EQ` calls that attempt to compare signed and unsigned errors by specifically declaring the literals to be unsigned. There is also an issue with Buck where it will not link to pthreads. facebook/buck#1443. Adding a `prebuilt_cxx_library` for pthread fixes that issue and the tests will compile and run. Finally, there was a warning about a missing return after a switch in `InstrumentationTest.cpp`. I added a `return ""` as a default, but it might be better to throw something. Thoughts? Pull Request resolved: https://github.com/facebook/yoga/pull/895 Reviewed By: davidaurelio Differential Revision: D15393082 Pulled By: davidaurelio fbshipit-source-id: 4f13ec2f016af39537c08fb591b188a6a0ed55ce --- lib/gtest/BUCK | 14 ++++++++++++-- tests/InstrumentationTest.cpp | 1 + tests/YGDefaultValuesTest.cpp | 2 +- tests/YGMeasureModeTest.cpp | 20 ++++++++++---------- tests/YGPersistenceTest.cpp | 28 ++++++++++++++-------------- 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/lib/gtest/BUCK b/lib/gtest/BUCK index 21dbc8b3..7be6a0bd 100644 --- a/lib/gtest/BUCK +++ b/lib/gtest/BUCK @@ -2,13 +2,21 @@ # # This source code is licensed under the MIT license found in the # LICENSE file in the root directory of this source tree. -load("//tools/build_defs/oss:yoga_defs.bzl", "YOGA_ROOTS", "subdir_glob", "yoga_cxx_library") +load("//tools/build_defs/oss:yoga_defs.bzl", "YOGA_ROOTS", "subdir_glob", "yoga_cxx_library", "yoga_prebuilt_cxx_library") COMPILER_FLAGS = [ "-std=c++14", "-Wno-missing-prototypes", ] +yoga_prebuilt_cxx_library( + name = "pthread", + exported_linker_flags = [ + "-lpthread", + ], + header_only = True, +) + yoga_cxx_library( name = "gtest", srcs = glob(["googletest/googletest/src/*.cc"]), @@ -20,5 +28,7 @@ yoga_cxx_library( ]), compiler_flags = COMPILER_FLAGS, visibility = YOGA_ROOTS, - deps = [], + deps = [ + ":pthread", + ], ) diff --git a/tests/InstrumentationTest.cpp b/tests/InstrumentationTest.cpp index e4128314..63a5f5f4 100644 --- a/tests/InstrumentationTest.cpp +++ b/tests/InstrumentationTest.cpp @@ -282,6 +282,7 @@ const char* markerTypeName(YGMarker type) { case YGMarkerBaselineFn: return "YGMarkerBaselineFn"; } + return ""; } template diff --git a/tests/YGDefaultValuesTest.cpp b/tests/YGDefaultValuesTest.cpp index f77b1924..6237dcb4 100644 --- a/tests/YGDefaultValuesTest.cpp +++ b/tests/YGDefaultValuesTest.cpp @@ -10,7 +10,7 @@ TEST(YogaTest, assert_default_values) { const YGNodeRef root = YGNodeNew(); - ASSERT_EQ(0, YGNodeGetChildCount(root)); + ASSERT_EQ(0u, YGNodeGetChildCount(root)); ASSERT_EQ(NULL, YGNodeGetChild(root, 1)); ASSERT_EQ(YGDirectionInherit, YGNodeStyleGetDirection(root)); diff --git a/tests/YGMeasureModeTest.cpp b/tests/YGMeasureModeTest.cpp index d3749c2f..f2927ce1 100644 --- a/tests/YGMeasureModeTest.cpp +++ b/tests/YGMeasureModeTest.cpp @@ -62,7 +62,7 @@ TEST(YogaTest, exactly_measure_stretched_child_column) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].width); ASSERT_EQ(YGMeasureModeExactly, constraintList.constraints[0].widthMode); @@ -91,7 +91,7 @@ TEST(YogaTest, exactly_measure_stretched_child_row) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].height); ASSERT_EQ(YGMeasureModeExactly, constraintList.constraints[0].heightMode); @@ -118,7 +118,7 @@ TEST(YogaTest, at_most_main_axis_column) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].height); ASSERT_EQ(YGMeasureModeAtMost, constraintList.constraints[0].heightMode); @@ -146,7 +146,7 @@ TEST(YogaTest, at_most_cross_axis_column) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].width); ASSERT_EQ(YGMeasureModeAtMost, constraintList.constraints[0].widthMode); @@ -174,7 +174,7 @@ TEST(YogaTest, at_most_main_axis_row) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].width); ASSERT_EQ(YGMeasureModeAtMost, constraintList.constraints[0].widthMode); @@ -203,7 +203,7 @@ TEST(YogaTest, at_most_cross_axis_row) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].height); ASSERT_EQ(YGMeasureModeAtMost, constraintList.constraints[0].heightMode); @@ -230,7 +230,7 @@ TEST(YogaTest, flex_child) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(2, constraintList.length); + ASSERT_EQ(2u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].height); ASSERT_EQ(YGMeasureModeAtMost, constraintList.constraints[0].heightMode); @@ -261,7 +261,7 @@ TEST(YogaTest, flex_child_with_flex_basis) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].height); ASSERT_EQ(YGMeasureModeExactly, constraintList.constraints[0].heightMode); @@ -290,7 +290,7 @@ TEST(YogaTest, overflow_scroll_column) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_FLOAT_EQ(100, constraintList.constraints[0].width); ASSERT_EQ(YGMeasureModeAtMost, constraintList.constraints[0].widthMode); @@ -323,7 +323,7 @@ TEST(YogaTest, overflow_scroll_row) { YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(1, constraintList.length); + ASSERT_EQ(1u, constraintList.length); ASSERT_TRUE(YGFloatIsUndefined(constraintList.constraints[0].width)); ASSERT_EQ(YGMeasureModeUndefined, constraintList.constraints[0].widthMode); diff --git a/tests/YGPersistenceTest.cpp b/tests/YGPersistenceTest.cpp index 9ad24111..338552c7 100644 --- a/tests/YGPersistenceTest.cpp +++ b/tests/YGPersistenceTest.cpp @@ -46,14 +46,14 @@ TEST(YogaTest, cloning_shared_root) { const YGNodeRef root2 = YGNodeClone(root); YGNodeStyleSetWidth(root2, 100); - ASSERT_EQ(2, YGNodeGetChildCount(root2)); + ASSERT_EQ(2u, YGNodeGetChildCount(root2)); // The children should have referential equality at this point. ASSERT_EQ(root_child0, YGNodeGetChild(root2, 0)); ASSERT_EQ(root_child1, YGNodeGetChild(root2, 1)); YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(2, YGNodeGetChildCount(root2)); + ASSERT_EQ(2u, YGNodeGetChildCount(root2)); // Relayout with no changed input should result in referential equality. ASSERT_EQ(root_child0, YGNodeGetChild(root2, 0)); ASSERT_EQ(root_child1, YGNodeGetChild(root2, 1)); @@ -62,7 +62,7 @@ TEST(YogaTest, cloning_shared_root) { YGNodeStyleSetHeight(root2, 200); YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR); - ASSERT_EQ(2, YGNodeGetChildCount(root2)); + ASSERT_EQ(2u, YGNodeGetChildCount(root2)); // Relayout with changed input should result in cloned children. const YGNodeRef root2_child0 = YGNodeGetChild(root2, 0); const YGNodeRef root2_child1 = YGNodeGetChild(root2, 1); @@ -112,26 +112,26 @@ TEST(YogaTest, mutating_children_of_a_clone_clones_only_after_layout) { const YGConfigRef config = YGConfigNew(); const YGNodeRef root = YGNodeNewWithConfig(config); - ASSERT_EQ(0, YGNodeGetChildCount(root)); + ASSERT_EQ(0u, YGNodeGetChildCount(root)); const YGNodeRef root2 = YGNodeClone(root); - ASSERT_EQ(0, YGNodeGetChildCount(root2)); + ASSERT_EQ(0u, YGNodeGetChildCount(root2)); const YGNodeRef root2_child0 = YGNodeNewWithConfig(config); YGNodeInsertChild(root2, root2_child0, 0); - ASSERT_EQ(0, YGNodeGetChildCount(root)); - ASSERT_EQ(1, YGNodeGetChildCount(root2)); + ASSERT_EQ(0u, YGNodeGetChildCount(root)); + ASSERT_EQ(1u, YGNodeGetChildCount(root2)); const YGNodeRef root3 = YGNodeClone(root2); - ASSERT_EQ(1, YGNodeGetChildCount(root2)); - ASSERT_EQ(1, YGNodeGetChildCount(root3)); + ASSERT_EQ(1u, YGNodeGetChildCount(root2)); + ASSERT_EQ(1u, YGNodeGetChildCount(root3)); ASSERT_EQ(YGNodeGetChild(root2, 0), YGNodeGetChild(root3, 0)); const YGNodeRef root3_child1 = YGNodeNewWithConfig(config); YGNodeInsertChild(root3, root3_child1, 1); - ASSERT_EQ(1, YGNodeGetChildCount(root2)); - ASSERT_EQ(2, YGNodeGetChildCount(root3)); + ASSERT_EQ(1u, YGNodeGetChildCount(root2)); + ASSERT_EQ(2u, YGNodeGetChildCount(root3)); ASSERT_EQ(root3_child1, YGNodeGetChild(root3, 1)); ASSERT_EQ(YGNodeGetChild(root2, 0), YGNodeGetChild(root3, 0)); @@ -139,8 +139,8 @@ TEST(YogaTest, mutating_children_of_a_clone_clones_only_after_layout) { ASSERT_EQ(root3_child1, YGNodeGetChild(root4, 1)); YGNodeRemoveChild(root4, root3_child1); - ASSERT_EQ(2, YGNodeGetChildCount(root3)); - ASSERT_EQ(1, YGNodeGetChildCount(root4)); + ASSERT_EQ(2u, YGNodeGetChildCount(root3)); + ASSERT_EQ(1u, YGNodeGetChildCount(root4)); ASSERT_EQ(YGNodeGetChild(root3, 0), YGNodeGetChild(root4, 0)); YGNodeCalculateLayout(root4, YGUndefined, YGUndefined, YGDirectionLTR); @@ -198,7 +198,7 @@ TEST(YogaTest, cloning_two_levels) { YGNodeRemoveAllChildren(root2); YGNodeInsertChild(root2, root2_child0, 0); YGNodeInsertChild(root2, root2_child1, 1); - ASSERT_EQ(2, YGNodeGetChildCount(root2)); + ASSERT_EQ(2u, YGNodeGetChildCount(root2)); YGNodeCalculateLayout(root2, YGUndefined, YGUndefined, YGDirectionLTR);