From 60f2ba2351a11255c5f878c5bf4f9342f70f8900 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Tue, 20 Jan 2026 20:49:06 +0000 Subject: [PATCH 01/15] Use _RO accessors for const targets --- src/assign.c | 10 +++++----- src/between.c | 12 ++++++------ src/bmerge.c | 6 +++--- src/dogroups.c | 6 +++--- src/fcast.c | 6 +++--- src/fmelt.c | 20 ++++++++++---------- src/forder.c | 4 ++-- src/frank.c | 16 ++++++++-------- src/freadR.c | 2 +- src/gsumm.c | 14 +++++++------- src/rbindlist.c | 2 +- src/shift.c | 2 +- src/subset.c | 6 +++--- src/uniqlist.c | 4 ++-- src/utils.c | 10 +++++----- src/vecseq.c | 4 ++-- 16 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/assign.c b/src/assign.c index 5901d5a152..05a55cb5a9 100644 --- a/src/assign.c +++ b/src/assign.c @@ -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; inrow) 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 @@ -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; inlevel) { @@ -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 nlevel will deflect UB guarded against in PR #5832 @@ -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; iu) @@ -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; iu) diff --git a/src/bmerge.c b/src/bmerge.c index 737e27f98d..832a958f90 100644 --- a/src/bmerge.c +++ b/src/bmerge.c @@ -357,7 +357,7 @@ 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 *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 @@ -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)], xvalival, 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 diff --git a/src/dogroups.c b/src/dogroups.c index 7fd1b956e9..e60c4c916b 100644 --- a/src/dogroups.c +++ b/src/dogroups.c @@ -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 maxGrpSize) maxGrpSize = ilens[i]; } @@ -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. diff --git a/src/fcast.c b/src/fcast.c index 0207e5dcb2..7d4c31cdb3 100644 --- a/src/fcast.c +++ b/src/fcast.c @@ -37,7 +37,7 @@ 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); for (int j=0; jnvec) internal_error(__func__, "'idx' must take values between 1 and length(vec); 1 <= idx <= %d", nvec); // # nocov @@ -150,7 +150,7 @@ static SEXP unlist_(SEXP xint) { int *ians = INTEGER(ans), k=0; for (int i=0; itotlen)); 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; jlmax; ++j) { const int thislen = data->narm ? length(VECTOR_ELT(data->not_NA_indices, j)) : data->nrow; SEXP str = STRING_ELT(dtnames, thisvaluecols[j]-1); @@ -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; jnarm) { for (int j=0; jlmax; ++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; knarm) { for (int j=0; jlmax; ++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; knarm) { for (int j=0; jlmax; ++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; knarm) { for (int j=0; jlmax; ++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=0"), nrow); - const int *xd = INTEGER(x), xlen=LENGTH(x); + const int *xd = INTEGER_RO(x), xlen=LENGTH(x); for (int i=0, last=INT_MIN; i Date: Wed, 21 Jan 2026 00:42:27 -0800 Subject: [PATCH 02/15] use read-only accessors when appropriate (#7615) --- src/forder.c | 16 ++++++++-------- src/fwriteR.c | 2 +- src/gsumm.c | 6 +++--- src/ijoin.c | 12 ++++++------ src/nafill.c | 2 +- src/uniqlist.c | 6 +++--- src/utils.c | 4 ++-- src/wrappers.c | 2 +- 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/forder.c b/src/forder.c index b86f599ee4..e7cb7f250c 100644 --- a/src/forder.c +++ b/src/forder.c @@ -723,7 +723,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP retStatsArg, SEXP sortGroupsA switch(TYPEOF(x)) { case INTSXP : case LGLSXP : { - int32_t *xd = INTEGER(x); + const int32_t *xd = INTEGER_RO(x); #pragma omp parallel for num_threads(getDTthreads(nrow, true)) for (int i=0; i=xd[i-1]) i++; } break; case REALSXP : if (inherits(x,"integer64")) { - int64_t *xd = (int64_t *)REAL(x); + const int64_t *xd = (int64_t *)REAL_RO(x); while (i=xd[i-1]) i++; } else { - double *xd = REAL(x); + const double *xd = REAL_RO(x); while (i=dtwiddle(xd[i-1])) i++; // TODO: change to loop over any NA or -Inf at the beginning and then proceed without dtwiddle() (but rounding) } break; @@ -1565,7 +1565,7 @@ SEXP binary(SEXP x) static bool all1(SEXP x) { if (!isInteger(x)) internal_error_with_cleanup(__func__, "all1 got non-integer"); // # nocov - int *xp = INTEGER(x); + const int *xp = INTEGER_RO(x); for (int i=0; i= 1 && ixi <= nlevels) ? STRING_ELT(levels, ix[i]-1) : NA_STRING); } newx = PROTECT(chmatch(xchar, ulevels, NA_INTEGER)); - int *inewx = INTEGER(newx); + const int *inewx = INTEGER_RO(newx); for (int i=0; i Date: Wed, 21 Jan 2026 23:33:46 -0800 Subject: [PATCH 03/15] one more _RO Co-authored-by: Benjamin Schwendinger <52290390+ben-schwen@users.noreply.github.com> --- src/bmerge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bmerge.c b/src/bmerge.c index 832a958f90..fa631bcc63 100644 --- a/src/bmerge.c +++ b/src/bmerge.c @@ -356,7 +356,7 @@ 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 *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) From 6dce9dfa724ec2d17e313ae0fa3c49d2b416810b Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 23 Jan 2026 11:27:36 -0800 Subject: [PATCH 04/15] review suggestions Co-authored-by: Benjamin Schwendinger <52290390+ben-schwen@users.noreply.github.com> --- src/fmelt.c | 2 +- src/gsumm.c | 7 ++++++- src/subset.c | 2 +- src/uniqlist.c | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/fmelt.c b/src/fmelt.c index a53aa25911..287ba4d0d4 100644 --- a/src/fmelt.c +++ b/src/fmelt.c @@ -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); diff --git a/src/gsumm.c b/src/gsumm.c index 9de8ce34f9..32c34b6375 100644 --- a/src/gsumm.c +++ b/src/gsumm.c @@ -1271,7 +1271,12 @@ SEXP gshift(SEXP x, SEXP nArg, SEXP fillArg, SEXP typeArg) { case INTSXP: { int *ansd=INTEGER(tmp); SHIFT(int, INTEGER, ansd[ansi++]=val); } break; case REALSXP: { double *ansd=REAL(tmp); SHIFT(double, REAL, ansd[ansi++]=val); } break; // integer64 is shifted as if it's REAL; and assigning fill=NA_INTEGER64 is ok as REAL - case CPLXSXP: { Rcomplex *ansd=COMPLEX(tmp); SHIFT(Rcomplex, COMPLEX, ansd[ansi++]=val); } break; +case RAWSXP: { Rbyte *ansd=RAW(tmp); SHIFT(Rbyte, RAW_RO, ansd[ansi++]=val); } break; +case LGLSXP: { int *ansd=LOGICAL(tmp); SHIFT(int, LOGICAL_RO, ansd[ansi++]=val); } break; +case INTSXP: { int *ansd=INTEGER(tmp); SHIFT(int, INTEGER_RO, ansd[ansi++]=val); } break; +case REALSXP: { double *ansd=REAL(tmp); SHIFT(double, REAL_RO, ansd[ansi++]=val); } break; +// integer64 is shifted as if it's REAL; and assigning fill=NA_INTEGER64 is ok as REAL +case CPLXSXP: { Rcomplex *ansd=COMPLEX(tmp); SHIFT(Rcomplex, COMPLEX_RO, ansd[ansi++]=val); } break; case STRSXP: { SHIFT(SEXP, STRING_PTR_RO, SET_STRING_ELT(tmp,ansi++,val)); } break; //case VECSXP: { SHIFT(SEXP, SEXPPTR_RO, SET_VECTOR_ELT(tmp,ansi++,val)); } break; default: diff --git a/src/subset.c b/src/subset.c index 28e83d03ff..673ef80cbd 100644 --- a/src/subset.c +++ b/src/subset.c @@ -110,7 +110,7 @@ const char *check_idx(SEXP idx, int max, bool *anyNA_out, bool *orderedSubset_ou if (!isInteger(idx)) internal_error(__func__, "Argument '%s' to %s is type '%s' not '%s'", "idx", "check_idx", type2char(TYPEOF(idx)), "integer"); // # nocov bool anyLess=false, anyNA=false; int last = INT32_MIN; - int *idxp = INTEGER(idx), n=LENGTH(idx); + const int *idxp = INTEGER_RO(idx), n=LENGTH(idx); for (int i=0; i Date: Fri, 23 Jan 2026 13:37:06 -0800 Subject: [PATCH 05/15] botched suggested change --- src/gsumm.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/gsumm.c b/src/gsumm.c index 32c34b6375..3a566ff689 100644 --- a/src/gsumm.c +++ b/src/gsumm.c @@ -1266,17 +1266,12 @@ SEXP gshift(SEXP x, SEXP nArg, SEXP fillArg, SEXP typeArg) { } \ } switch(TYPEOF(x)) { - case RAWSXP: { Rbyte *ansd=RAW(tmp); SHIFT(Rbyte, RAW, ansd[ansi++]=val); } break; - case LGLSXP: { int *ansd=LOGICAL(tmp); SHIFT(int, LOGICAL, ansd[ansi++]=val); } break; - case INTSXP: { int *ansd=INTEGER(tmp); SHIFT(int, INTEGER, ansd[ansi++]=val); } break; - case REALSXP: { double *ansd=REAL(tmp); SHIFT(double, REAL, ansd[ansi++]=val); } break; + case RAWSXP: { Rbyte *ansd=RAW(tmp); SHIFT(Rbyte, RAW_RO, ansd[ansi++]=val); } break; + case LGLSXP: { int *ansd=LOGICAL(tmp); SHIFT(int, LOGICAL_RO, ansd[ansi++]=val); } break; + case INTSXP: { int *ansd=INTEGER(tmp); SHIFT(int, INTEGER_RO, ansd[ansi++]=val); } break; + case REALSXP: { double *ansd=REAL(tmp); SHIFT(double, REAL_RO, ansd[ansi++]=val); } break; // integer64 is shifted as if it's REAL; and assigning fill=NA_INTEGER64 is ok as REAL -case RAWSXP: { Rbyte *ansd=RAW(tmp); SHIFT(Rbyte, RAW_RO, ansd[ansi++]=val); } break; -case LGLSXP: { int *ansd=LOGICAL(tmp); SHIFT(int, LOGICAL_RO, ansd[ansi++]=val); } break; -case INTSXP: { int *ansd=INTEGER(tmp); SHIFT(int, INTEGER_RO, ansd[ansi++]=val); } break; -case REALSXP: { double *ansd=REAL(tmp); SHIFT(double, REAL_RO, ansd[ansi++]=val); } break; -// integer64 is shifted as if it's REAL; and assigning fill=NA_INTEGER64 is ok as REAL -case CPLXSXP: { Rcomplex *ansd=COMPLEX(tmp); SHIFT(Rcomplex, COMPLEX_RO, ansd[ansi++]=val); } break; + case CPLXSXP: { Rcomplex *ansd=COMPLEX(tmp); SHIFT(Rcomplex, COMPLEX_RO, ansd[ansi++]=val); } break; case STRSXP: { SHIFT(SEXP, STRING_PTR_RO, SET_STRING_ELT(tmp,ansi++,val)); } break; //case VECSXP: { SHIFT(SEXP, SEXPPTR_RO, SET_VECTOR_ELT(tmp,ansi++,val)); } break; default: From 9a58485344c8838ba30da7bc3efe9c056146c6c7 Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 24 Jan 2026 14:56:15 +0300 Subject: [PATCH 06/15] Use more read-only accessors --- src/bmerge.c | 4 ++-- src/cj.c | 6 +++--- src/fcast.c | 6 +++--- src/fifelse.c | 12 ++++++------ src/freadR.c | 6 +++--- src/fsort.c | 2 +- src/gsumm.c | 10 +++++----- src/nafill.c | 6 +++--- src/shift.c | 8 ++++---- src/subset.c | 2 +- 10 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/bmerge.c b/src/bmerge.c index fa631bcc63..c222501756 100644 --- a/src/bmerge.c +++ b/src/bmerge.c @@ -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 diff --git a/src/cj.c b/src/cj.c index 2d59d0511f..0125141a8f 100644 --- a/src/cj.c +++ b/src/cj.c @@ -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 @@ -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++) { @@ -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++) { diff --git a/src/fcast.c b/src/fcast.c index 7d4c31cdb3..8b846d8401 100644 --- a/src/fcast.c +++ b/src/fcast.c @@ -39,7 +39,7 @@ SEXP fcast(SEXP lhs, SEXP val, SEXP nrowArg, SEXP ncolArg, SEXP idxArg, SEXP fil case LGLSXP: { 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; j1 ? 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); @@ -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); @@ -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>bitshift) + 1; @@ -224,7 +224,7 @@ void *gather(SEXP x, bool *anyNA) const bool verbose = GetVerbose(); switch (TYPEOF(x)) { case LGLSXP: case INTSXP: { - const int *restrict thisx = INTEGER(x); + const int *restrict thisx = INTEGER_RO(x); #pragma omp parallel for num_threads(getDTthreads(nBatch, false)) for (int b=0; b Date: Sat, 24 Jan 2026 15:24:55 +0300 Subject: [PATCH 07/15] gfirstlast: use _RO accessors Keep the casts because they are currently necessary for the integer64 case. --- src/gsumm.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/gsumm.c b/src/gsumm.c index 1f9ab65419..5051d72ca8 100644 --- a/src/gsumm.c +++ b/src/gsumm.c @@ -977,12 +977,12 @@ static SEXP gfirstlast(SEXP x, const bool first, const int w, const bool headw) } \ } switch(TYPEOF(x)) { - case LGLSXP: { int *ansd=LOGICAL(ans); DO(int, LOGICAL, NA_LOGICAL, ansd[ansi++]=val) } break; - case INTSXP: { int *ansd=INTEGER(ans); DO(int, INTEGER, NA_INTEGER, ansd[ansi++]=val) } break; + case LGLSXP: { int *ansd=LOGICAL(ans); DO(int, LOGICAL_RO, NA_LOGICAL, ansd[ansi++]=val) } break; + case INTSXP: { int *ansd=INTEGER(ans); DO(int, INTEGER_RO, NA_INTEGER, ansd[ansi++]=val) } break; case REALSXP: if (INHERITS(x, char_integer64)) { - int64_t *ansd=(int64_t *)REAL(ans); DO(int64_t, REAL, NA_INTEGER64, ansd[ansi++]=val) } - else { double *ansd=REAL(ans); DO(double, REAL, NA_REAL, ansd[ansi++]=val) } break; - case CPLXSXP: { Rcomplex *ansd=COMPLEX(ans); DO(Rcomplex, COMPLEX, NA_CPLX, ansd[ansi++]=val) } break; + int64_t *ansd=(int64_t *)REAL(ans); DO(int64_t, REAL_RO, NA_INTEGER64, ansd[ansi++]=val) } + else { double *ansd=REAL(ans); DO(double, REAL_RO, NA_REAL, ansd[ansi++]=val) } break; + case CPLXSXP: { Rcomplex *ansd=COMPLEX(ans); DO(Rcomplex, COMPLEX_RO, NA_CPLX, ansd[ansi++]=val) } break; case STRSXP: DO(SEXP, STRING_PTR_RO, NA_STRING, SET_STRING_ELT(ans,ansi++,val)) break; case VECSXP: DO(SEXP, SEXPPTR_RO, ScalarLogical(NA_LOGICAL), SET_VECTOR_ELT(ans,ansi++,val)) break; default: From a1f96965d13397046fe5a73d19f93b12ea050617 Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 22 Jan 2026 09:51:37 +0100 Subject: [PATCH 08/15] add coccinelle rules to use read only accessors --- .ci/linters/cocci/use_ro_accessors.cocci | 35 ++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 .ci/linters/cocci/use_ro_accessors.cocci diff --git a/.ci/linters/cocci/use_ro_accessors.cocci b/.ci/linters/cocci/use_ro_accessors.cocci new file mode 100644 index 0000000000..bfdc6e1b4b --- /dev/null +++ b/.ci/linters/cocci/use_ro_accessors.cocci @@ -0,0 +1,35 @@ +@@ +expression E; +@@ +-(const int*) INTEGER(E) ++INTEGER_RO(E) + +@@ +expression E; +@@ +-(const double*) REAL(E) ++REAL_RO(E) + +@@ +expression E; +@@ +-(const int*) LOGICAL(E) ++LOGICAL_RO(E) + +@@ +expression E; +@@ +-(const Rbyte*) RAW(E) ++RAW_RO(E) + +@@ +expression E; +@@ +-(const Rcomplex*) COMPLEX(E) ++COMPLEX_RO(E) + +@@ +expression E; +@@ +-(const SEXP*) STRING_PTR(E) ++STRING_PTR_RO(E) From 0a2e0906fb4e16b240ab03c1fd3ffc8cbbc5767d Mon Sep 17 00:00:00 2001 From: Benjamin Schwendinger Date: Thu, 22 Jan 2026 10:04:41 +0100 Subject: [PATCH 09/15] CI should also run when cocci files are touched --- .github/workflows/code-quality.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/code-quality.yaml b/.github/workflows/code-quality.yaml index 7e0d655aee..2fda8f60f7 100644 --- a/.github/workflows/code-quality.yaml +++ b/.github/workflows/code-quality.yaml @@ -29,6 +29,7 @@ jobs: c: - '**/*.c' - '**/*.h' + - '.ci/linters/cocci/**/*.cocci' po: - 'po/**/*.po' md: From 70d37da5a8548d67337ebade6e8a25fc64bdf9df Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 24 Jan 2026 15:16:16 +0300 Subject: [PATCH 10/15] cocci_linter.R: fix zero-length error reports --- .ci/linters/c/cocci_linter.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/linters/c/cocci_linter.R b/.ci/linters/c/cocci_linter.R index a2502f43b9..9c5998cb85 100644 --- a/.ci/linters/c/cocci_linter.R +++ b/.ci/linters/c/cocci_linter.R @@ -10,12 +10,12 @@ cocci_linter = if (!nzchar(Sys.which("spatch"))) function(...) {} else function( stdout = TRUE, stderr = FALSE ) if (length(out) > 0) { - cat(sprintf("In file '%s', Coccinelle linter '%s' located the following problems:\n", c_obj$path, spfile)) + cat(sprintf("In file '%s', Coccinelle linter '%s' located the following problems:\n", c_obj$c_obj, spfile)) writeLines(out) bad = TRUE } if (!is.null(status <- attr(out, 'status'))) { - cat(sprintf("While working on file '%s', Coccinelle linter '%s' failed with exit code %d:\n", c_obj$path, spfile, status)) + cat(sprintf("While working on file '%s', Coccinelle linter '%s' failed with exit code %d:\n", c_obj$c_obj, spfile, status)) bad = TRUE } } From 83406ed106ffbbec47c8336ac1dedffeb4c9a5a0 Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sat, 24 Jan 2026 15:26:03 +0300 Subject: [PATCH 11/15] use_ro_accessors.cocci: expand linter rules --- .ci/linters/cocci/use_ro_accessors.cocci | 49 ++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/.ci/linters/cocci/use_ro_accessors.cocci b/.ci/linters/cocci/use_ro_accessors.cocci index bfdc6e1b4b..753317055c 100644 --- a/.ci/linters/cocci/use_ro_accessors.cocci +++ b/.ci/linters/cocci/use_ro_accessors.cocci @@ -1,3 +1,43 @@ +@@ +type T; +const T *variable; +expression E; +@@ +- variable = REAL(E) ++ variable = REAL_RO(E) + +@@ +type T; +const T *variable; +expression E; +@@ +- variable = INTEGER(E) ++ variable = INTEGER_RO(E) + +@@ +type T; +const T *variable; +expression E; +@@ +- variable = COMPLEX(E) ++ variable = COMPLEX_RO(E) + +@@ +type T; +const T *variable; +expression E; +@@ +- variable = RAW(E) ++ variable = RAW_RO(E) + +@@ +type T; +const T *variable; +expression E; +@@ +- variable = LOGICAL(E) ++ variable = LOGICAL_RO(E) + @@ expression E; @@ @@ -18,18 +58,21 @@ expression E; @@ expression E; +type T; @@ --(const Rbyte*) RAW(E) +-(const T*) RAW(E) +RAW_RO(E) @@ expression E; +type T; @@ --(const Rcomplex*) COMPLEX(E) +-(const T*) COMPLEX(E) +COMPLEX_RO(E) @@ expression E; +type T; @@ --(const SEXP*) STRING_PTR(E) +-(const T*) STRING_PTR(E) +STRING_PTR_RO(E) From 9f71ddcef639c6e798d5881e0b3c297ca000da8a Mon Sep 17 00:00:00 2001 From: aitap Date: Sun, 25 Jan 2026 08:10:59 +0000 Subject: [PATCH 12/15] Apply suggestions from code review Co-authored-by: Michael Chirico --- .ci/linters/cocci/use_ro_accessors.cocci | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.ci/linters/cocci/use_ro_accessors.cocci b/.ci/linters/cocci/use_ro_accessors.cocci index 753317055c..018c1eb719 100644 --- a/.ci/linters/cocci/use_ro_accessors.cocci +++ b/.ci/linters/cocci/use_ro_accessors.cocci @@ -1,3 +1,4 @@ +/* Ensure _RO accessors are used for assignment to 'const' variables */ @@ type T; const T *variable; @@ -38,6 +39,7 @@ expression E; - variable = LOGICAL(E) + variable = LOGICAL_RO(E) +/* Just use _RO accessors directly instead of 'const' casting the writeable one */ @@ expression E; @@ From 5121ef57fbe71e18f4b20a5c9c7290cca705fde3 Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sun, 25 Jan 2026 11:06:26 +0300 Subject: [PATCH 13/15] Revert "cocci_linter.R: fix zero-length error reports" This reverts commit 70d37da5a8548d67337ebade6e8a25fc64bdf9df. --- .ci/linters/c/cocci_linter.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/linters/c/cocci_linter.R b/.ci/linters/c/cocci_linter.R index 9c5998cb85..a2502f43b9 100644 --- a/.ci/linters/c/cocci_linter.R +++ b/.ci/linters/c/cocci_linter.R @@ -10,12 +10,12 @@ cocci_linter = if (!nzchar(Sys.which("spatch"))) function(...) {} else function( stdout = TRUE, stderr = FALSE ) if (length(out) > 0) { - cat(sprintf("In file '%s', Coccinelle linter '%s' located the following problems:\n", c_obj$c_obj, spfile)) + cat(sprintf("In file '%s', Coccinelle linter '%s' located the following problems:\n", c_obj$path, spfile)) writeLines(out) bad = TRUE } if (!is.null(status <- attr(out, 'status'))) { - cat(sprintf("While working on file '%s', Coccinelle linter '%s' failed with exit code %d:\n", c_obj$c_obj, spfile, status)) + cat(sprintf("While working on file '%s', Coccinelle linter '%s' failed with exit code %d:\n", c_obj$path, spfile, status)) bad = TRUE } } From 1de948cfb41c5d59cc3c9165de7d28c74b40ba34 Mon Sep 17 00:00:00 2001 From: Ivan K Date: Sun, 25 Jan 2026 11:12:58 +0300 Subject: [PATCH 14/15] Fix the file path list element Setting $c_obj instead of $path caused the error messages to be zero-length due to the latter being returned as NULL. Co-Authored-By: Michael Chirico Co-Authored-By: Benjamin Schwendinger <52290390+ben-schwen@users.noreply.github.com> --- .ci/linters/c/00preprocess.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/linters/c/00preprocess.R b/.ci/linters/c/00preprocess.R index 046db97646..3cd49144d5 100644 --- a/.ci/linters/c/00preprocess.R +++ b/.ci/linters/c/00preprocess.R @@ -1,5 +1,5 @@ .preprocess = function (f) list( - c_obj = f, lines = readLines(f), + path = f, lines = readLines(f), preprocessed = system2( "gcc", shQuote(c("-fpreprocessed", "-E", f)), stdout = TRUE, stderr = FALSE From 07aae7a8221f6b46c396f91e0afe24ecbbf1fd37 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 25 Jan 2026 11:18:08 +0300 Subject: [PATCH 15/15] Reorder checks according to SEXPTYPE order --- .ci/linters/cocci/use_ro_accessors.cocci | 39 ++++++++++++------------ 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/.ci/linters/cocci/use_ro_accessors.cocci b/.ci/linters/cocci/use_ro_accessors.cocci index 018c1eb719..d21a42fc5a 100644 --- a/.ci/linters/cocci/use_ro_accessors.cocci +++ b/.ci/linters/cocci/use_ro_accessors.cocci @@ -4,67 +4,66 @@ type T; const T *variable; expression E; @@ -- variable = REAL(E) -+ variable = REAL_RO(E) +- variable = RAW(E) ++ variable = RAW_RO(E) @@ type T; const T *variable; expression E; @@ -- variable = INTEGER(E) -+ variable = INTEGER_RO(E) +- variable = LOGICAL(E) ++ variable = LOGICAL_RO(E) @@ type T; const T *variable; expression E; @@ -- variable = COMPLEX(E) -+ variable = COMPLEX_RO(E) +- variable = INTEGER(E) ++ variable = INTEGER_RO(E) @@ type T; const T *variable; expression E; @@ -- variable = RAW(E) -+ variable = RAW_RO(E) +- variable = REAL(E) ++ variable = REAL_RO(E) @@ type T; const T *variable; expression E; @@ -- variable = LOGICAL(E) -+ variable = LOGICAL_RO(E) +- variable = COMPLEX(E) ++ variable = COMPLEX_RO(E) /* Just use _RO accessors directly instead of 'const' casting the writeable one */ @@ expression E; +type T; @@ --(const int*) INTEGER(E) -+INTEGER_RO(E) +-(const T*) RAW(E) ++RAW_RO(E) @@ expression E; @@ --(const double*) REAL(E) -+REAL_RO(E) +-(const int*) LOGICAL(E) ++LOGICAL_RO(E) @@ expression E; @@ --(const int*) LOGICAL(E) -+LOGICAL_RO(E) +-(const int*) INTEGER(E) ++INTEGER_RO(E) @@ expression E; -type T; @@ --(const T*) RAW(E) -+RAW_RO(E) - +-(const double*) REAL(E) ++REAL_RO(E) @@ expression E; type T;