Fix missing value propagation in as_integers/doubles() (#265)

* Correctly propagate missing values in `as_integers()`

Also:
- Switch to `R_xlen_t` over `size_t` since this is what `size()` returns and is what our constructors prefer
- Initialize return value with `ret(len)` since that seems cleaner
- Forward declare required bits from `doubles.hpp`. Use of `is_na(<dbl>)` is new, so it makes sense that we need to forward declare it. Even before this PR, we used the `[]` double operator, so you might be wondering why we need the forward declaration. Well, it only previously worked by luck because we had a hidden dependency on `cpp11/doubles.hpp` in `test-integers.cpp` by including `cpp11/doubles.hpp` before `cpp11/integers.hpp`. I swapped them around in the test file and it indeed failed without this forward declaration.

* Correctly propagate missing values in `as_doubles()`

Also:
- Switch to `R_xlen_t` over `size_t` since this is what `size()` returns and is what our constructors prefer
- Initialize sized return value, rather than using `push_back()`, which should be faster
- Forward declare required bits from `integers.hpp`. There is no `is_na(<int>)` specialization to forward declare, but there is a `na<int>()` forward declaration that is required, because that is used by the default `is_na()` implementation
- Move double specializations of `na()` and `is_na()` before the implementation of `as_doubles()`. Since `as_doubles()` now calls the `na<double>()` specialization, it has to be implemented (or forward declared) ahead of time.

* NEWS bullet

* Guess at how to make the clang formatter happy

* Separate `cpp11/integers.hpp` into its own include "block"

* Typo, `convertable` -> `convertible`

---------

Co-authored-by: Romain François <romain@rstudio.com>

Author: DavisVaughan

NEWS.md

@@ -1,5 +1,8 @@
 # cpp11 (development version)
 
+* `as_doubles()` and `as_integers()` now propagate missing values correctly
+  (#265).
+
 * Fixed a performance issue related to nested `unwind_protect()` calls (#298).
 
 * Minor performance improvements to the cpp11 protect code. (@kevinushey)

cpp11test/src/test-doubles.cpp

@@ -393,6 +393,11 @@ context("doubles-C++") {
     e.push_back("a");
     e.push_back("b");
     expect_error(cpp11::as_doubles(e));
+
+    cpp11::writable::integers na;
+    na.push_back(cpp11::na<int>());
+    cpp11::doubles na2(cpp11::as_doubles(na));
+    expect_true(cpp11::is_na(na2[0]));
   }
 
   test_that("doubles operator[] and at") {

cpp11test/src/test-integers.cpp

@@ -1,7 +1,9 @@
 #include "Rversion.h"
-#include "cpp11/doubles.hpp"
+
 #include "cpp11/function.hpp"
 #include "cpp11/integers.hpp"
+
+#include "cpp11/doubles.hpp"
 #include "cpp11/strings.hpp"
 
 #include <testthat.h>
@@ -36,6 +38,11 @@ context("integers-C++") {
     expect_true(t[2] == 100000);
     expect_true(t[3] == 100000);
     expect_true(TYPEOF(t) == INTSXP);
+
+    cpp11::writable::doubles na;
+    na.push_back(cpp11::na<double>());
+    cpp11::integers na2(cpp11::as_integers(na));
+    expect_true(cpp11::is_na(na2[0]));
   }
 
   test_that("integers.push_back()") {

inst/include/cpp11/as.hpp

@@ -73,7 +73,7 @@ using enable_if_c_string = enable_if_t<std::is_same<T, const char*>::value, R>;
 
 // https://stackoverflow.com/a/1521682/2055486
 //
-inline bool is_convertable_without_loss_to_integer(double value) {
+inline bool is_convertible_without_loss_to_integer(double value) {
   double int_part;
   return std::modf(value, &int_part) == 0.0;
 }
@@ -100,7 +100,7 @@ enable_if_integral<T, T> as_cpp(SEXP from) {
         return NA_INTEGER;
       }
       double value = REAL_ELT(from, 0);
-      if (is_convertable_without_loss_to_integer(value)) {
+      if (is_convertible_without_loss_to_integer(value)) {
         return value;
       }
     }

inst/include/cpp11/doubles.hpp

@@ -135,33 +135,44 @@ typedef r_vector<double> doubles;
 
 }  // namespace writable
 
+template <>
+inline double na() {
+  return NA_REAL;
+}
+
+template <>
+inline bool is_na(const double& x) {
+  return ISNA(x);
+}
+
+// forward declarations
 typedef r_vector<int> integers;
 
+template <>
+int na();
+
+template <>
+int r_vector<int>::operator[](const R_xlen_t pos) const;
+
 inline doubles as_doubles(sexp x) {
   if (TYPEOF(x) == REALSXP) {
     return as_cpp<doubles>(x);
-  }
-
-  else if (TYPEOF(x) == INTSXP) {
+  } else if (TYPEOF(x) == INTSXP) {
     integers xn = as_cpp<integers>(x);
-    size_t len = xn.size();
-    writable::doubles ret;
-    for (size_t i = 0; i < len; ++i) {
-      ret.push_back(static_cast<double>(xn[i]));
+    R_xlen_t len = xn.size();
+    writable::doubles ret(len);
+    for (R_xlen_t i = 0; i < len; ++i) {
+      int el = xn[i];
+      if (is_na(el)) {
+        ret[i] = na<double>();
+      } else {
+        ret[i] = static_cast<double>(el);
+      }
     }
     return ret;
   }
 
   throw type_error(REALSXP, TYPEOF(x));
 }
 
-template <>
-inline double na() {
-  return NA_REAL;
-}
-
-template <>
-inline bool is_na(const double& x) {
-  return ISNA(x);
-}
 }  // namespace cpp11

inst/include/cpp11/integers.hpp

@@ -145,25 +145,32 @@ inline int na() {
   return NA_INTEGER;
 }
 
-// forward declaration
-
+// forward declarations
 typedef r_vector<double> doubles;
 
+template <>
+bool is_na(const double& x);
+
+template <>
+double r_vector<double>::operator[](const R_xlen_t pos) const;
+
 inline integers as_integers(sexp x) {
   if (TYPEOF(x) == INTSXP) {
     return as_cpp<integers>(x);
   } else if (TYPEOF(x) == REALSXP) {
     doubles xn = as_cpp<doubles>(x);
-    size_t len = (xn.size());
-    writable::integers ret = writable::integers(len);
-    for (size_t i = 0; i < len; ++i) {
+    R_xlen_t len = xn.size();
+    writable::integers ret(len);
+    for (R_xlen_t i = 0; i < len; ++i) {
       double el = xn[i];
-      if (!is_convertable_without_loss_to_integer(el)) {
+      if (is_na(el)) {
+        ret[i] = na<int>();
+      } else if (is_convertible_without_loss_to_integer(el)) {
+        ret[i] = static_cast<int>(el);
+      } else {
         throw std::runtime_error("All elements must be integer-like");
       }
-      ret[i] = (static_cast<int>(el));
     }
-
     return ret;
   }