diff --git a/CMakeLists.txt b/CMakeLists.txt index 249b7d3b..67d81cd3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -18,14 +18,10 @@ endif() # s2geometry needs to use the same C++ standard that absl used to avoid # undefined symbol errors since ABSL_HAVE_STD_STRING_VIEW etc will # end up defined differently. There is probably a better way to achieve -# this than assuming what absl used. s2geometry should actually work fine -# with C++11, but there are a few places that use std::exchange instead of -# absl::exchange, is_x_t, and some atomic constructors that would need -# to change. File a bug if you need support for earlier versions. -# TODO(jrosenstock,b/205952836): Use absl functions if possible. -set(CMAKE_CXX_STANDARD 17) +# this than assuming what absl used. +set(CMAKE_CXX_STANDARD 11) set(CMAKE_CXX_STANDARD_REQUIRED ON) -# No compiler-specific extensions, i.e. -std=c++17, not -std=gnu++17. +# No compiler-specific extensions, i.e. -std=c++11, not -std=gnu++11. set(CMAKE_CXX_EXTENSIONS OFF) list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/third_party/cmake") diff --git a/README.md b/README.md index 9fe71bd9..cf9a5ba4 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ S2 documentation can be found on [s2geometry.io](http://s2geometry.io). ## Requirements for End Users * [CMake](http://www.cmake.org/) -* A C++ compiler with C++17 support, such as [g++ 7](https://gcc.gnu.org/) +* A C++ compiler with C++11 support, such as [g++ >= 4.7](https://gcc.gnu.org/) * [Abseil](https://github.com/abseil/abseil-cpp) (standard library extensions) * [OpenSSL](https://github.com/openssl/openssl) (for its bignum library) * [gflags command line flags](https://github.com/gflags/gflags), optional @@ -85,6 +85,10 @@ cd s2geometry First, [install Abseil](https://github.com/abseil/abseil-cpp/blob/master/CMake/README.md#traditional-cmake-set-up). It must be configured with `-DCMAKE_POSITION_INDEPENDENT_CODE=ON`. +s2geometry must be configured to use the came C++ version that +abseil uses. The easiest way to achieve this is to pass +`-DCMAKE_CXX_STANDARD=11` (or `-DCMAKE_CXX_STANDARD=17`) to `cmake` +when compiling both abseil and s2geometry. From the appropriate directory depending on how you got the source: @@ -92,7 +96,8 @@ From the appropriate directory depending on how you got the source: mkdir build cd build # You can omit -DGTEST_ROOT to skip tests; see above for macOS. -cmake -DGTEST_ROOT=/usr/src/gtest -DCMAKE_PREFIX_PATH=/path/to/absl/install .. +# Use the same CMAKE_CXX_STANDARD value that was used with absl. +cmake -DGTEST_ROOT=/usr/src/gtest -DCMAKE_PREFIX_PATH=/path/to/absl/install -DCMAKE_CXX_STANDARD=11 .. make -j $(nproc) make test ARGS="-j$(nproc)" # If GTEST_ROOT specified above. sudo make install diff --git a/src/s2/base/casts.h b/src/s2/base/casts.h index 65579eb1..22070e52 100644 --- a/src/s2/base/casts.h +++ b/src/s2/base/casts.h @@ -58,7 +58,7 @@ template // use like this: down_cast(foo); inline To down_cast(From* f) { // so we only accept pointers - static_assert((std::is_base_of>::value), + static_assert((std::is_base_of>::value), "target type not derived from source type"); // We skip the assert and hence the dynamic_cast if RTTI is disabled. @@ -82,13 +82,13 @@ template inline To down_cast(From& f) { static_assert(std::is_lvalue_reference::value, "target type not a reference"); - static_assert((std::is_base_of>::value), + static_assert((std::is_base_of>::value), "target type not derived from source type"); // We skip the assert and hence the dynamic_cast if RTTI is disabled. #if !defined(__GNUC__) || defined(__GXX_RTTI) // RTTI: debug mode only - assert(dynamic_cast*>(&f) != nullptr); + assert(dynamic_cast*>(&f) != nullptr); #endif // !defined(__GNUC__) || defined(__GXX_RTTI) return static_cast(f); diff --git a/src/s2/mutable_s2shape_index.h b/src/s2/mutable_s2shape_index.h index 56a5eaef..96dfb015 100644 --- a/src/s2/mutable_s2shape_index.h +++ b/src/s2/mutable_s2shape_index.h @@ -541,7 +541,7 @@ class MutableS2ShapeIndex final : public S2ShapeIndex { FRESH, // There are no pending updates. }; // Reads and writes to this field are guarded by "lock_". - std::atomic index_status_ = FRESH; + std::atomic index_status_{FRESH}; // UpdateState holds temporary data related to thread synchronization. It // is only allocated while updates are being applied. diff --git a/src/s2/r1interval.h b/src/s2/r1interval.h index ba0b8c26..b670b5c1 100644 --- a/src/s2/r1interval.h +++ b/src/s2/r1interval.h @@ -148,9 +148,14 @@ class R1Interval { // Expand the interval so that it contains the given point "p". void AddPoint(double p) { - if (is_empty()) { set_lo(p); set_hi(p); } - else if (p < lo()) { set_lo(p); } // NOLINT - else if (p > hi()) { set_hi(p); } // NOLINT + if (is_empty()) { + set_lo(p); + set_hi(p); + } else if (p < lo()) { + set_lo(p); + } else if (p > hi()) { + set_hi(p); + } } // Expand the interval so that it contains the given interval "y". diff --git a/src/s2/s1interval.h b/src/s2/s1interval.h index 74c7885d..b11b2baf 100644 --- a/src/s2/s1interval.h +++ b/src/s2/s1interval.h @@ -21,6 +21,7 @@ #include #include #include +#include #include "s2/base/logging.h" #include "s2/_fp_contract_off.h" diff --git a/src/s2/s2boolean_operation.h b/src/s2/s2boolean_operation.h index 5d22ff05..e04266cc 100644 --- a/src/s2/s2boolean_operation.h +++ b/src/s2/s2boolean_operation.h @@ -21,6 +21,7 @@ #include #include #include + #include "s2/s2builder.h" #include "s2/s2builder_graph.h" #include "s2/s2builder_layer.h" @@ -452,7 +453,7 @@ class S2BooleanOperation { private: std::unique_ptr snap_function_; - PolygonModel polygon_model_ = PolygonModel::SEMI_OPEN;; + PolygonModel polygon_model_ = PolygonModel::SEMI_OPEN; PolylineModel polyline_model_ = PolylineModel::CLOSED; bool polyline_loops_have_boundaries_ = true; bool split_all_crossing_polyline_edges_ = false; diff --git a/src/s2/s2boolean_operation_test.cc b/src/s2/s2boolean_operation_test.cc index 089afc36..bda0c717 100644 --- a/src/s2/s2boolean_operation_test.cc +++ b/src/s2/s2boolean_operation_test.cc @@ -18,7 +18,9 @@ #include "s2/s2boolean_operation.h" #include +#include #include +#include #include diff --git a/src/s2/s2buffer_operation.cc b/src/s2/s2buffer_operation.cc index 91959c80..ab053f9c 100644 --- a/src/s2/s2buffer_operation.cc +++ b/src/s2/s2buffer_operation.cc @@ -51,6 +51,7 @@ #include #include #include +#include #include #include "absl/memory/memory.h" @@ -128,6 +129,10 @@ static constexpr S1Angle kMinRequestedError = S1Angle::Radians(2 * DBL_ERR); static constexpr S1Angle kMaxAbsoluteInterpolationError = S2::kGetPointOnLineError + S2::kGetPointOnRayPerpendicularError; +// TODO(user, b/210097200): Remove when we require c++17 for opensource. +constexpr double S2BufferOperation::Options::kMinErrorFraction; +constexpr double S2BufferOperation::Options::kMaxCircleSegments; + S2BufferOperation::Options::Options() : snap_function_( make_unique(S1Angle::Zero())) { diff --git a/src/s2/s2buffer_operation_test.cc b/src/s2/s2buffer_operation_test.cc index a0709a25..76fcac48 100644 --- a/src/s2/s2buffer_operation_test.cc +++ b/src/s2/s2buffer_operation_test.cc @@ -18,8 +18,11 @@ #include "s2/s2buffer_operation.h" #include +#include #include #include +#include +#include #include "s2/base/casts.h" #include "s2/base/logging.h" @@ -288,8 +291,8 @@ void TestMinimumDistance(const MutableS2ShapeIndex& input, S2ClosestEdgeQuery::EdgeTarget in_target(a.v0, a.v1); for (const auto& out_result : out_query.FindClosestEdges(&in_target)) { auto b = output.shape(out_result.shape_id())->edge(out_result.edge_id()); - ASSERT_GE(s2pred::CompareEdgePairDistance( - a.v0, a.v1, b.v0, b.v1, min_dist), 0);; + ASSERT_GE( + s2pred::CompareEdgePairDistance(a.v0, a.v1, b.v0, b.v1, min_dist), 0); } } } diff --git a/src/s2/s2builder.cc b/src/s2/s2builder.cc index 9ca2ca85..a935ba8b 100644 --- a/src/s2/s2builder.cc +++ b/src/s2/s2builder.cc @@ -74,6 +74,7 @@ #include #include #include +#include #include #include "absl/cleanup/cleanup.h" diff --git a/src/s2/s2builder.h b/src/s2/s2builder.h index bfe3cb78..8f67f8e8 100644 --- a/src/s2/s2builder.h +++ b/src/s2/s2builder.h @@ -21,6 +21,8 @@ #ifndef S2_S2BUILDER_H_ #define S2_S2BUILDER_H_ +#include +#include #include #include #include diff --git a/src/s2/s2builder_graph.cc b/src/s2/s2builder_graph.cc index 26448916..68162ee7 100644 --- a/src/s2/s2builder_graph.cc +++ b/src/s2/s2builder_graph.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "s2/base/logging.h" diff --git a/src/s2/s2builder_graph.h b/src/s2/s2builder_graph.h index a919502d..767e2482 100644 --- a/src/s2/s2builder_graph.h +++ b/src/s2/s2builder_graph.h @@ -18,9 +18,11 @@ #ifndef S2_S2BUILDER_GRAPH_H_ #define S2_S2BUILDER_GRAPH_H_ +#include #include #include #include +#include #include #include diff --git a/src/s2/s2builder_graph_test.cc b/src/s2/s2builder_graph_test.cc index 45b1402f..1bfd00f1 100644 --- a/src/s2/s2builder_graph_test.cc +++ b/src/s2/s2builder_graph_test.cc @@ -65,7 +65,7 @@ TEST(Graph, LabelsRequestedButNotProvided) { vector vertices{S2Point(1, 0, 0)}; vector edges{{0, 0}}; vector input_edge_id_set_ids{0}; - IdSetLexicon input_edge_id_set_lexicon, label_set_lexicon;; + IdSetLexicon input_edge_id_set_lexicon, label_set_lexicon; vector label_set_ids; // Empty means no labels are present. Graph g{ options, &vertices, &edges, &input_edge_id_set_ids, diff --git a/src/s2/s2builder_test.cc b/src/s2/s2builder_test.cc index 243eb00c..d9e419d7 100644 --- a/src/s2/s2builder_test.cc +++ b/src/s2/s2builder_test.cc @@ -84,7 +84,7 @@ using s2textformat::MakePolylineOrDie; using EdgeType = S2Builder::EdgeType; using InputEdgeId = S2Builder::Graph::InputEdgeId; using Graph = S2Builder::Graph; -using GraphOptions = S2Builder::GraphOptions;; +using GraphOptions = S2Builder::GraphOptions; S2_DEFINE_int32(iteration_multiplier, 1, "Iteration multiplier for randomized tests"); diff --git a/src/s2/s2builderutil_closed_set_normalizer.cc b/src/s2/s2builderutil_closed_set_normalizer.cc index 8be85f72..dc1965b6 100644 --- a/src/s2/s2builderutil_closed_set_normalizer.cc +++ b/src/s2/s2builderutil_closed_set_normalizer.cc @@ -18,6 +18,7 @@ #include "s2/s2builderutil_closed_set_normalizer.h" #include +#include #include "absl/memory/memory.h" #include "s2/s2builder_layer.h" diff --git a/src/s2/s2closest_cell_query_test.cc b/src/s2/s2closest_cell_query_test.cc index 0b0cf065..3c418606 100644 --- a/src/s2/s2closest_cell_query_test.cc +++ b/src/s2/s2closest_cell_query_test.cc @@ -222,18 +222,6 @@ static const S1Angle kTestCapRadius = S2Testing::KmToAngle(10); using TestingResult = pair; -// Converts to the format required by CheckDistanceResults() in s2testing.h. -vector ConvertResults( - const vector& results) { - vector testing_results; - for (const auto& result : results) { - testing_results.push_back( - TestingResult(result.distance(), - LabelledCell(result.cell_id(), result.label()))); - } - return testing_results; -} - // Use "query" to find the closest cell(s) to the given target, and extract // the query results into the given vector. Also verify that the results // satisfy the search criteria. diff --git a/src/s2/s2edge_distances.cc b/src/s2/s2edge_distances.cc index eeebdbba..20ef1aab 100644 --- a/src/s2/s2edge_distances.cc +++ b/src/s2/s2edge_distances.cc @@ -334,10 +334,13 @@ bool UpdateEdgePairMinDistance( // // The calculation below computes each of the six vertex-vertex distances // twice (this could be optimized). - return (UpdateMinDistance(a0, b0, b1, min_dist) | - UpdateMinDistance(a1, b0, b1, min_dist) | - UpdateMinDistance(b0, a0, a1, min_dist) | - UpdateMinDistance(b1, a0, a1, min_dist)); + // + // We do not want the short circuit behavior of ||. Suppress + // -Wbitwise-instead-of-logical errors by converting to int. + return (int{UpdateMinDistance(a0, b0, b1, min_dist)} | + int{UpdateMinDistance(a1, b0, b1, min_dist)} | + int{UpdateMinDistance(b0, a0, a1, min_dist)} | + int{UpdateMinDistance(b1, a0, a1, min_dist)}); } bool UpdateEdgePairMaxDistance( @@ -357,10 +360,13 @@ bool UpdateEdgePairMaxDistance( // // The calculation below computes each of the six vertex-vertex distances // twice (this could be optimized). - return (UpdateMaxDistance(a0, b0, b1, max_dist) | - UpdateMaxDistance(a1, b0, b1, max_dist) | - UpdateMaxDistance(b0, a0, a1, max_dist) | - UpdateMaxDistance(b1, a0, a1, max_dist)); + // + // We do not want the short circuit behavior of ||. Suppress + // -Wbitwise-instead-of-logical errors by converting to int. + return (int{UpdateMaxDistance(a0, b0, b1, max_dist)} | + int{UpdateMaxDistance(a1, b0, b1, max_dist)} | + int{UpdateMaxDistance(b0, a0, a1, max_dist)} | + int{UpdateMaxDistance(b1, a0, a1, max_dist)}); } std::pair GetEdgePairClosestPoints( diff --git a/src/s2/s2lax_polygon_shape.cc b/src/s2/s2lax_polygon_shape.cc index 41befa88..e71476a4 100644 --- a/src/s2/s2lax_polygon_shape.cc +++ b/src/s2/s2lax_polygon_shape.cc @@ -33,7 +33,7 @@ namespace { template std::unique_ptr make_unique_for_overwrite(size_t n) { // We only need to support this one variant. - static_assert(std::is_array_v); + static_assert(std::is_array::value); return std::unique_ptr(new typename absl::remove_extent_t[n]); } } // namespace diff --git a/src/s2/s2lax_polygon_shape.h b/src/s2/s2lax_polygon_shape.h index fa843d25..08d8a64d 100644 --- a/src/s2/s2lax_polygon_shape.h +++ b/src/s2/s2lax_polygon_shape.h @@ -76,7 +76,10 @@ // (S2BooleanOperation, S2ClosestEdgeQuery, etc). class S2LaxPolygonShape : public S2Shape { public: - static constexpr TypeTag kTypeTag = 5; + // Define as enum so we don't have to declare storage. + // TODO(user, b/210097200): Use static constexpr when C++17 is allowed + // in opensource. + enum : TypeTag { kTypeTag = 5 }; // Constructs an empty polygon. S2LaxPolygonShape() : num_loops_(0), num_vertices_(0) {} @@ -149,7 +152,7 @@ class S2LaxPolygonShape : public S2Shape { // The loop that contained the edge returned by the previous call to the // edge() method. This is used as a hint to speed up edge location when // there are many loops. - mutable std::atomic prev_loop_ = 0; + mutable std::atomic prev_loop_{0}; int32 num_vertices_; std::unique_ptr vertices_; @@ -206,7 +209,7 @@ class EncodedS2LaxPolygonShape : public S2Shape { // The loop that contained the edge returned by the previous call to the // edge() method. This is used as a hint to speed up edge location when // there are many loops. - mutable std::atomic prev_loop_ = 0; + mutable std::atomic prev_loop_{0}; s2coding::EncodedS2PointVector vertices_; s2coding::EncodedUintVector loop_starts_; diff --git a/src/s2/s2lax_polygon_shape_test.cc b/src/s2/s2lax_polygon_shape_test.cc index ed573792..091a3655 100644 --- a/src/s2/s2lax_polygon_shape_test.cc +++ b/src/s2/s2lax_polygon_shape_test.cc @@ -248,7 +248,11 @@ TEST(S2LaxPolygonShape, ManyLoopPolygon) { } } std::shuffle(edges.begin(), edges.end(), std::mt19937_64()); - for (const auto [e, i, j] : edges) { + // TODO(user,b/210097200): Use structured bindings when we require + // C++17 in opensource. + for (const auto t : edges) { + int e, i, j; + std::tie(e, i, j) = t; 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/s2lax_polyline_shape.h b/src/s2/s2lax_polyline_shape.h index e97159fa..17f85086 100644 --- a/src/s2/s2lax_polyline_shape.h +++ b/src/s2/s2lax_polyline_shape.h @@ -37,7 +37,10 @@ // or use S2LaxClosedPolylineShape defined in s2_lax_loop_shape.h.) class S2LaxPolylineShape : public S2Shape { public: - static constexpr TypeTag kTypeTag = 4; + // Define as enum so we don't have to declare storage. + // TODO(user, b/210097200): Use static constexpr when C++17 is allowed + // in opensource. + enum : TypeTag { kTypeTag = 4 }; // Constructs an empty polyline. S2LaxPolylineShape() : num_vertices_(0) {} @@ -102,7 +105,10 @@ class S2LaxPolylineShape : public S2Shape { // into a large contiguous buffer that contains other encoded data as well. class EncodedS2LaxPolylineShape : public S2Shape { public: - static constexpr TypeTag kTypeTag = S2LaxPolylineShape::kTypeTag; + // Define as enum so we don't have to declare storage. + // TODO(user, b/210097200): Use static constexpr when C++17 is allowed + // in opensource. + enum : TypeTag { kTypeTag = S2LaxPolylineShape::kTypeTag }; // Constructs an uninitialized object; requires Init() to be called. EncodedS2LaxPolylineShape() {} diff --git a/src/s2/s2point_compression.cc b/src/s2/s2point_compression.cc index cb36f918..f94eed66 100644 --- a/src/s2/s2point_compression.cc +++ b/src/s2/s2point_compression.cc @@ -345,6 +345,8 @@ void S2EncodePointsCompressed(Span points, bool S2DecodePointsCompressed(Decoder* decoder, int level, Span points) { + S2_DCHECK_LE(level, S2::kMaxCellLevel); + Faces faces; if (!faces.Decode(points.size(), decoder)) { return false; diff --git a/src/s2/s2point_vector_shape.h b/src/s2/s2point_vector_shape.h index 3c33a2e2..f58c0e97 100644 --- a/src/s2/s2point_vector_shape.h +++ b/src/s2/s2point_vector_shape.h @@ -29,7 +29,10 @@ // This class is useful for adding a collection of points to an S2ShapeIndex. class S2PointVectorShape : public S2Shape { public: - static constexpr TypeTag kTypeTag = 3; + // Define as enum so we don't have to declare storage. + // TODO(user, b/210097200): Use static constexpr when C++17 is allowed + // in opensource. + enum : TypeTag { kTypeTag = 3 }; // Constructs an empty point vector. S2PointVectorShape() {} @@ -90,7 +93,10 @@ class S2PointVectorShape : public S2Shape { // into a large contiguous buffer that contains other encoded data as well. class EncodedS2PointVectorShape : public S2Shape { public: - static constexpr TypeTag kTypeTag = S2PointVectorShape::kTypeTag; + // Define as enum so we don't have to declare storage. + // TODO(user, b/210097200): Use static constexpr when C++17 is allowed + // in opensource. + enum : TypeTag { kTypeTag = S2PointVectorShape::kTypeTag }; // Constructs an uninitialized object; requires Init() to be called. EncodedS2PointVectorShape() {} diff --git a/src/s2/s2polygon.h b/src/s2/s2polygon.h index bed41e5f..87475547 100644 --- a/src/s2/s2polygon.h +++ b/src/s2/s2polygon.h @@ -718,7 +718,10 @@ class S2Polygon final : public S2Region { // such that the polygon interior is always on the left. class Shape : public S2Shape { public: - static constexpr TypeTag kTypeTag = 1; + // Define as enum so we don't have to declare storage. + // TODO(user, b/210097200): Use static constexpr when C++17 is + // allowed in opensource. + enum : TypeTag { kTypeTag = 1 }; Shape() : polygon_(nullptr), loop_starts_(nullptr) {} ~Shape() override; @@ -759,7 +762,7 @@ class S2Polygon final : public S2Region { // edge() method. This is used as a hint to speed up edge location when // there are many loops. Note that this field does not take up any space // due to field packing with S2Shape::id_. - mutable std::atomic prev_loop_ = 0; + mutable std::atomic prev_loop_{0}; const S2Polygon* polygon_; diff --git a/src/s2/s2polyline.cc b/src/s2/s2polyline.cc index 472da7a3..8a7bc073 100644 --- a/src/s2/s2polyline.cc +++ b/src/s2/s2polyline.cc @@ -41,6 +41,7 @@ #include "s2/s2edge_distances.h" #include "s2/s2error.h" #include "s2/s2latlng_rect_bounder.h" +#include "s2/s2point.h" #include "s2/s2point_compression.h" #include "s2/s2pointutil.h" #include "s2/s2polyline_measures.h" @@ -431,8 +432,11 @@ bool S2Polyline::DecodeUncompressed(Decoder* const decoder) { return false; } num_vertices_ = decoder->get32(); + + // Check the bytes available before allocating memory in case of + // corrupt/malicious input. + if (decoder->avail() < num_vertices_ * sizeof(S2Point)) return false; vertices_.reset(new S2Point[num_vertices_]); - if (decoder->avail() < num_vertices_ * sizeof(vertices_[0])) return false; decoder->getn(&vertices_[0], num_vertices_ * sizeof(vertices_[0])); if (absl::GetFlag(FLAGS_s2debug) && s2debug_override_ == S2Debug::ALLOW) { @@ -505,6 +509,8 @@ void S2Polyline::EncodeCompressed(Encoder* encoder, bool S2Polyline::DecodeCompressed(Decoder* decoder) { if (decoder->avail() < sizeof(uint8)) return false; const int snap_level = decoder->get8(); + if (snap_level > S2::kMaxCellLevel) return false; + vector points; uint32 num_vertices; if (!decoder->get_varint32(&num_vertices)) return false; @@ -513,6 +519,10 @@ bool S2Polyline::DecodeCompressed(Decoder* decoder) { Init(points); return true; } + + // TODO(b/209937354): Prevent large allocations like in DecodeUncompressed. + // This is more complicated due to the compressed encoding, but perhaps the + // minimum required size can be bounded. points.resize(num_vertices); if (!S2DecodePointsCompressed(decoder, snap_level, absl::MakeSpan(points))) { return false; diff --git a/src/s2/s2polyline.h b/src/s2/s2polyline.h index 304d49ef..a647489e 100644 --- a/src/s2/s2polyline.h +++ b/src/s2/s2polyline.h @@ -33,6 +33,7 @@ #include "s2/s2debug.h" #include "s2/s2error.h" #include "s2/s2latlng_rect.h" +#include "s2/s2point_span.h" #include "s2/s2region.h" #include "s2/s2shape.h" @@ -137,6 +138,11 @@ class S2Polyline final : public S2Region { return vertices_[k]; } + // Returns an S2PointSpan containing the polyline's vertices. + S2PointSpan vertices_span() const { + return S2PointSpan(vertices_.get(), num_vertices_); + } + // Return the length of the polyline. S1Angle GetLength() const; @@ -319,7 +325,10 @@ class S2Polyline final : public S2Region { // data (see S2Shape for details). class Shape : public S2Shape { public: - static constexpr TypeTag kTypeTag = 2; + // Define as enum so we don't have to declare storage. + // TODO(user, b/210097200): Use static constexpr when C++17 is + // allowed in opensource. + enum : TypeTag { kTypeTag = 2 }; Shape() : polyline_(nullptr) {} // Must call Init(). diff --git a/src/s2/s2polyline_test.cc b/src/s2/s2polyline_test.cc index 877a7742..07535b53 100644 --- a/src/s2/s2polyline_test.cc +++ b/src/s2/s2polyline_test.cc @@ -32,6 +32,7 @@ #include "s2/s1angle.h" #include "s2/s2builderutil_snap_functions.h" #include "s2/s2cell.h" +#include "s2/s2coords.h" #include "s2/s2debug.h" #include "s2/s2latlng.h" #include "s2/s2pointutil.h" @@ -80,6 +81,8 @@ TEST(S2Polyline, Basic) { S2Point(0, 1, 0))); semi_equator.Reverse(); EXPECT_EQ(S2Point(1, 0, 0), semi_equator.vertex(2)); + ASSERT_EQ(3, semi_equator.vertices_span().size()); + EXPECT_EQ(S2Point(1, 0, 0), semi_equator.vertices_span()[2]); } TEST(S2Polyline, NoData) { @@ -536,6 +539,30 @@ TEST(S2Polyline, DecodeCompressedBadData) { EXPECT_FALSE(decoded_polyline.Decode(&decoder)); } +TEST(S2Polyline, DecodeCompressedMaxCellLevel) { + Encoder encoder; + encoder.Ensure(6); + encoder.put8(2); // kCurrentCompressedEncodingVersionNumber + encoder.put8(S2::kMaxCellLevel); + encoder.put32(0); + Decoder decoder(encoder.base(), encoder.length()); + + S2Polyline polyline; + EXPECT_TRUE(polyline.Decode(&decoder)); +} + +TEST(S2Polyline, DecodeCompressedCellLevelTooHigh) { + Encoder encoder; + encoder.Ensure(6); + encoder.put8(2); // kCurrentCompressedEncodingVersionNumber + encoder.put8(S2::kMaxCellLevel + 1); + encoder.put32(0); + Decoder decoder(encoder.base(), encoder.length()); + + S2Polyline polyline; + EXPECT_FALSE(polyline.Decode(&decoder)); +} + TEST(S2PolylineShape, Basic) { unique_ptr polyline(MakePolyline("0:0, 1:0, 1:1, 2:1")); S2Polyline::Shape shape(polyline.get()); diff --git a/src/s2/s2region_coverer_test.cc b/src/s2/s2region_coverer_test.cc index a0a36e87..8d0d1f7d 100644 --- a/src/s2/s2region_coverer_test.cc +++ b/src/s2/s2region_coverer_test.cc @@ -105,7 +105,10 @@ static void CheckCovering(const S2RegionCoverer::Options& options, if (covering.size() > options.max_cells()) { // If the covering has more than the requested number of cells, then check // that the cell count cannot be reduced by using the parent of some cell. - for (const auto [_, cells] : min_level_cells) { + // TODO(user,b/210097200): Use structured bindings when we require + // C++17 in opensource. + for (const auto& p : min_level_cells) { + const int cells = p.second; EXPECT_EQ(cells, 1); } } diff --git a/src/s2/s2shape_index_region.h b/src/s2/s2shape_index_region.h index 7b90e183..eee0dacd 100644 --- a/src/s2/s2shape_index_region.h +++ b/src/s2/s2shape_index_region.h @@ -360,7 +360,11 @@ bool S2ShapeIndexRegion::VisitIntersectingShapes( clipped.num_edges() > 0 || !clipped.contains_center(); } } - for (const auto& [shape_id, not_contains] : shape_not_contains) { + // TODO(user,b/210097200): Use structured bindings when we require + // C++17 in opensource. + for (const auto& p : shape_not_contains) { + const int shape_id = p.first; + const bool not_contains = p.second; if (!visitor(index().shape(shape_id), !not_contains)) return false; } return true; diff --git a/src/s2/thread_testing.cc b/src/s2/thread_testing.cc index 4df547f7..472e3843 100644 --- a/src/s2/thread_testing.cc +++ b/src/s2/thread_testing.cc @@ -17,6 +17,7 @@ #include "s2/thread_testing.h" +#include #include #include diff --git a/src/s2/util/coding/coder.cc b/src/s2/util/coding/coder.cc index ab5841f0..1a61edc2 100644 --- a/src/s2/util/coding/coder.cc +++ b/src/s2/util/coding/coder.cc @@ -24,23 +24,25 @@ #include #include +#include "absl/utility/utility.h" + #include "s2/base/integral_types.h" #include "s2/base/logging.h" #include "s2/base/port.h" Encoder::Encoder(Encoder&& other) - : buf_(std::exchange(other.buf_, nullptr)), - limit_(std::exchange(other.limit_, nullptr)), - underlying_buffer_(std::exchange(other.underlying_buffer_, nullptr)), - orig_(std::exchange(other.orig_, nullptr)) {} + : buf_(absl::exchange(other.buf_, nullptr)), + limit_(absl::exchange(other.limit_, nullptr)), + underlying_buffer_(absl::exchange(other.underlying_buffer_, nullptr)), + orig_(absl::exchange(other.orig_, nullptr)) {} Encoder& Encoder::operator=(Encoder&& other) { if (this == &other) return *this; if (ensure_allowed()) DeleteBuffer(underlying_buffer_, capacity()); - buf_ = std::exchange(other.buf_, nullptr); - limit_ = std::exchange(other.limit_, nullptr); - underlying_buffer_ = std::exchange(other.underlying_buffer_, nullptr); - orig_ = std::exchange(other.orig_, nullptr); + buf_ = absl::exchange(other.buf_, nullptr); + limit_ = absl::exchange(other.limit_, nullptr); + underlying_buffer_ = absl::exchange(other.underlying_buffer_, nullptr); + orig_ = absl::exchange(other.orig_, nullptr); return *this; } @@ -69,7 +71,11 @@ void Encoder::EnsureSlowPath(size_t N) { // Double buffer size, but make sure we always have at least N extra bytes const size_t current_len = length(); - const auto [new_buffer, new_capacity] = + // Used in opensource; avoid structured bindings (a C++17 feature) to + // remain C++11-compatible. b/210097200 + unsigned char* new_buffer; + size_t new_capacity; + std::tie(new_buffer, new_capacity) = NewBuffer(std::max(current_len + N, 2 * current_len)); if (underlying_buffer_) { diff --git a/src/s2/util/coding/coder.h b/src/s2/util/coding/coder.h index 89b2c021..f6556774 100644 --- a/src/s2/util/coding/coder.h +++ b/src/s2/util/coding/coder.h @@ -30,6 +30,7 @@ #include "absl/base/macros.h" #include "absl/meta/type_traits.h" #include "absl/numeric/int128.h" +#include "absl/utility/utility.h" #include "s2/base/casts.h" #include "s2/base/integral_types.h" @@ -142,10 +143,13 @@ class Encoder { private: // All encoding operations are done through the Writer. This avoids aliasing // between `buf_` and `this` which allows the compiler to avoid reloading - // `buf_` repeatedly. See https://godbolt.org/z/sfEnePYWP. + // `buf_` repeatedly. See https://godbolt.org/z/zM36s3ded. struct Writer { Encoder* enc; - char* p{reinterpret_cast(enc->buf_)}; + char* p; + + explicit Writer(Encoder* e) + : enc(e), p(reinterpret_cast(enc->buf_)) {} ~Writer() { enc->buf_ = reinterpret_cast(p); @@ -153,7 +157,7 @@ class Encoder { S2_DCHECK_LE(enc->buf_, enc->limit_); } - char* skip(ptrdiff_t N) { return std::exchange(p, p + N); } + char* skip(ptrdiff_t N) { return absl::exchange(p, p + N); } void put8(unsigned char v) { *p++ = v; } void put16(uint16 v) { LittleEndian::Store16(skip(2), v); } @@ -182,7 +186,7 @@ class Encoder { } }; - Writer writer() { return Writer{this}; } + Writer writer() { return Writer(this); } static std::pair NewBuffer(size_t size); static void DeleteBuffer(unsigned char* buf, size_t size); diff --git a/src/s2/util/math/exactfloat/exactfloat.cc b/src/s2/util/math/exactfloat/exactfloat.cc index 51f4426b..e0e9471f 100644 --- a/src/s2/util/math/exactfloat/exactfloat.cc +++ b/src/s2/util/math/exactfloat/exactfloat.cc @@ -419,7 +419,7 @@ std::string ExactFloat::ToStringWithMaxDigits(int max_digits) const { // Use fixed format. We split this into two cases depending on whether // the integer portion is non-zero or not. if (exp10 > 0) { - if (exp10 >= digits.size()) { + if (static_cast(exp10) >= digits.size()) { str += digits; for (int i = exp10 - digits.size(); i > 0; --i) { str.push_back('0'); diff --git a/src/s2/util/math/matrix3x3.h b/src/s2/util/math/matrix3x3.h index c013a61e..24282503 100644 --- a/src/s2/util/math/matrix3x3.h +++ b/src/s2/util/math/matrix3x3.h @@ -33,7 +33,7 @@ #include #include -#include +#include #include "s2/base/logging.h" #include "s2/util/math/mathutil.h" diff --git a/src/s2/util/math/vector.h b/src/s2/util/math/vector.h index 5a5eb915..fb214229 100644 --- a/src/s2/util/math/vector.h +++ b/src/s2/util/math/vector.h @@ -28,6 +28,7 @@ #include #include // NOLINT(readability/streams) #include +#include #include #include "s2/base/integral_types.h" diff --git a/src/s2/value_lexicon_test.cc b/src/s2/value_lexicon_test.cc index 28c1d5fb..a6d29d2a 100644 --- a/src/s2/value_lexicon_test.cc +++ b/src/s2/value_lexicon_test.cc @@ -17,7 +17,9 @@ #include "s2/value_lexicon.h" +#include #include +#include #include #include "absl/memory/memory.h"