R_xlen_t as the vector length to support long vector #303
Conversation
|
|
||
| template<> | ||
| struct is_arithmetic<const unsigned long> : public true_type { }; | ||
|
|
There was a problem hiding this comment.
The following lines need to be protected by something like #if define(RCPP_HAS_LONG_LONG_TYPES). We may even use rcpp_long_long_type etc -- see the sibbling file traits/longlong.h.
|
Not a single failure is due to the A few are due to my 'cache' of packages having been emptier than usual. Rerunning now, previous result summary committed in rcpp-logs repo. |
| @@ -53,7 +53,7 @@ namespace Rcpp{ | |||
| } else { | |||
| if( ref.isNULL( ) ) throw index_out_of_bounds() ; | |||
|
|
|||
| if( static_cast<R_len_t>(index) > ::Rf_length(ref.get__()) ) throw index_out_of_bounds() ; | |||
| if( static_cast<R_xlen_t>(index) > ::Rf_length(ref.get__()) ) throw index_out_of_bounds() ; | |||
There was a problem hiding this comment.
Here you cast to R_xlen_t but check against ::Rf_length. Is that intentional? Should we be using ::Rf_xlength? (In practice it doesn't really matter for this class as it represents dotted pairs and I highly doubt those ever get very large in size...)
|
I made a quick review and all looks well to me barring some very minor comments. Thanks for your hard work -- this has been sorely needed for quite some time! I'll give a 👍 to merging this after @eddelbuettel's tests run through; we might want to do a second pass and scrutinize the use of Also, we could try rebuilding the Rcpp gallery with this just as one last thing to check. |
|
I've merged this into feature/r_xtlen_t-vector-length here: https://github.com/RcppCore/Rcpp/tree/feature/r_xlen_t-vector-length We should make further fixes and tweaks on that branch. Hopefully this can be merged to master soon! |
Most of changes are about
R_xlen_tandRf_xlength.Please take a careful review on
Vector.handis_arithmeticadded.The type trait is used because many packages depend the implicit conversion in
Vectorconstructor.