Skip to content
Open
Show file tree
Hide file tree
Changes from all 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 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
2026-09-30 Kevin Ushey <kevinushey@gmail.com>

* inst/include/Rcpp/vector/traits.h: Move the out-of-bounds warning into
a cold, out-of-line helper; cache the vector length in proxy_cache
* inst/include/Rcpp/vector/Vector.h: Return the cached length from
size() and length() rather than calling Rf_xlength() each time
* inst/include/Rcpp/vector/Matrix.h: Cache the number of columns
* inst/include/Rcpp/vector/SubMatrix.h: Idem

2026-09-22 Iñaki Ucar <iucar@fedoraproject.org>

* inst/include/Rcpp/sugar/matrix/col.h: Fix Col constructor using ncol()
Expand Down
3 changes: 3 additions & 0 deletions inst/NEWS.Rd
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
warnings (Iñaki in \ghpr{1508} closing \ghit{1497})
\item The \code{col} constructor now uses \code{nrow} in an
initialization (Iñaki in \ghpr{1509})
\item Bounds-checked element access is faster, and vector lengths and
matrix column counts are now cached (Kevin in \ghpr{1511} closing
\ghit{1510})
}
\item Changes in Rcpp Documentation:
\itemize{
Expand Down
30 changes: 16 additions & 14 deletions inst/include/Rcpp/vector/Matrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace Rcpp{
template <int RTYPE, template <class> class StoragePolicy = PreserveStorage >
class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, true, Matrix<RTYPE,StoragePolicy> > {
int nrows ;
int ncols ;

public:
using Vector<RTYPE, StoragePolicy>::size; // disambiguate diamond pattern for g++-6 and later
Expand All @@ -49,33 +50,33 @@ class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, tru
typedef typename VECTOR::Proxy Proxy ;
typedef typename VECTOR::const_Proxy const_Proxy ;

Matrix() : VECTOR(Dimension(0, 0)), nrows(0) {}
Matrix() : VECTOR(Dimension(0, 0)), nrows(0), ncols(0) {}

Matrix(SEXP x) : VECTOR(x), nrows( VECTOR::dims()[0] ) {}
Matrix(SEXP x) : VECTOR(x), nrows( VECTOR::dims()[0] ), ncols( VECTOR::dims()[1] ) {}

Matrix( const Dimension& dims) : VECTOR( Rf_allocMatrix( RTYPE, dims[0], dims[1] ) ), nrows(dims[0]) {
Matrix( const Dimension& dims) : VECTOR( Rf_allocMatrix( RTYPE, dims[0], dims[1] ) ), nrows(dims[0]), ncols(dims[1]) {
if( dims.size() != 2 ) throw not_a_matrix();
VECTOR::init() ;
}
Matrix( const int& nrows_, const int& ncols) : VECTOR( Dimension( nrows_, ncols ) ),
nrows(nrows_)
Matrix( const int& nrows_, const int& ncols_) : VECTOR( Dimension( nrows_, ncols_ ) ),
nrows(nrows_), ncols(ncols_)
{}

template <typename Iterator>
Matrix( const int& nrows_, const int& ncols, Iterator start ) :
VECTOR( start, start + (static_cast<R_xlen_t>(nrows_)*ncols) ),
nrows(nrows_)
Matrix( const int& nrows_, const int& ncols_, Iterator start ) :
VECTOR( start, start + (static_cast<R_xlen_t>(nrows_)*ncols_) ),
nrows(nrows_), ncols(ncols_)
{
VECTOR::attr( "dim" ) = Dimension( nrows, ncols ) ;
}

Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n) {}
Matrix( const int& n) : VECTOR( Dimension( n, n ) ), nrows(n), ncols(n) {}


Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows) {}
Matrix( const Matrix& other) : VECTOR( other.get__() ), nrows(other.nrows), ncols(other.ncols) {}

template <bool NA, typename MAT>
Matrix( const MatrixBase<RTYPE,NA,MAT>& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast<int>(other.nrow()), static_cast<int>(other.ncol()) ) ), nrows(static_cast<int>(other.nrow())) {
Matrix( const MatrixBase<RTYPE,NA,MAT>& other ) : VECTOR( Rf_allocMatrix( RTYPE, static_cast<int>(other.nrow()), static_cast<int>(other.ncol()) ) ), nrows(static_cast<int>(other.nrow())), ncols(static_cast<int>(other.ncol())) {
import_matrix_expression<NA,MAT>( other, nrows, ncol() ) ;
}

Expand All @@ -86,20 +87,21 @@ class Matrix : public Vector<RTYPE, StoragePolicy>, public MatrixBase<RTYPE, tru
if( ! ::Rf_isMatrix(x) ) throw not_a_matrix();
VECTOR::set__( x ) ;
nrows = other.nrows ;
ncols = other.ncols ;
return *this ;
}
Matrix& operator=( const SubMatrix<RTYPE>& ) ;

explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()) {}
explicit Matrix( const no_init_matrix& obj) : VECTOR(Rf_allocMatrix(RTYPE, obj.nrow(), obj.ncol())), nrows(obj.nrow()), ncols(obj.ncol()) {}

