Skip to content

Use _RO accessors for const targets #7611

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
ben-schwen merged 7 commits into from
Jan 24, 2026
Merged

Use _RO accessors for const targets #7611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
60f2ba2
Use _RO accessors for const targets
MichaelChirico Jan 20, 2026
a9f74ec
use read-only accessors when appropriate (#7615)
MichaelChirico Jan 21, 2026
5fe9198
one more _RO
MichaelChirico Jan 22, 2026
6dce9df
review suggestions
MichaelChirico Jan 23, 2026
fe92745
botched suggested change
MichaelChirico Jan 23, 2026
9a58485
Use more read-only accessors
aitap Jan 24, 2026
9b034c8
gfirstlast: use _RO accessors
aitap Jan 24, 2026
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
10 changes: 5 additions & 5 deletions src/assign.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
error(_("i is type '%s'. Must be integer, or numeric is coerced with warning. If i is a logical subset, simply wrap with which(), and take the which() outside the loop if possible for efficiency."), type2char(TYPEOF(rows)));
targetlen = length(rows);
numToDo = 0;
const int *rowsd = INTEGER(rows);
const int *rowsd = INTEGER_RO(rows);
for (int i=0; i<targetlen; ++i) {
if ((rowsd[i]<0 && rowsd[i]!=NA_INTEGER) || rowsd[i]>nrow)
error(_("i[%d] is %d which is out of range [1,nrow=%d]"), i+1, rowsd[i], nrow); // set() reaches here (test 2005.2); := reaches the same error in subset.c first
Expand Down Expand Up @@ -660,7 +660,7 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values)
if (ndelete) {
// delete any columns assigned NULL (there was a 'continue' earlier in loop above)
int *tt = (int *)R_alloc(ndelete, sizeof(*tt));
const int *colsd=INTEGER(cols), ncols=length(cols), ndt=length(dt);
const int *colsd=INTEGER_RO(cols), ncols=length(cols), ndt=length(dt);
for (int i=0, k=0; i<ncols; ++i) { // find which ones to delete and put them in tt
// Aside: a new column being assigned NULL (something odd to do) would have been warned above, added above, and now deleted. Just
// easier to code it this way; e.g. so that other columns may be added or removed ok by the same query.
Expand Down Expand Up @@ -743,7 +743,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
// allow assigning level numbers to factor columns; test 425, 426, 429 and 1945
const int nlevel = length(getAttrib(target, R_LevelsSymbol));
if (isInteger(source)) {
const int *sd = INTEGER(source);
const int *sd = INTEGER_RO(source);
for (int i=0; i<slen; ++i) {
const int val = sd[i+soff];
if ((val<1 && val!=NA_INTEGER) || val>nlevel) {
Expand All @@ -754,7 +754,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
}
}
} else {
const double *sd = REAL(source);
const double *sd = REAL_RO(source);
for (int i=0; i<slen; ++i) {
const double val = sd[i+soff];
// Since nlevel is an int, val < 1 || val > nlevel will deflect UB guarded against in PR #5832
Expand Down Expand Up @@ -800,7 +800,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
const int nSource = length(source);
int *newSourceD = INTEGER(newSource);
if (sourceIsFactor) {
const int *sourceD = INTEGER(source);
const int *sourceD = INTEGER_RO(source);
for (int i=0; i<nSource; ++i) { // convert source integers to refer to target levels
const int val = sourceD[i];
newSourceD[i] = val==NA_INTEGER ? NA_INTEGER : -hash_lookup(marks, sourceLevelsD[val-1], 0); // retains NA factor levels here via TL(NA_STRING); e.g. ordered factor
Expand Down
12 changes: 6 additions & 6 deletions src/between.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAboundsArg, S

switch (TYPEOF(x)) {
case INTSXP: {
const int *lp = INTEGER(lower);
const int *up = INTEGER(upper);
const int *xp = INTEGER(x);
const int *lp = INTEGER_RO(lower);
const int *up = INTEGER_RO(upper);
const int *xp = INTEGER_RO(x);
if (check) for (int i=0; i<longestBound; ++i) {
const int l=lp[i & lowMask], u=up[i & uppMask];
if (l!=NA_INTEGER && u!=NA_INTEGER && l>u)
Expand Down Expand Up @@ -128,9 +128,9 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAboundsArg, S
}
if (verbose) Rprintf(_("between parallel processing of integer64 took %8.3fs\n"), omp_get_wtime()-tic);
} else {
const double *lp = REAL(lower);
const double *up = REAL(upper);
const double *xp = REAL(x);
const double *lp = REAL_RO(lower);
const double *up = REAL_RO(upper);
const double *xp = REAL_RO(x);
if (check) for (int i=0; i<longestBound; ++i) {
const double l=lp[i & lowMask], u=up[i & uppMask];
if (!isnan(l) && !isnan(u) && l>u)
Expand Down
12 changes: 6 additions & 6 deletions src/bmerge.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ SEXP bmerge(SEXP idt, SEXP xdt, SEXP icolsArg, SEXP xcolsArg, SEXP xoArg, SEXP r
if ((LENGTH(icolsArg)==0 || LENGTH(xcolsArg)==0) && LENGTH(idt)>0) // We let through LENGTH(i) == 0 for tests 2126.*
internal_error(__func__, "icols and xcols must be non-empty integer vectors");
if (LENGTH(icolsArg) > LENGTH(xcolsArg)) internal_error(__func__, "length(icols) [%d] > length(xcols) [%d]", LENGTH(icolsArg), LENGTH(xcolsArg)); // # nocov
icols = INTEGER(icolsArg);
xcols = INTEGER(xcolsArg);
icols = INTEGER_RO(icolsArg);
xcols = INTEGER_RO(xcolsArg);
xN = LENGTH(xdt) ? LENGTH(VECTOR_ELT(xdt,0)) : 0;
iN = ilen = anslen = LENGTH(idt) ? LENGTH(VECTOR_ELT(idt,0)) : 0;
ncol = LENGTH(icolsArg); // there may be more sorted columns in x than involved in the join
Expand Down Expand Up @@ -356,8 +356,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg

switch (TYPEOF(xc)) {
case LGLSXP : case INTSXP : { // including factors
const int *icv = isDataCol ? INTEGER(ic) : NULL;
const int *xcv = INTEGER(xc);
const int *icv = isDataCol ? INTEGER_RO(ic) : NULL;
const int *xcv = INTEGER_RO(xc);
const int ival = isDataCol ? icv[ir] : thisgrp;
#define ISNAT(x) ((x)==NA_INTEGER)
#define WRAP(x) (x) // wrap not needed for int
Expand Down Expand Up @@ -388,8 +388,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg
#define WRAP(x) (x)
DO(const int64_t xval=xcv[XIND(mid)], xval<ival, xval>ival, int64_t, ival-xcv[XIND(xlow)], xcv[XIND(xupp)]-ival, ival)
} else {
const double *icv = REAL(ic);
const double *xcv = REAL(xc);
const double *icv = REAL_RO(ic);
const double *xcv = REAL_RO(xc);
const double ival = icv[ir];
const uint64_t ivalt = dtwiddle(ival); // TO: remove dtwiddle by dealing with NA, NaN, -Inf, +Inf up front
#undef ISNAT
Expand Down
6 changes: 3 additions & 3 deletions src/cj.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ SEXP cj(SEXP base_list)
switch(TYPEOF(source)) {
case LGLSXP:
case INTSXP: {
const int *restrict sourceP = INTEGER(source);
const int *restrict sourceP = INTEGER_RO(source);
int *restrict targetP = INTEGER(target);
#pragma omp parallel for num_threads(getDTthreads(thislen*eachrep, true))
// default static schedule so two threads won't write to same cache line in last column
Expand All @@ -41,7 +41,7 @@ SEXP cj(SEXP base_list)
}
} break;
case REALSXP: {
const double *restrict sourceP = REAL(source);
const double *restrict sourceP = REAL_RO(source);
double *restrict targetP = REAL(target);
#pragma omp parallel for num_threads(getDTthreads(thislen*eachrep, true))
for (int i = 0; i < thislen; i++) {
Expand All @@ -55,7 +55,7 @@ SEXP cj(SEXP base_list)
}
} break;
case CPLXSXP: {
const Rcomplex *restrict sourceP = COMPLEX(source);
const Rcomplex *restrict sourceP = COMPLEX_RO(source);
Rcomplex *restrict targetP = COMPLEX(target);
#pragma omp parallel for num_threads(getDTthreads(thislen*eachrep, true))
for (int i = 0; i < thislen; i++) {
Expand Down
6 changes: 3 additions & 3 deletions src/dogroups.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
iSD = PROTECT(R_getVar(install(".iSD"), env, false)); nprotect++; // 1-row and possibly no cols (if no i variables are used via JIS)
xSD = PROTECT(R_getVar(install(".xSD"), env, false)); nprotect++;
R_len_t maxGrpSize = 0;
const int *ilens = INTEGER(lens), n=LENGTH(lens);
const int *ilens = INTEGER_RO(lens), n=LENGTH(lens);
for (R_len_t i=0; i<n; ++i) {
if (ilens[i] > maxGrpSize) maxGrpSize = ilens[i];
}
Expand Down Expand Up @@ -184,8 +184,8 @@ SEXP dogroups(SEXP dt, SEXP dtcols, SEXP groups, SEXP grpcols, SEXP jiscols, SEX
Rboolean jexpIsSymbolOtherThanSD = (isSymbol(jexp) && strcmp(CHAR(PRINTNAME(jexp)),".SD")!=0); // test 559

ansloc = 0;
const int *istarts = INTEGER(starts);
const int *iorder = INTEGER(order);
const int *istarts = INTEGER_RO(starts);
const int *iorder = INTEGER_RO(order);

// We just want to set anyNA for later. We do it only once for the whole operation
// because it is a rare edge case for it to be true. See #4892.
Expand Down
12 changes: 6 additions & 6 deletions src/fcast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ SEXP fcast(SEXP lhs, SEXP val, SEXP nrowArg, SEXP ncolArg, SEXP idxArg, SEXP fil
switch (thistype) {
case INTSXP:
case LGLSXP: {
const int *ithiscol = INTEGER(thiscol);
const int *ithiscol = INTEGER_RO(thiscol);
const int *ithisfill = NULL;
if (some_fill) ithisfill = INTEGER(thisfill);
if (some_fill) ithisfill = INTEGER_RO(thisfill);
for (int j=0; j<ncols; ++j) {
SET_VECTOR_ELT(ans, nlhs+j+i*ncols, target=allocVector(thistype, nrows) );
int *itarget = INTEGER(target);
Expand All @@ -51,9 +51,9 @@ SEXP fcast(SEXP lhs, SEXP val, SEXP nrowArg, SEXP ncolArg, SEXP idxArg, SEXP fil
}
} break;
case REALSXP: {
const double *dthiscol = REAL(thiscol);
const double *dthiscol = REAL_RO(thiscol);
const double *dthisfill = NULL;
if (some_fill) dthisfill = REAL(thisfill);
if (some_fill) dthisfill = REAL_RO(thisfill);
for (int j=0; j<ncols; ++j) {
SET_VECTOR_ELT(ans, nlhs+j+i*ncols, target=allocVector(thistype, nrows) );
double *dtarget = REAL(target);
Expand All @@ -65,9 +65,9 @@ SEXP fcast(SEXP lhs, SEXP val, SEXP nrowArg, SEXP ncolArg, SEXP idxArg, SEXP fil
}
} break;
case CPLXSXP: {
const Rcomplex *zthiscol = COMPLEX(thiscol);
const Rcomplex *zthiscol = COMPLEX_RO(thiscol);
const Rcomplex *zthisfill = NULL;
if (some_fill) zthisfill = COMPLEX(thisfill);
if (some_fill) zthisfill = COMPLEX_RO(thisfill);
for (int j=0; j<ncols; ++j) {
SET_VECTOR_ELT(ans, nlhs+j+i*ncols, target=allocVector(thistype, nrows) );
Rcomplex *ztarget = COMPLEX(target);
Expand Down
12 changes: 6 additions & 6 deletions src/fifelse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b, SEXP na) {
const int64_t bmask = len2>1 ? INT64_MAX : 0;
const int64_t nmask = len3>1 ? INT64_MAX : 0;

const int *restrict pl = LOGICAL(l);
const int *restrict pl = LOGICAL_RO(l);
SEXP ans = PROTECT(allocVector(tans, len0)); nprotect++;
if (!na_a)
copyMostAttrib(a, ans);
Expand Down Expand Up @@ -237,7 +237,7 @@ SEXP fcaseR(SEXP rho, SEXP args) {
if (!isLogical(whens)) {
error(_("Argument #%d must be logical but was of type %s."), 2*i+1, type2char(TYPEOF(whens)));
}
const int *restrict pwhens = LOGICAL(whens);
const int *restrict pwhens = LOGICAL_RO(whens);
l = 0;
if (i == 0) {
n_ans = xlength(whens);
Expand Down Expand Up @@ -306,7 +306,7 @@ SEXP fcaseR(SEXP rho, SEXP args) {
switch(TYPEOF(ans)) {
case LGLSXP: {
const int *restrict pthens;
if (!naout) pthens = LOGICAL(thens); // the content is not useful if out is NA_LOGICAL scalar
if (!naout) pthens = LOGICAL_RO(thens); // the content is not useful if out is NA_LOGICAL scalar
int *restrict pans = LOGICAL(ans);
const int pna = NA_LOGICAL;
for (int64_t j=0; j<n_undecided; ++j) {
Expand All @@ -323,7 +323,7 @@ SEXP fcaseR(SEXP rho, SEXP args) {
} break;
case INTSXP: {
const int *restrict pthens;
if (!naout) pthens = INTEGER(thens); // the content is not useful if out is NA_LOGICAL scalar
if (!naout) pthens = INTEGER_RO(thens); // the content is not useful if out is NA_LOGICAL scalar
int *restrict pans = INTEGER(ans);
const int pna = NA_INTEGER;
for (int64_t j=0; j<n_undecided; ++j) {
Expand All @@ -340,7 +340,7 @@ SEXP fcaseR(SEXP rho, SEXP args) {
} break;
case REALSXP: {
const double *restrict pthens;
if (!naout) pthens = REAL(thens); // the content is not useful if out is NA_LOGICAL scalar
if (!naout) pthens = REAL_RO(thens); // the content is not useful if out is NA_LOGICAL scalar
double *restrict pans = REAL(ans);
const double na_double = INHERITS(ans, char_integer64) ? NA_INT64_D : NA_REAL;
const double pna = na_double;
Expand All @@ -358,7 +358,7 @@ SEXP fcaseR(SEXP rho, SEXP args) {
} break;
case CPLXSXP: {
const Rcomplex *restrict pthens;
if (!naout) pthens = COMPLEX(thens); // the content is not useful if out is NA_LOGICAL scalar
if (!naout) pthens = COMPLEX_RO(thens); // the content is not useful if out is NA_LOGICAL scalar
Rcomplex *restrict pans = COMPLEX(ans);
const Rcomplex pna = NA_CPLX;
for (int64_t j=0; j<n_undecided; ++j) {
Expand Down
22 changes: 11 additions & 11 deletions src/fmelt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ SEXP set_diff(SEXP x, int n) {
if (n <= 0) error(_("'n' must be a positive integer"));
SEXP table = PROTECT(seq_int(n, 1)); // TODO: using match to 1:n seems odd here, why use match at all
SEXP xmatch = PROTECT(match(x, table, 0)); // Old comment:took a while to realise: matches vec against x - thanks to comment from Matt in assign.c!
const int *ixmatch = INTEGER(xmatch);
const int *ixmatch = INTEGER_RO(xmatch);
int *buf = (int *) R_alloc(n, sizeof(*buf));
int j=0;
for (int i=0; i<n; ++i) {
Expand All @@ -39,7 +39,7 @@ SEXP which(SEXP x, Rboolean val) {
int j=0, n = length(x);
SEXP ans;
if (!isLogical(x)) error(_("Argument to 'which' must be logical"));
const int *ix = LOGICAL(x);
const int *ix = LOGICAL_RO(x);
int *buf = (int *) R_alloc(n, sizeof(*buf));
for (int i=0; i<n; ++i) {
if (ix[i] == val) {
Expand Down Expand Up @@ -70,7 +70,7 @@ static const char *concat(SEXP vec, SEXP idx) {
int nidx=length(idx), nvec=length(vec);
ans[0]='\0';
if (nidx==0) return ans;
const int *iidx = INTEGER(idx);
const int *iidx = INTEGER_RO(idx);
for (int i=0; i<nidx; ++i) {
if (iidx[i]<1 || iidx[i]>nvec)
internal_error(__func__, "'idx' must take values between 1 and length(vec); 1 <= idx <= %d", nvec); // # nocov
Expand Down Expand Up @@ -150,7 +150,7 @@ static SEXP unlist_(SEXP xint) {
int *ians = INTEGER(ans), k=0;
for (int i=0; i<n; ++i) {
SEXP tmp = VECTOR_ELT(xint, i);
const int *itmp = INTEGER(tmp), n2=length(tmp);
const int *itmp = INTEGER_RO(tmp), n2=length(tmp);
for (int j=0; j<n2; ++j)
ians[k++] = itmp[j];
}
Expand Down Expand Up @@ -522,7 +522,7 @@ SEXP getvaluecols(SEXP DT, SEXP dtnames, Rboolean valfactor, Rboolean verbose, s
int thislen = 0;
if (data->narm) {
SEXP thisidx = VECTOR_ELT(data->not_NA_indices, j);
ithisidx = INTEGER(thisidx);
ithisidx = INTEGER_RO(thisidx);
thislen = length(thisidx);
}
size_t size = RTYPE_SIZEOF(thiscol);
Expand Down Expand Up @@ -600,7 +600,7 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str
if (!varfactor) {
SET_VECTOR_ELT(ansvars, 0, target=allocVector(STRSXP, data->totlen));
if (!data->measure_is_list) {//one value column to output.
const int *thisvaluecols = INTEGER(VECTOR_ELT(data->valuecols, 0));
const int *thisvaluecols = INTEGER_RO(VECTOR_ELT(data->valuecols, 0));
for (int j=0, ansloc=0; j<data->lmax; ++j) {
const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow;
SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1);
Expand All @@ -622,7 +622,7 @@ SEXP getvarcols(SEXP DT, SEXP dtnames, Rboolean varfactor, Rboolean verbose, str
SEXP thisvaluecols = VECTOR_ELT(data->valuecols, 0);
int len = length(thisvaluecols);
levels = PROTECT(allocVector(STRSXP, len)); protecti++;
const int *vd = INTEGER(thisvaluecols);
const int *vd = INTEGER_RO(thisvaluecols);
for (int j=0; j<len; ++j) SET_STRING_ELT(levels, j, STRING_ELT(dtnames, vd[j]-1));
SEXP m = PROTECT(chmatch(levels, levels, 0)); protecti++; // do we have any dups?
int numRemove = 0; // remove dups and any for which narm and all-NA
Expand Down Expand Up @@ -706,7 +706,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
if (data->narm) {
for (int j=0; j<data->lmax; ++j) {
SEXP thisidx = VECTOR_ELT(data->not_NA_indices, j);
const int *ithisidx = INTEGER(thisidx);
const int *ithisidx = INTEGER_RO(thisidx);
const int thislen = length(thisidx);
for (int k=0; k<thislen; ++k)
dtarget[counter + k] = dthiscol[ithisidx[k]-1];
Expand All @@ -725,7 +725,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
if (data->narm) {
for (int j=0; j<data->lmax; ++j) {
SEXP thisidx = VECTOR_ELT(data->not_NA_indices, j);
const int *ithisidx = INTEGER(thisidx);
const int *ithisidx = INTEGER_RO(thisidx);
const int thislen = length(thisidx);
for (int k=0; k<thislen; ++k)
itarget[counter + k] = ithiscol[ithisidx[k]-1];
Expand All @@ -740,7 +740,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
if (data->narm) {
for (int j=0; j<data->lmax; ++j) {
SEXP thisidx = VECTOR_ELT(data->not_NA_indices, j);
const int *ithisidx = INTEGER(thisidx);
const int *ithisidx = INTEGER_RO(thisidx);
const int thislen = length(thisidx);
for (int k=0; k<thislen; ++k)
SET_STRING_ELT(target, counter + k, STRING_ELT(thiscol, ithisidx[k]-1));
Expand All @@ -760,7 +760,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
if (data->narm) {
for (int j=0; j<data->lmax; ++j) {
SEXP thisidx = VECTOR_ELT(data->not_NA_indices, j);
const int *ithisidx = INTEGER(thisidx);
const int *ithisidx = INTEGER_RO(thisidx);
const int thislen = length(thisidx);
for (int k=0; k<thislen; ++k)
SET_VECTOR_ELT(target, counter + k, VECTOR_ELT(thiscol, ithisidx[k]-1));
Expand Down
Loading
Loading