Repository navigation
Should we add an accessor for integer64? #7618
Description
Activity
I took a look through the codebase and found several places where integer64 data is accessed using direct casts such as (const int64_t *)DATAPTR_RO(x) and (int64_t *)DATAPTR(x). These appear in multiple files with slight variations, which seems to be the inconsistency this issue is pointing out.
Other core types (integer, real, logical, etc.) already have standardized accessor macros that improve readability and reduce the need for repeated casts. Following the same pattern, it seems reasonable to introduce something like:
#define INTEGER64_RO(x) ((const int64_t *) DATAPTR_RO(x)) #define INTEGER64(x) ((int64_t *) DATAPTR(x))Placing these alongside the existing accessors (in the same header where INTEGER_RO, REAL_RO, etc. are defined) would keep the style consistent with the rest of the codebase.
This would allow replacing the scattered casts with a single standardized form and improve internal consistency without changing behavior.
If this direction makes sense, I’d be happy to prepare a PR that introduces the macros and updates the current usages accordingly.
DATAPTRis non-API and may eventually be removed. There'sDATAPTR_RWintroduced in R-devel, but it's only intended to be used in some rare ALTREP method implementations. Sinceinteger64vectors are stored inREALSXPvectors, we should probably be usingREALandREAL_ROto access their data pointers.Reacted by Benjamin SchwendingerHi @MichaelChirico, I am a computer science student in an open source software class and me and my team are interested in getting involved with this project. I noticed there still don't seem to be any accessors as defined in this issue, so I was just curious as to whether this still needs to be implemented or if it is unimportant? Thanks!
Yes, I think it makes sense. I would proceed as follows.
- Get
INTEGER64_RO()/INTEGER64()accessors working and defined from {data.table} - Try defining them in {bit64} instead and referencing that definition from {data.table}. Is it possible to keep {bit64} as a Suggests dependency in that case?
Make sure that you are running this from the latest development version of R to minimize the chance of using non-API functions as warned by Ivan.
- Get
- added a commit that references this issue
on Apr 15, 2026 Hi @MichaelChirico, the macro works as defined in
src/data.table.hand just created an issue to implement this in {bit64} r-lib/bit64#312 (comment). We just wanted to clarify that this is following the progression you were envisioning? Thanks!the macro works as defined in src/data.table.h
I don't see such a macro there?
this is following the progression you were envisioning
Yes!
Hello @MichaelChirico, we are working on a test for the
inst/tests/tests.Rrawfor our PR but we noticed that tests 897-899.3 seems to already be coveringinterger64logic. We were concerned as to whether our tests would be meaningful or not since there this issue provides no additional logic. Thanks!Hello @MichaelChirico, we are working on a test for the
inst/tests/tests.Rrawfor our PR but we noticed that tests 897-899.3 seems to already be coveringinterger64logic. We were concerned as to whether our tests would be meaningful or not since there this issue provides no additional logic. Thanks!Since the PR only introduces the
INTEGER64_ROmacro and refactors code by using it instead, I guess no new tests are needed.
Our existing tests of differentinteger64use should act as regression tests.
Should we add an accessor for
integer64?#define INTEGER64_RO(x) ((const int64_t *) DATAPTR_RO(x))Originally posted by @ben-schwen in #7611 (comment)
Makes high-level sense to avoid coming up with different ways of doing this throughout the codebase.
It should probably be paired with an
INTEGER64(x)accessor too.And possibly, this should be something exported by {bit64} itself, although we may not want
LinkingTo{bit64}...