Merge pull request #80 from r-spatial/fix-nested-anon

Fix nested anon type/union warning

Author: paleolimbot

R/s2-xptr.R

@@ -54,7 +54,11 @@ validate_s2_xptr <- function(x) {
 
 #' @export
 rep.s2_xptr <- function(x, ...) {
-  new_s2_xptr(NextMethod(), class(x))
+  if (length(x) == 0) {
+    new_s2_xptr(list(), class(x))
+  } else {
+    new_s2_xptr(NextMethod(), class(x))
+  }
 }
 
 #' @method rep_len s2_xptr

data-raw/update-s2.R

@@ -102,6 +102,12 @@ print_next <- function() {
     "Replace calls to log(<int literal>), sqrt(<int literal>), and ldexp(<int literal>, ...) ",
     "with an explicit doouble (e.g., sqrt(3) -> sqrt(3.0) to fix build errors on CRAN Solaris"
   )
+  cli::cat_bullet(
+    "Ensure the code compiles on clang with -Wnested-anon-types. ",
+    "These errors can be fixed by declaring the anonymous types just above the union. ",
+    "(e.g., encoded_s2point_vector.h:91)"
+  )
+
   cli::cat_bullet("Replace `abort()` with `cpp_compat_abort()`")
   cli::cat_bullet("Replace `cerr`/`cout` with `cpp_compat_cerr`/`cpp_compat_cout`")
   cli::cat_bullet("Replace `srandom()` with `cpp_compat_srandom()`")

inst/include/s2/encoded_s2point_vector.h

@@ -90,26 +90,35 @@ class EncodedS2PointVector {
   // TODO(ericv): Once additional formats have been implemented, consider
   // using std::variant<> instead.  It's unclear whether this would have
   // better or worse performance than the current approach.
+
+  // dd: These structs are anonymous in the upstream S2 code; however,
+  // this generates CMD-check failure due to the [-Wnested-anon-types]
+  // (anonymous types declared in an anonymous union are an extension)
+  // The approach here just names the types.
+  struct CellIDStruct {
+    EncodedStringVector blocks;
+    uint64 base;
+    uint8 level;
+    bool have_exceptions;
+
+    // TODO(ericv): Use std::atomic_flag to cache the last point decoded in
+    // a thread-safe way.  This reduces benchmark times for actual polygon
+    // operations (e.g. S2ClosestEdgeQuery) by about 15%.
+  };
+
+  struct UncompressedStruct {
+    const S2Point* points;
+  };
+
   enum Format : uint8 {
     UNCOMPRESSED = 0,
     CELL_IDS = 1,
   };
   Format format_;
   uint32 size_;
   union {
-    struct {
-      const S2Point* points;
-    } uncompressed_;
-    struct {
-      EncodedStringVector blocks;
-      uint64 base;
-      uint8 level;
-      bool have_exceptions;
-
-      // TODO(ericv): Use std::atomic_flag to cache the last point decoded in
-      // a thread-safe way.  This reduces benchmark times for actual polygon
-      // operations (e.g. S2ClosestEdgeQuery) by about 15%.
-    } cell_ids_;
+    struct UncompressedStruct uncompressed_;
+    struct CellIDStruct cell_ids_;
   };
 };
 

tests/testthat/test-s2-xptr.R

@@ -20,6 +20,8 @@ test_that("s2_xptr validation works", {
 })
 
 test_that("s2_xptr subsetting and concatenation work", {
+  expect_length(rep(new_s2_xptr(list()), 10), 0)
+
   xptr <- new_s2_xptr(list(NULL, NULL))
   expect_identical(xptr[1], new_s2_xptr(list(NULL)))
   expect_identical(xptr[[1]], xptr[1])