From cd2179aeff11ef8cb5021d88c0e93a86dcf6cae7 Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Tue, 12 May 2026 22:26:03 +0530 Subject: [PATCH] Fix macOS/clang warnings in tests (#493) - Gate exactfloat scalbn/scalbln helpers on non-Apple; use ldexp in logb. - Drop redundant Result alias; fix structured-binding loop reference. - Migrate tests off deprecated decode factories and HexStringToBytes overload. --- src/s2/encoded_s2shape_index_test.cc | 38 ++++++++++++++----- src/s2/mutable_s2shape_index_test.cc | 3 +- src/s2/s2closest_edge_query_test.cc | 8 ++-- src/s2/s2lax_polygon_shape_test.cc | 2 +- src/s2/s2shapeutil_coding_test.cc | 15 ++++++-- .../util/math/exactfloat/exactfloat_test.cc | 4 +- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/s2/encoded_s2shape_index_test.cc b/src/s2/encoded_s2shape_index_test.cc index 55766cda4..25205bdbc 100644 --- a/src/s2/encoded_s2shape_index_test.cc +++ b/src/s2/encoded_s2shape_index_test.cc @@ -327,8 +327,10 @@ class LazyDecodeTest : public s2testing::ReaderWriterTest { encoded_.assign(encoder.base(), encoder.length()); Decoder decoder(encoded_.data(), encoded_.size()); + S2Error error; ABSL_CHECK( - index_.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder))); + index_.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder, error))); + ABSL_CHECK(error.ok()) << error.message(); } void WriteOp() override { @@ -373,13 +375,17 @@ TEST(EncodedS2ShapeIndex, JavaByteCompatibility) { // bytes is the encoded data of an S2ShapeIndex with a null shape and a // polyline with one edge. It was derived by base-16 encoding the buffer of // an encoder to which expected was encoded. - string bytes = absl::HexStringToBytes( + string bytes; + ASSERT_TRUE(absl::HexStringToBytes( "100036020102000000B4825F3C81FDEF3F27DCF7C958DE913F1EDD892B0BDF913FFC7FB8" - "B805F6EF3F28516A6D8FDBA13F27DCF7C958DEA13F28C809010408020010"); + "B805F6EF3F28516A6D8FDBA13F27DCF7C958DEA13F28C809010408020010", + &bytes)); Decoder decoder(bytes.data(), bytes.length()); MutableS2ShapeIndex actual; + S2Error decode_error; ASSERT_TRUE( - actual.Init(&decoder, s2shapeutil::FullDecodeShapeFactory(&decoder))); + actual.Init(&decoder, s2shapeutil::FullDecodeShapeFactory(&decoder, decode_error))); + ASSERT_TRUE(decode_error.ok()) << decode_error.message(); s2testing::ExpectEqual(expected, actual); } @@ -537,8 +543,10 @@ static void BM_MutableIndexDecodeDestruct(benchmark::State& state) { for (auto _ : state) { Decoder decoder(encoder.base(), encoder.length()); MutableS2ShapeIndex index; + S2Error error; ABSL_CHECK( - index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder))); + index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder, error))); + ABSL_CHECK(error.ok()) << error.message(); } } BENCHMARK(BM_MutableIndexDecodeDestruct)->Apply(BenchmarkArgsSnapped); @@ -559,8 +567,10 @@ static void BM_EncodedIndexInitDestruct(benchmark::State& state) { for (auto _ : state) { Decoder decoder(encoder.base(), encoder.length()); EncodedS2ShapeIndex index; + S2Error error; ABSL_CHECK( - index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder))); + index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder, error))); + ABSL_CHECK(error.ok()) << error.message(); } } BENCHMARK(BM_EncodedIndexInitDestruct)->Apply(BenchmarkArgsSnapped); @@ -679,8 +689,10 @@ static void BM_ContainsPointEncodedIndexPolygon(benchmark::State& state) { } Decoder decoder(encoder.base(), encoder.length()); EncodedS2ShapeIndex index; + S2Error init_error; ABSL_CHECK( - index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder))); + index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder, init_error))); + ABSL_CHECK(init_error.ok()) << init_error.message(); int i = 0, num_edges = index.shape(0)->num_edges(); for (auto _ : state) { S2Point test_point = index.shape(0)->edge(i).v0; @@ -728,8 +740,10 @@ static void BM_VertexDistanceEncodedIndexPolygon(benchmark::State& state) { } Decoder decoder(encoder.base(), encoder.length()); EncodedS2ShapeIndex index; + S2Error init_error; ABSL_CHECK( - index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder))); + index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder, init_error))); + ABSL_CHECK(init_error.ok()) << init_error.message(); int i = 0, num_edges = index.shape(0)->num_edges(); for (auto _ : state) { S2ClosestEdgeQuery query(&index); @@ -781,8 +795,10 @@ static void BM_DecodeIndexAndPolygonContainsPointSnapped( for (auto _ : state) { Decoder decoder(encoder.base(), encoder.length()); MutableS2ShapeIndex index; + S2Error error; ABSL_CHECK( - index.Init(&decoder, s2shapeutil::FullDecodeShapeFactory(&decoder))); + index.Init(&decoder, s2shapeutil::FullDecodeShapeFactory(&decoder, error))); + ABSL_CHECK(error.ok()) << error.message(); S2ContainsPointQuery query(&index); benchmark::DoNotOptimize(query.Contains(index.shape(0)->edge(0).v0)); } @@ -804,8 +820,10 @@ static void BM_EncodedIndexAndPolygonContainsPointSnapped( for (auto _ : state) { Decoder decoder(encoder.base(), encoder.length()); EncodedS2ShapeIndex index; + S2Error error; ABSL_CHECK( - index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder))); + index.Init(&decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder, error))); + ABSL_CHECK(error.ok()) << error.message(); S2ContainsPointQuery query(&index); benchmark::DoNotOptimize(query.Contains(index.shape(0)->edge(0).v0)); } diff --git a/src/s2/mutable_s2shape_index_test.cc b/src/s2/mutable_s2shape_index_test.cc index 101241bc1..175e36ec2 100644 --- a/src/s2/mutable_s2shape_index_test.cc +++ b/src/s2/mutable_s2shape_index_test.cc @@ -817,7 +817,8 @@ TEST_F(MutableS2ShapeIndexTest, DecoderCatchesInvalidIndex) { Decoder decoder(encoded.data(), encoded.size()); MutableS2ShapeIndex index; - bool ok = index.Init(&decoder, s2shapeutil::FullDecodeShapeFactory(&decoder)); + S2Error error; + bool ok = index.Init(&decoder, s2shapeutil::FullDecodeShapeFactory(&decoder, error)); EXPECT_FALSE(ok); } diff --git a/src/s2/s2closest_edge_query_test.cc b/src/s2/s2closest_edge_query_test.cc index 9787b63b9..51e9a88e6 100644 --- a/src/s2/s2closest_edge_query_test.cc +++ b/src/s2/s2closest_edge_query_test.cc @@ -50,6 +50,7 @@ #include "s2/s2closest_edge_query_testing.h" #include "s2/s2edge_crossings.h" #include "s2/s2edge_distances.h" +#include "s2/s2error.h" #include "s2/s2fractal.h" #include "s2/s2latlng.h" #include "s2/s2loop.h" @@ -343,8 +344,6 @@ TEST_F(VisitClosestEdgesTest, CanBreakFromShapeIteration) { } TEST_F(VisitClosestEdgesTest, CanBreakFromBruteForce) { - using Result = S2ClosestEdgeQuery::Result; - S2ClosestEdgeQuery::Options options; options.set_use_brute_force(true); options.set_include_interiors(false); @@ -969,8 +968,11 @@ static void BenchmarkFindClosest( e_data = string(encoder.base(), encoder.length()); Decoder decoder(e_data.data(), e_data.length()); if (encode_shapes) { + S2Error lazy_error; ABSL_CHECK(e_index.Init( - &decoder, s2shapeutil::LazyDecodeShapeFactory(&decoder))); + &decoder, + s2shapeutil::LazyDecodeShapeFactory(&decoder, lazy_error))); + ABSL_CHECK(lazy_error.ok()) << lazy_error.message(); } else { ABSL_CHECK(e_index.Init( &decoder, s2shapeutil::VectorShapeFactory(m_index.ReleaseAll()))); diff --git a/src/s2/s2lax_polygon_shape_test.cc b/src/s2/s2lax_polygon_shape_test.cc index 42506769a..ea02ac9a0 100644 --- a/src/s2/s2lax_polygon_shape_test.cc +++ b/src/s2/s2lax_polygon_shape_test.cc @@ -348,7 +348,7 @@ TEST(S2LaxPolygonShape, ManyLoopPolygon) { } } std::shuffle(edges.begin(), edges.end(), std::mt19937_64()); - for (const auto [e, i, j] : edges) { + for (const auto& [e, i, j] : edges) { EXPECT_EQ(shape.chain_position(e), S2Shape::ChainPosition(i, j)); auto v0 = loops[i][j]; auto v1 = loops[i][(j + 1) % loops[i].size()]; diff --git a/src/s2/s2shapeutil_coding_test.cc b/src/s2/s2shapeutil_coding_test.cc index abdf63d60..e9389130e 100644 --- a/src/s2/s2shapeutil_coding_test.cc +++ b/src/s2/s2shapeutil_coding_test.cc @@ -26,6 +26,7 @@ #include "absl/strings/escaping.h" #include "s2/util/coding/coder.h" #include "s2/mutable_s2shape_index.h" +#include "s2/s2error.h" #include "s2/s2lax_polygon_shape.h" #include "s2/s2lax_polyline_shape.h" #include "s2/s2point_vector_shape.h" @@ -58,8 +59,10 @@ TEST(FastEncodeTaggedShapes, MixedShapes) { index->Encode(&encoder); Decoder decoder(encoder.base(), encoder.length()); MutableS2ShapeIndex decoded_index; + S2Error error; ASSERT_TRUE(decoded_index.Init( - &decoder, s2shapeutil::FullDecodeShapeFactory(&decoder))); + &decoder, s2shapeutil::FullDecodeShapeFactory(&decoder, error))); + ASSERT_TRUE(error.ok()) << error.message(); EXPECT_EQ(s2textformat::ToString(*index), s2textformat::ToString(decoded_index)); } @@ -71,7 +74,8 @@ TEST(DecodeTaggedShapes, DecodeFromByteString) { auto polyline = s2textformat::MakePolylineOrDie("1:1, 1:2, 1:3"); index->Add(make_unique(*polyline)); index->Add(make_unique(*polygon)); - string bytes = absl::HexStringToBytes( + string bytes; + ASSERT_TRUE(absl::HexStringToBytes( "2932007C00E4002E0192010310000000000000F03F000000000000000000000000000000" "008AAFF597C0FEEF3F1EDD892B0BDF913F00000000000000000418B4825F3C81FDEF3F27" "DCF7C958DE913F1EDD892B0BDF913FD44A8442C3F9EF3FCE5B5A6FA6DDA13F1EDD892B0B" @@ -85,11 +89,14 @@ TEST(DecodeTaggedShapes, DecodeFromByteString) { "00000000003C4A985423D8EF3F199E8D966CD0B13F28516A6D8FDBB13FF6FF70710BECEF" "3F000000000000000028516A6D8FDBB13F28C83900010003010403040504070400073807" "0E1B24292B3213000009030002130000110300092B00010001000000010D000002230410" - "04020400020113082106110A4113000111030101"); + "04020400020113082106110A4113000111030101", + &bytes)); Decoder decoder(bytes.data(), bytes.length()); MutableS2ShapeIndex decoded_index; + S2Error decode_error; ASSERT_TRUE(decoded_index.Init( - &decoder, s2shapeutil::FullDecodeShapeFactory(&decoder))); + &decoder, s2shapeutil::FullDecodeShapeFactory(&decoder, decode_error))); + ASSERT_TRUE(decode_error.ok()) << decode_error.message(); EXPECT_EQ(s2textformat::ToString(*index), s2textformat::ToString(decoded_index)); } diff --git a/src/s2/util/math/exactfloat/exactfloat_test.cc b/src/s2/util/math/exactfloat/exactfloat_test.cc index 585253807..d1d9b04bd 100644 --- a/src/s2/util/math/exactfloat/exactfloat_test.cc +++ b/src/s2/util/math/exactfloat/exactfloat_test.cc @@ -96,7 +96,7 @@ double logb(double a) { // If "a" is denormalized, logb() is supposed to return the exponent that // "a" would have if it were normalized. (But it doesn't.) if (a != 0 && fabs(a) < std::numeric_limits::min()) { - return ::logb(scalbn(a, 100)) - 100; + return ::logb(ldexp(a, 100)) - 100; } return ::logb(a); } @@ -111,6 +111,7 @@ double ldexp(double a, int exp) { return r; } +#if !defined(__APPLE__) double scalbn(double a, int exp) { return ldexp(a, exp); // See ldexp(). } @@ -121,6 +122,7 @@ double scalbln(double a, long exp) { std::min(static_cast(INT_MAX), exp)); return ldexp(a, exp); // See ldexp(). } +#endif // !defined(__APPLE__) // The C standard does not specify the result of converting a floating-point // number to an integer if the argument is NaN or out of range. This applies