Remove cast to R_xlen_t (closes #1334)#1335
Conversation
| @@ -331,7 +331,7 @@ class Vector : | |||
| } | |||
| inline iterator begin() { return cache.get() ; } | |||
| inline iterator end() { return cache.get() + static_cast<int>(size()) ; } | |||
| inline iterator end() { return cache.get() + static_cast<R_xlen_t>(size()) ; } | |||
| inline const_iterator begin() const{ return cache.get_const() ; } | |||
| inline const_iterator end() const{ return cache.get_const() + size() ; } | |||
There was a problem hiding this comment.
A related question is whether the two const interators on lines 336 and 338 should also get the cast.
There was a problem hiding this comment.
It feels certainly wrong that there is a cast only there. What image did you use to reproduce the warnings?
There was a problem hiding this comment.
Also, given that this was address in two steps (one, two), and this line was part of the first PR, is it possible that, after the second set of changes, the static_cast is not needed anymore? Because size() already returns R_xlen_t. (Specifically, after this change).
We should be fine just dropping the cast.
There was a problem hiding this comment.
I think so too. We shuld be able to drop the cast.
I just installed clang++-17, at the time likely on Debian maybe using unstable. It is currently available in my default Ubuntu 24.04 setup.
There are other packages having issues with clang++-18 and now clang++-19 so we should maybe try those.
There was a problem hiding this comment.
No issues with clang++-19 with the compiler flags that tickled this, either in installation or check. Here is installation (in a rocker/r-base container using Debian testing/unstable)
root@e47796620ac8:/work# install.r * installing *source* package found in current working directory ... * installing *source* package ‘Rcpp’ ... ** using staged installation ** libs using C++ compiler: ‘Debian clang version 19.1.0 (4)’ clang++-19 -I"/usr/share/R/include" -DNDEBUG -I../inst/include/ -fpic -Wall -O3 -pedantic -Wconversion -Wno-sign-conversion -c api.cpp -o api.o clang++-19 -I"/usr/share/R/include" -DNDEBUG -I../inst/include/ -fpic -Wall -O3 -pedantic -Wconversion -Wno-sign-conversion -c attributes.cpp -o attributes.o clang++-19 -I"/usr/share/R/include" -DNDEBUG -I../inst/include/ -fpic -Wall -O3 -pedantic -Wconversion -Wno-sign-conversion -c barrier.cpp -o barrier.o clang++-19 -I"/usr/share/R/include" -DNDEBUG -I../inst/include/ -fpic -Wall -O3 -pedantic -Wconversion -Wno-sign-conversion -c date.cpp -o date.o clang++-19 -I"/usr/share/R/include" -DNDEBUG -I../inst/include/ -fpic -Wall -O3 -pedantic -Wconversion -Wno-sign-conversion -c module.cpp -o module.o clang++-19 -I"/usr/share/R/include" -DNDEBUG -I../inst/include/ -fpic -Wall -O3 -pedantic -Wconversion -Wno-sign-conversion -c rcpp_init.cpp -o rcpp_init.o clang++-19 -Wl,-S -shared -L/usr/lib/R/lib -Wl,-z,relro -o Rcpp.so api.o attributes.o barrier.o date.o module.o rcpp_init.o -L/usr/lib/R/lib -lR installing to /usr/local/lib/R/site-library/00LOCK-work/00new/Rcpp/libs ** R ** inst ** byte-compile and prepare package for lazy loading **help*** installing help indices ** building package indices ** installing vignettes ** testing if installed package can be loaded from temporary location ** checking absolute paths in shared objects and dynamic libraries ** testing if installed package can be loaded from final location ** testing if installed package keeps a record of temporary installation path * DONE (Rcpp) root@e47796620ac8:/work# There was a problem hiding this comment.
(To clarify, that was after I removed the cast.)
There was a problem hiding this comment.
I've now pushed it. I will take the slow road here and let the current (2 1/2 day total !!) rev.dep test finish, fill-in missing build dependencies it uncovers and then re-run again with the cast removed.
eddelbuettelforce-pushed the bugfix/vector_end_cast branch from 7c35c14 to 9cac874Compareeddelbuettel changed the title 
No issues found in reverse dependency checking so in it goes! |
This addresses the issue highlighted in #1334 by @Zilong-Li. Thanks again for noticing I closed this prematurely---my bad!
A rev.dep. has been started on that old machine so it will be a while until it is done. So far no issues but many, many more packages to go.
Checklist
R CMD checkstill passes all tests