Merge pull request #920 from nolanw123/master

* Changes to fix Subsetter's use of 'int' for indexing

Author: eddelbuettel

ChangeLog

@@ -1,3 +1,8 @@
+2018-11-09  William Nolan  <[email protected]>
+
+        * inst/include/Rcpp/vector/Subsetter.h: Fixed to use R_xlen_t instead of int for indexing
+        and added warning when indexing by IntegerVector (only for large enough vectors)
+
 2018-11-05  Dirk Eddelbuettel  <[email protected]>
 
         * DESCRIPTION (Date, Version): Version 1.0, and happy 10th birthday!

inst/include/Rcpp/vector/Subsetter.h

@@ -22,6 +22,8 @@
 #ifndef Rcpp_vector_Subsetter_h_
 #define Rcpp_vector_Subsetter_h_
 
+#include <limits>
+
 namespace Rcpp {
 
 template <
@@ -51,14 +53,14 @@ class SubsetProxy {
     // Enable e.g. x[y] = z
     template <int OtherRTYPE, template <class> class OtherStoragePolicy>
     SubsetProxy& operator=(const Vector<OtherRTYPE, OtherStoragePolicy>& other) {
-        int n = other.size();
+        R_xlen_t n = other.size();
 
         if (n == 1) {
-            for (int i=0; i < indices_n; ++i) {
+            for (R_xlen_t i=0; i < indices_n; ++i) {
                 lhs[ indices[i] ] = other[0];
             }
         } else if (n == indices_n) {
-            for (int i=0; i < n; ++i) {
+            for (R_xlen_t i=0; i < n; ++i) {
                 lhs[ indices[i] ] = other[i];
             }
         } else {
@@ -70,28 +72,28 @@ class SubsetProxy {
     // Enable e.g. x[y] = 1;
     // TODO: std::enable_if<primitive> with C++11
     SubsetProxy& operator=(double other) {
-        for (int i=0; i < indices_n; ++i) {
+        for (R_xlen_t i=0; i < indices_n; ++i) {
             lhs[ indices[i] ] = other;
         }
         return *this;
     }
 
     SubsetProxy& operator=(int other) {
-        for (int i=0; i < indices_n; ++i) {
+        for (R_xlen_t i=0; i < indices_n; ++i) {
             lhs[ indices[i] ] = other;
         }
         return *this;
     }
 
     SubsetProxy& operator=(const char* other) {
-        for (int i=0; i < indices_n; ++i) {
+        for (R_xlen_t i=0; i < indices_n; ++i) {
             lhs[ indices[i] ] = other;
         }
         return *this;
     }
 
     SubsetProxy& operator=(bool other) {
-        for (int i=0; i < indices_n; ++i) {
+        for (R_xlen_t i=0; i < indices_n; ++i) {
             lhs[ indices[i] ] = other;
         }
         return *this;
@@ -107,12 +109,12 @@ class SubsetProxy {
 
     SubsetProxy& operator=(const SubsetProxy& other) {
         if (other.indices_n == 1) {
-            for (int i=0; i < indices_n; ++i) {
+            for (R_xlen_t i=0; i < indices_n; ++i) {
                 lhs[ indices[i] ] = other.lhs[other.indices[0]];
             }
         }
         else if (indices_n == other.indices_n) {
-            for (int i=0; i < indices_n; ++i)
+            for (R_xlen_t i=0; i < indices_n; ++i)
                 lhs[ indices[i] ] = other.lhs[other.indices[i]];
             }
         else {
@@ -132,34 +134,40 @@ class SubsetProxy {
 private:
 
     #ifndef RCPP_NO_BOUNDS_CHECK
-    void check_indices(int* x, int n, int size) {
-        for (int i=0; i < n; ++i) {
+    template <typename IDX>
+    void check_indices(IDX* x, R_xlen_t n, R_xlen_t size) {
+        for (IDX i=0; i < n; ++i) {
             if (x[i] < 0 or x[i] >= size) {
+                if(std::numeric_limits<IDX>::is_integer && size > std::numeric_limits<IDX>::max()) {
+                    stop("use NumericVector to index an object of this size");
+                }
                 stop("index error");
             }
         }
     }
     #else
-    void check_indices(int* x, int n, int size) {}
+    template <typename IDX>
+    void check_indices(IDX* x, IDX n, IDX size) {}
     #endif
 
     void get_indices( traits::identity< traits::int2type<INTSXP> > t ) {
         indices.reserve(rhs_n);
-        int* ptr = INTEGER(rhs);
+        int* ptr = INTEGER(rhs); // ok to use int * here, we'll catch any problems inside check_indices
         check_indices(ptr, rhs_n, lhs_n);
-        for (int i=0; i < rhs_n; ++i) {
+        for (R_xlen_t i=0; i < rhs_n; ++i) {
             indices.push_back( rhs[i] );
         }
         indices_n = rhs_n;        
     }
 
     void get_indices( traits::identity< traits::int2type<REALSXP> > t ) {
         indices.reserve(rhs_n);
-        Vector<INTSXP, StoragePolicy> tmp =
-            as< Vector<INTSXP, StoragePolicy> >(rhs);
-        int* ptr = INTEGER(tmp);
-        check_indices(ptr, rhs_n, lhs_n);
-        for (int i=0; i < rhs_n; ++i) {
+        std::vector<R_xlen_t> tmp(rhs.size()); // create temp R_xlen_t type indices from reals
+        for(size_t i = 0 ; i < tmp.size() ; ++i) {
+            tmp[i] = rhs[i];
+        }
+        check_indices(&tmp[0], rhs_n, lhs_n);
+        for (R_xlen_t i=0; i < rhs_n; ++i) {
             indices.push_back( tmp[i] );
         }
         indices_n = rhs_n;
@@ -171,7 +179,7 @@ class SubsetProxy {
         if (Rf_isNull(names)) stop("names is null");
         SEXP* namesPtr = STRING_PTR(names);
         SEXP* rhsPtr = STRING_PTR(rhs);
-        for (int i = 0; i < rhs_n; ++i) {
+        for (R_xlen_t i = 0; i < rhs_n; ++i) {
             SEXP* match = std::find(namesPtr, namesPtr + lhs_n, *(rhsPtr + i));
             if (match == namesPtr + lhs_n)
                 stop("not found");
@@ -186,7 +194,7 @@ class SubsetProxy {
             stop("logical subsetting requires vectors of identical size");
         }
         int* ptr = LOGICAL(rhs);
-        for (int i=0; i < rhs_n; ++i) {
+        for (R_xlen_t i=0; i < rhs_n; ++i) {
             if (ptr[i] == NA_INTEGER) {
                 stop("can't subset using a logical vector with NAs");
             }
@@ -199,13 +207,13 @@ class SubsetProxy {
 
     Vector<RTYPE, StoragePolicy> get_vec() const {
         Vector<RTYPE, StoragePolicy> output = no_init(indices_n);
-        for (int i=0; i < indices_n; ++i) {
+        for (R_xlen_t i=0; i < indices_n; ++i) {
             output[i] = lhs[ indices[i] ];
         }
         SEXP names = Rf_getAttrib(lhs, R_NamesSymbol);
         if (!Rf_isNull(names)) {
             Shield<SEXP> out_names( Rf_allocVector(STRSXP, indices_n) );
-            for (int i=0; i < indices_n; ++i) {
+            for (R_xlen_t i=0; i < indices_n; ++i) {
                 SET_STRING_ELT(out_names, i, STRING_ELT(names, indices[i]));
             }
             Rf_setAttrib(output, R_NamesSymbol, out_names);
@@ -216,13 +224,13 @@ class SubsetProxy {
 
     LHS_t& lhs;
     const RHS_t& rhs;
-    int lhs_n;
-    int rhs_n;
+    R_xlen_t lhs_n;
+    R_xlen_t rhs_n;
 
-    std::vector<int> indices;
+    std::vector<R_xlen_t> indices;
 
     // because of the above, we keep track of the size
-    int indices_n;
+    R_xlen_t indices_n;
 
 public:
 
@@ -234,10 +242,10 @@ class SubsetProxy {
                           RHS_NA_OTHER, RHS_T_OTHER>& other) {                        \
         Vector<RTYPE, StoragePolicy> result(indices_n);                               \
         if (other.indices_n == 1) {                                                   \
-            for (int i = 0; i < indices_n; ++i)                                       \
+            for (R_xlen_t i = 0; i < indices_n; ++i)                                  \
                 result[i] = lhs[indices[i]] __OPERATOR__ other.lhs[other.indices[0]]; \
         } else if (indices_n == other.indices_n) {                                    \
-            for (int i = 0; i < indices_n; ++i)                                       \
+            for (R_xlen_t i = 0; i < indices_n; ++i)                                  \
                 result[i] = lhs[indices[i]] __OPERATOR__ other.lhs[other.indices[i]]; \
         } else {                                                                      \
             stop("index error");                                                      \