inline int ncol() const {
return VECTOR::dims()[1];
return ncols ;
}
inline int nrow() const {
return nrows ;
}
inline int cols() const {
return VECTOR::dims()[1];
return ncols ;
}
inline int rows() const {
return nrows ;
Expand Down
3 changes: 2 additions & 1 deletion inst/include/Rcpp/vector/SubMatrix.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class SubMatrix : public Rcpp::MatrixBase< RTYPE, true, SubMatrix<RTYPE> > {
} ;

template <int RTYPE, template <class> class StoragePolicy >
Matrix<RTYPE,StoragePolicy>::Matrix( const SubMatrix<RTYPE>& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()) {
Matrix<RTYPE,StoragePolicy>::Matrix( const SubMatrix<RTYPE>& sub ) : VECTOR( Rf_allocMatrix( RTYPE, sub.nrow(), sub.ncol() )), nrows(sub.nrow()), ncols(sub.ncol()) {
int nc = sub.ncol() ;
iterator start = VECTOR::begin() ;
iterator rhs_it ;
Expand All @@ -73,6 +73,7 @@ Matrix<RTYPE,StoragePolicy>& Matrix<RTYPE,StoragePolicy>::operator=( const SubMa
int nc = sub.ncol(), nr = sub.nrow() ;
if( nc != nrow() || nr != ncol() ){
nrows = nr ;
ncols = nc ;
VECTOR::set__( Rf_allocMatrix( RTYPE, nr, nc ) ) ;
}
iterator start = VECTOR::begin() ;
Expand Down
6 changes: 3 additions & 3 deletions inst/include/Rcpp/vector/Vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,17 +263,17 @@ class Vector :
#endif

/**
* the length of the vector, uses Rf_xlength
* the length of the vector, as cached by update()
*/
inline R_xlen_t length() const {
return ::Rf_xlength( Storage::get__() ) ;
return cache.get_size() ;
}

/**
* alias of length
*/
inline R_xlen_t size() const {
return ::Rf_xlength( Storage::get__() ) ;
return cache.get_size() ;
}

/**
Expand Down
24 changes: 19 additions & 5 deletions inst/include/Rcpp/vector/traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@
namespace Rcpp{
namespace traits{

// Kept out of line and marked cold so that the (almost never taken)
// warning path doesn't bloat element access and block optimization of
// hot loops, e.g. in sugar expressions.
#if defined(__GNUC__)
__attribute__((noinline, cold))
#endif
inline void warn_index_out_of_bounds(R_xlen_t i, R_xlen_t size) {
warning("subscript out of bounds (index %s >= vector size %s)", i, size); // #nocov
}

template <int RTYPE, template <class> class StoragePolicy = PreserveStorage >
class r_vector_cache{
public:
Expand All @@ -38,11 +48,12 @@ namespace traits{

inline void update( const VECTOR& v ) {
start = ::Rcpp::internal::r_vector_start<RTYPE>(v) ;
size = v.size();
size = ::Rf_xlength(v.get__());
}

inline iterator get() const { return start; }
inline const_iterator get_const() const { return start; }
inline R_xlen_t get_size() const { return size; }

inline proxy ref() { check_index(0); return start[0] ;}
inline proxy ref(R_xlen_t i) { check_index(i); return start[i] ; }
Expand All @@ -55,7 +66,7 @@ namespace traits{
void check_index(R_xlen_t i) const {
#ifndef RCPP_NO_BOUNDS_CHECK
if (i >= size) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe the likely/unlikely macro technique used in the kernel helps too in these cases?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see there are C++ attributes too, but that means C++20.

warning("subscript out of bounds (index %s >= vector size %s)", i, size); // #nocov
warn_index_out_of_bounds(i, size); // #nocov
}
#endif
}
Expand All @@ -73,13 +84,15 @@ namespace traits{
typedef typename r_vector_proxy<RTYPE, StoragePolicy>::type proxy ;
typedef typename r_vector_const_proxy<RTYPE, StoragePolicy>::type const_proxy ;

proxy_cache(): p(0){}
proxy_cache(): p(0), size(0){}
~proxy_cache(){}
void update( const VECTOR& v ){
p = const_cast<VECTOR*>(&v) ;
size = ::Rf_xlength(v.get__());
}
inline iterator get() const { return iterator( proxy(*p, 0 ) ) ;}
inline const_iterator get_const() const { return const_iterator( const_proxy(*p, 0) ) ; }
inline R_xlen_t get_size() const { return size; }

inline proxy ref() { check_index(0); return proxy(*p,0) ; }
inline proxy ref(R_xlen_t i) { check_index(i); return proxy(*p,i);}
Expand All @@ -89,11 +102,12 @@ namespace traits{

private:
VECTOR* p ;
R_xlen_t size ;

void check_index(R_xlen_t i) const {
#ifndef RCPP_NO_BOUNDS_CHECK
if (i >= p->size()) {
warning("subscript out of bounds (index %s >= vector size %s)", i, p->size()); // #nocov
if (i >= size) {
warn_index_out_of_bounds(i, size); // #nocov
}
#endif
}
Expand Down
Loading