Merge branch 'feature/pr342-at-accessors'

Author: eddelbuettel

ChangeLog

@@ -1,3 +1,12 @@
+2015-08-19  Florian Plaza Oñate <[email protected]>
+        * inst/include/Rcpp/vector/Vector.h: Add 'at' methods which implement 
+        accessors with bounds cheking (pull request #342, fixes issue #341)
+        * inst/include/Rcpp/vector/Matrix.h: Ditto
+        * inst/unitTests/cpp/Vector.cpp: Add unit tests for at acessors
+        * inst/unitTests/cpp/Matrix.cpp: Ditto
+        * inst/unitTests/runit.Vector.R: Ditto
+        * inst/unitTests/runit.Matrix.R: Ditto
+
 2015-08-18  Dirk Eddelbuettel  <[email protected]>
 
         * inst/unitTests/runit.Module.client.package.R: Disabled for bad

inst/include/Rcpp/vector/Matrix.h

@@ -135,6 +135,13 @@ class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, tru
        return static_cast< const Vector<RTYPE>* >( this )->operator[]( offset( i, j ) ) ;
     }
 
+    inline Proxy at( const size_t& i, const size_t& j) {
+        return static_cast< Vector<RTYPE>* >( this )->operator()( i, j ) ;
+    }
+    inline const_Proxy at( const size_t& i, const size_t& j) const {
+        return static_cast< const Vector<RTYPE>* >( this )->operator()( i, j ) ;
+    }
+
     inline Row operator()( int i, internal::NamedPlaceHolder ) {
       return Row( *this, i ) ;
     }

inst/include/Rcpp/vector/Vector.h

@@ -278,26 +278,26 @@ class Vector :
     /**
      * offset based on the dimensions of this vector
      */
-    size_t offset(const size_t& i, const size_t& j) const {
+    R_xlen_t offset(const int& i, const int& j) const {
         if( !::Rf_isMatrix(Storage::get__()) ) throw not_a_matrix() ;
 
         /* we need to extract the dimensions */
-        int *dim = dims() ;
-        size_t nrow = static_cast<size_t>(dim[0]) ;
-        size_t ncol = static_cast<size_t>(dim[1]) ;
-        if( i >= nrow || j >= ncol ) throw index_out_of_bounds() ;
-        return i + nrow*j ;
+        const int* dim = dims() ;
+        const int nrow = dim[0] ;
+        const int ncol = dim[1] ;
+        if(i < 0|| i >= nrow || j < 0 || j >= ncol ) throw index_out_of_bounds() ;
+        return i + static_cast<R_xlen_t>(nrow)*j ;
     }
 
     /**
      * one dimensional offset doing bounds checking to ensure
      * it is valid
      */
-    size_t offset(const size_t& i) const {
-        if( static_cast<R_xlen_t>(i) >= ::Rf_xlength(Storage::get__()) ) throw index_out_of_bounds() ;
+    R_xlen_t offset(const R_xlen_t& i) const {
+        if(i < 0 || i >= ::Rf_xlength(Storage::get__()) ) throw index_out_of_bounds() ;
         return i ;
     }
-
+    
     R_xlen_t offset(const std::string& name) const {
         SEXP names = RCPP_GET_NAMES( Storage::get__() ) ;
         if( Rf_isNull(names) ) throw index_out_of_bounds();
@@ -318,7 +318,7 @@ class Vector :
 
     inline iterator begin() { return cache.get() ; }
     inline iterator end() { return cache.get() + size() ; }
-	inline const_iterator begin() const{ return cache.get_const() ; }
+    inline const_iterator begin() const{ return cache.get_const() ; }
     inline const_iterator end() const{ return cache.get_const() + size() ; }
 
     inline Proxy operator[]( R_xlen_t i ){ return cache.ref(i) ; }
@@ -331,6 +331,13 @@ class Vector :
         return cache.ref( offset(i) ) ;
     }
 
+    inline Proxy at( const size_t& i) {
+       return cache.ref( offset(i) ) ;
+    }
+    inline const_Proxy at( const size_t& i) const {
+       return cache.ref( offset(i) ) ;
+    }
+
     inline Proxy operator()( const size_t& i, const size_t& j) {
         return cache.ref( offset(i,j) ) ;
     }

inst/unitTests/cpp/Matrix.cpp

@@ -238,3 +238,7 @@ NumericVector runit_const_Matrix_column( const NumericMatrix& m ){
    return col1 ;
 }
 
+// [[Rcpp::export]]
+int mat_access_with_bounds_checking(const IntegerMatrix m, int i, int j) {
+    return m.at(i, j);
+}

inst/unitTests/cpp/Vector.cpp

@@ -777,3 +777,8 @@ int noprotect_vector( Vector<REALSXP, NoProtectStorage> x){
 int noprotect_matrix( Matrix<REALSXP, NoProtectStorage> x){
   return x.nrow() ;
 }
+
+// [[Rcpp::export]]
+int vec_access_with_bounds_checking(const IntegerVector x, int index) {
+    return x.at(index);
+}

inst/unitTests/runit.Matrix.R

@@ -171,4 +171,16 @@ if (.runThisTest) {
       checkEquals( m[,1], m[,2] )
     }
 
+    test.IntegerMatrix.accessor.with.bounds.checking <- function() {
+        m <- matrix(seq(1L, 12, by=1L), nrow=4L, ncol=3L)
+        checkEquals(mat_access_with_bounds_checking(m, 0, 0), 1)
+        checkEquals(mat_access_with_bounds_checking(m, 1, 2), 10)
+        checkEquals(mat_access_with_bounds_checking(m, 3, 2), 12)
+        checkException(mat_access_with_bounds_checking(m, 4, 2) , msg = "index out of bounds not detected" )
+        checkException(mat_access_with_bounds_checking(m, 3, 3) , msg = "index out of bounds not detected" )
+        checkException(mat_access_with_bounds_checking(m, 3, -1) , msg = "index out of bounds not detected" )
+        checkException(mat_access_with_bounds_checking(m, -1, 2) , msg = "index out of bounds not detected" )
+        checkException(mat_access_with_bounds_checking(m, -1, -1) , msg = "index out of bounds not detected" )
+    }
+
 }

inst/unitTests/runit.Vector.R

@@ -683,6 +683,12 @@ if (.runThisTest) {
         x <- matrix(rnorm(10), nrow=2)
         checkIdentical( noprotect_matrix(x), 2L )
     }
-    
+
+    test.IntegerVector.accessor.with.bounds.checking <- function() {
+        x <- seq(1L, 5L, by=1L)
+        checkEquals(vec_access_with_bounds_checking(x, 3), 4)
+        checkException(vec_access_with_bounds_checking(x, 5) , msg = "index out of bounds not detected" )
+        checkException(vec_access_with_bounds_checking(x, -1) , msg = "index out of bounds not detected" )
+    }
 }