Use read-only accessors and const when possible (#7368)

* Changed many function calls to use read only variants when appropriate

* implemented improvements as requested

* minor update

Author: badasahog

src/fastmean.c

@@ -37,7 +37,7 @@ SEXP fastmean(SEXP args)
     tmp = CADDR(args);
     if (!isLogical(tmp) || LENGTH(tmp)!=1 || LOGICAL(tmp)[0]==NA_LOGICAL)
       error(_("%s should be TRUE or FALSE"), "narm");  // # nocov ; [.data.table should construct the .External call correctly
-    narm=LOGICAL(tmp)[0];
+    narm=LOGICAL_RO(tmp)[0];
   }
   PROTECT(ans = allocNAVector(REALSXP, 1));
   copyMostAttrib(x, ans);
@@ -50,8 +50,8 @@ SEXP fastmean(SEXP args)
     case LGLSXP:
     case INTSXP:
       for (int i=0; i<l; ++i) {
-        if(INTEGER(x)[i] == NA_INTEGER) continue;
-        s += INTEGER(x)[i];   // no under/overflow here, s is long double not integer
+        if(INTEGER_RO(x)[i] == NA_INTEGER) continue;
+        s += INTEGER_RO(x)[i];   // no under/overflow here, s is long double not integer
         n++;
       }
       if (n>0)
