Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add at accessors with bounds checking#342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
eddelbuettel merged 14 commits intoRcppCore:masterfromfplazaonate:at-accessors-patch
Aug 25, 2015
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletionsChangeLog
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
2015-08-19 Florian Plaza Oñate <florian.plaza@jouy.inra.fr>
* 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 <edd@debian.org>

* inst/unitTests/runit.Module.client.package.R: Disabled for bad
Expand Down
7 changes: 7 additions & 0 deletionsinst/include/Rcpp/vector/Matrix.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 ) ;
}
Expand Down
27 changes: 17 additions & 10 deletionsinst/include/Rcpp/vector/Vector.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -278,26 +278,26 @@ class Vector :
/**
* offset based on the dimensions of this vector
*/
size_t offset(constsize_t& i, constsize_t& j) const {
R_xlen_t offset(constint& i, constint& j) const {
if( !::Rf_isMatrix(Storage::get__()) ) throw not_a_matrix() ;

/* we need to extract the dimensions */
int *dim = dims() ;
size_tnrow =static_cast<size_t>(dim[0]) ;
size_tncol =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 intnrow = dim[0] ;
const intncol = 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(constsize_t& i) const {
if( static_cast<R_xlen_t>(i) >= ::Rf_xlength(Storage::get__()) ) throw index_out_of_bounds() ;
R_xlen_t offset(constR_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();
Expand All@@ -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) ; }
Expand All@@ -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) ) ;
}
Expand Down
4 changes: 4 additions & 0 deletionsinst/unitTests/cpp/Matrix.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
}
5 changes: 5 additions & 0 deletionsinst/unitTests/cpp/Vector.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
}
12 changes: 12 additions & 0 deletionsinst/unitTests/runit.Matrix.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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" )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Am I missing something or should the msg be "index out of bounds" because the fact that we have an exception means yourthrow went into effect?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Actually, the error message is written if there is no error generated (c.f. RUnit documentation)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I guess you are correct -- if it passes we just getTRUE. But if you look at our code the msg field, when we set it all :), mostly just states where this came from. So "matrix bounds check" would be fine, and s/matrix/vector/ for the other one.

Not that important. I'll try merge this maybe late this evening if I get to it. It now needs a manual intervention because ChangeLog changed in the meantime ...

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" )
}

}
8 changes: 7 additions & 1 deletioninst/unitTests/runit.Vector.R
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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" )
}
}

[8]ページ先頭
©2009-2025 Movatter.jp