@@ -61,8 +61,8 @@ SEXP fastmean(SEXP args)
       break;
     case REALSXP:
       for (int i=0; i<l; ++i) {
-        if(ISNAN(REAL(x)[i])) continue;  // TO DO: could drop this line and let NA propagate?
-        s += REAL(x)[i];
+        if(ISNAN(REAL_RO(x)[i])) continue;  // TO DO: could drop this line and let NA propagate?
+        s += REAL_RO(x)[i];
         n++;
       }
       if (n==0) {
@@ -72,8 +72,8 @@ SEXP fastmean(SEXP args)
       s /= n;
       if(R_FINITE((double)s)) {
         for (int i=0; i<l; ++i) {
-          if(ISNAN(REAL(x)[i])) continue;
-          t += (REAL(x)[i] - s);
+          if(ISNAN(REAL_RO(x)[i])) continue;
+          t += (REAL_RO(x)[i] - s);
         }
         s += t/n;
       }
@@ -87,21 +87,21 @@ SEXP fastmean(SEXP args)
     case LGLSXP:
     case INTSXP:
       for (int i=0; i<l; ++i) {
-        if(INTEGER(x)[i] == NA_INTEGER) {UNPROTECT(1); return(ans);}
-        s += INTEGER(x)[i];
+        if(INTEGER_RO(x)[i] == NA_INTEGER) {UNPROTECT(1); return(ans);}
+        s += INTEGER_RO(x)[i];
       }
       REAL(ans)[0] = (double) (s/l);
       break;
     case REALSXP:
       for (int i=0; i<l; ++i) {
-        if(ISNAN(REAL(x)[i])) {UNPROTECT(1); return(ans);}
-        s += REAL(x)[i];
+        if(ISNAN(REAL_RO(x)[i])) {UNPROTECT(1); return(ans);}
+        s += REAL_RO(x)[i];
       }
       s /= l;
       if(R_FINITE((double)s)) {
         for (int i=0; i<l; ++i) {
           // no NA if got this far
-          t += (REAL(x)[i] - s);
+          t += (REAL_RO(x)[i] - s);
         }
         s += t/LENGTH(x);
       }

src/fwrite.c

@@ -437,7 +437,7 @@ void writePOSIXct(const void *col, int64_t row, char **pch)
     int m = ((x - xi) * 10000000); // 7th digit used to round up if 9
     m += (m % 10);  // 9 is numerical accuracy, 8 or less then we truncate to last microsecond
     m /= 10;
-    int carry = m / 1000000; // Need to know if we rounded up to a whole second
+    const int carry = m / 1000000; // Need to know if we rounded up to a whole second
     m -= carry * 1000000;
     xi += carry;
     if (xi >= 0) {

src/inrange.c

@@ -4,8 +4,8 @@
 SEXP inrange(SEXP ansArg, SEXP xoArg, SEXP startsArg, SEXP lenArg)
 {
   int *ans = INTEGER(ansArg);
-  const int *xo = INTEGER(xoArg);
-  const int *starts = INTEGER(startsArg), *len = INTEGER(lenArg);
+  const int *xo = INTEGER_RO(xoArg);
+  const int *starts = INTEGER_RO(startsArg), *len = INTEGER_RO(lenArg);
   const int n = length(startsArg), nxo = length(xoArg);
   for (int i = 0; i < n; i++) {
     for (int j = starts[i] - 1; j < starts[i] - 1 + len[i]; j++) {

src/programming.c

@@ -7,7 +7,7 @@ static void substitute_call_arg_names(SEXP expr, SEXP env)
   SEXP arg_names = getAttrib(expr, R_NamesSymbol);
   if (!isNull(arg_names)) {
     SEXP env_names = getAttrib(env, R_NamesSymbol);
-    int *imatches = INTEGER(PROTECT(chmatch(arg_names, env_names, 0)));
+    const int *imatches = INTEGER_RO(PROTECT(chmatch(arg_names, env_names, 0)));
     const SEXP *env_sub = SEXPPTR_RO(env);
     SEXP tmp = expr;
     for (int i = 0; i < length(arg_names); i++, tmp = CDR(tmp)) { // substitute call arg names

src/reorder.c

@@ -42,7 +42,7 @@ SEXP reorder(SEXP x, SEXP order)
   int nprotect = 0;
   if (ALTREP(order)) { order=PROTECT(copyAsPlain(order)); nprotect++; }  // TODO: if it's an ALTREP sequence some optimizations are possible rather than expand
 
-  const int *restrict idx = INTEGER(order);
+  const int *restrict idx = INTEGER_RO(order);
   int i=0;
   while (i<nrow && idx[i] == i+1) ++i;
   const int start=i;

src/subset.c

@@ -49,17 +49,17 @@ void subsetVectorRaw(SEXP ans, SEXP source, SEXP idx, const bool anyNA)
 
   switch(TYPEOF(source)) {
   case INTSXP: case LGLSXP: {
-    int *sp = INTEGER(source);
+    const int *sp = INTEGER_RO(source);
     int *ap = INTEGER(ans);
     PARLOOP(NA_INTEGER)
   } break;
   case REALSXP : {
     if (INHERITS(source, char_integer64)) {
-      int64_t *sp = (int64_t *)REAL(source);
+      const int64_t *sp = (int64_t *)REAL_RO(source);
       int64_t *ap = (int64_t *)REAL(ans);
       PARLOOP(INT64_MIN)
     } else {
-      double *sp = REAL(source);
+      const double *sp = REAL_RO(source);
       double *ap = REAL(ans);
       PARLOOP(NA_REAL)
     }
@@ -88,12 +88,12 @@ void subsetVectorRaw(SEXP ans, SEXP source, SEXP idx, const bool anyNA)
     }
   } break;
   case CPLXSXP : {
-    Rcomplex *sp = COMPLEX(source);
+    const Rcomplex *sp = COMPLEX_RO(source);
     Rcomplex *ap = COMPLEX(ans);
     PARLOOP(NA_CPLX)
   } break;
   case RAWSXP : {
-    Rbyte *sp = RAW(source);
+    const Rbyte *sp = RAW_RO(source);
     Rbyte *ap = RAW(ans);
     PARLOOP(0)
   } break;

src/transpose.c

@@ -9,18 +9,18 @@ SEXP transpose(SEXP l, SEXP fill, SEXP ignoreArg, SEXP keepNamesArg, SEXP listCo
     error(_("l must be a list."));
   if (!length(l))
     return(copyAsPlain(l));
-  if (!isLogical(ignoreArg) || LOGICAL(ignoreArg)[0] == NA_LOGICAL)
+  if (!isLogical(ignoreArg) || LOGICAL_RO(ignoreArg)[0] == NA_LOGICAL)
     error(_("ignore.empty should be logical TRUE/FALSE."));
-  bool ignore = LOGICAL(ignoreArg)[0];
+  const bool ignore = LOGICAL_RO(ignoreArg)[0];
   if (!(isNull(keepNamesArg) || (isString(keepNamesArg) && LENGTH(keepNamesArg) == 1)))
     error(_("keep.names should be either NULL, or the name of the first column of the result in which to place the names of the input"));
-  bool rn = !isNull(keepNamesArg);
+  const bool rn = !isNull(keepNamesArg);
   if (length(fill) != 1)
     error(_("fill must be a length 1 vector, such as the default NA"));
-  R_len_t ln = LENGTH(l);
+  const R_len_t ln = LENGTH(l);
   if (!IS_TRUE_OR_FALSE(listColsArg))
     error(_("list.cols should be logical TRUE/FALSE."));
-  bool listCol = LOGICAL(listColsArg)[0];
+  const bool listCol = LOGICAL_RO(listColsArg)[0];
 
   // preprocessing
   int maxlen = 0, zerolen = 0;
@@ -39,7 +39,7 @@ SEXP transpose(SEXP l, SEXP fill, SEXP ignoreArg, SEXP keepNamesArg, SEXP listCo
   if (listCol) maxtype = VECSXP; // need to keep preprocessing for zerolen
   fill = PROTECT(coerceVector(fill, maxtype)); nprotect++;
   SEXP ans = PROTECT(allocVector(VECSXP, maxlen + rn)); nprotect++;
-  int anslen = (ignore) ? (ln - zerolen) : ln;
+  const int anslen = (ignore) ? (ln - zerolen) : ln;
   if (rn) {
     SEXP tt;
     SET_VECTOR_ELT(ans, 0, tt = allocVector(STRSXP, anslen));
@@ -61,22 +61,22 @@ SEXP transpose(SEXP l, SEXP fill, SEXP ignoreArg, SEXP keepNamesArg, SEXP listCo
     } else PROTECT(li); // extra PROTECT just to help rchk by avoiding two counter variables
     switch (maxtype) {
     case LGLSXP: {
-      const int *ili = LOGICAL(li);
-      const int ifill = LOGICAL(fill)[0];
+      const int *ili = LOGICAL_RO(li);
+      const int ifill = LOGICAL_RO(fill)[0];
       for (int j = 0; j < maxlen; j++) {
         LOGICAL(ansp[j + rn])[k] = j < len ? ili[j] : ifill;
       }
     } break;
     case INTSXP: {
-      const int *ili = INTEGER(li);
-      const int ifill = INTEGER(fill)[0];
+      const int *ili = INTEGER_RO(li);
+      const int ifill = INTEGER_RO(fill)[0];
       for (int j = 0; j < maxlen; j++) {
         INTEGER(ansp[j + rn])[k] = j < len ? ili[j] : ifill;
       }
     } break;
     case REALSXP: {
-      const double *dli = REAL(li);
-      const double dfill = REAL(fill)[0];
+      const double *dli = REAL_RO(li);
+      const double dfill = REAL_RO(fill)[0];
       for (int j = 0; j < maxlen; j++) {
         REAL(ansp[j + rn])[k] = j < len ? dli[j] : dfill;
       }

src/types.c

@@ -73,7 +73,7 @@ SEXP testMsgR(SEXP status, SEXP x, SEXP k) {
     internal_error(__func__, "status, nx, nk must be integer"); // # nocov
   int protecti = 0;
   const bool verbose = GetVerbose();
-  int istatus = INTEGER(status)[0], nx = INTEGER(x)[0], nk = INTEGER(k)[0];
+  int istatus = INTEGER_RO(status)[0], nx = INTEGER_RO(x)[0], nk = INTEGER_RO(k)[0];
 
   // TODO below chunk into allocAnsList helper - not easy for variable length of inner vectors
   SEXP ans = PROTECT(allocVector(VECSXP, nk * nx)); protecti++;