From c9a036552ddd809ca0b49f5ef73d469b8f17a621 Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Thu, 25 Jul 2024 22:27:15 -0700
Subject: [PATCH 1/3] Move to STRING_PTR_RO in all sure-fire 'const' cases

---
 src/assign.c    | 20 ++++++++++----------
 src/between.c   |  6 +++---
 src/bmerge.c    |  4 ++--
 src/chmatch.c   |  2 +-
 src/cj.c        |  2 +-
 src/fifelse.c   | 10 +++++-----
 src/fmelt.c     |  6 +++---
 src/forder.c    | 14 +++++++-------
 src/frank.c     |  4 ++--
 src/fwriteR.c   |  2 +-
 src/gsumm.c     |  8 ++++----
 src/rbindlist.c | 14 +++++++-------
 src/uniqlist.c  |  4 ++--
 src/utils.c     |  8 ++++----
 14 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/src/assign.c b/src/assign.c
index d937601eb..b1623875e 100644
--- a/src/assign.c
+++ b/src/assign.c
@@ -799,7 +799,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
       if (sourceIsFactor) { sourceLevels=PROTECT(getAttrib(source, R_LevelsSymbol)); protecti++; }
       if (!sourceIsFactor || !R_compute_identical(sourceLevels, targetLevels, 0)) {  // !sourceIsFactor for test 2115.6
         const int nTargetLevels=length(targetLevels), nSourceLevels=length(sourceLevels);
-        const SEXP *targetLevelsD=STRING_PTR(targetLevels), *sourceLevelsD=STRING_PTR(sourceLevels);
+        const SEXP *targetLevelsD=STRING_PTR_RO(targetLevels), *sourceLevelsD=STRING_PTR_RO(sourceLevels);
         SEXP newSource = PROTECT(allocVector(INTSXP, length(source))); protecti++;
         savetl_init();
         for (int k=0; k<nTargetLevels; ++k) {
@@ -835,7 +835,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
             newSourceD[i] = val==NA_INTEGER ? NA_INTEGER : -TRUELENGTH(sourceLevelsD[val-1]); // retains NA factor levels here via TL(NA_STRING); e.g. ordered factor
           }
         } else {
-          const SEXP *sourceD = STRING_PTR(source);
+          const SEXP *sourceD = STRING_PTR_RO(source);
           for (int i=0; i<nSource; ++i) {  // convert source integers to refer to target levels
             const SEXP val = sourceD[i];
             newSourceD[i] = val==NA_STRING ? NA_INTEGER : -TRUELENGTH(val);
@@ -1103,7 +1103,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
   } break;
   case STRSXP :
     if (sourceIsFactor) {
-      const SEXP *ld = STRING_PTR(PROTECT(getAttrib(source, R_LevelsSymbol))); protecti++;
+      const SEXP *ld = STRING_PTR_RO(PROTECT(getAttrib(source, R_LevelsSymbol))); protecti++;
       BODY(int, INTEGER, SEXP, val==NA_INTEGER ? NA_STRING : ld[val-1],  SET_STRING_ELT(target, off+i, cval))
     } else {
       if (!isString(source)) {
@@ -1125,7 +1125,7 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
           source = PROTECT(coerceVector(source, STRSXP)); protecti++;
         }
       }
-      BODY(SEXP, STRING_PTR, SEXP, val,  SET_STRING_ELT(target, off+i, cval))
+      BODY(SEXP, STRING_PTR_RO, SEXP, val,  SET_STRING_ELT(target, off+i, cval))
     }
   case VECSXP :
   case EXPRSXP : {  // #546 #4350
@@ -1140,12 +1140,12 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con
       // the UNPROTECT can be at the end of the CAST before the SET_VECTOR_ELT, because SET_VECTOR_ELT will protect it and there's no other code in between
       // the PROTECT is now needed because of the call to LOGICAL() which could feasibly gc inside it.
       // copyMostAttrib is inside CAST so as to be outside loop.  See the history in #4350 and its follow up
-      case RAWSXP:  BODY(Rbyte,    RAW,        SEXP, PROTECT(allocVector(RAWSXP, 1));RAW(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),             SET_VECTOR_ELT(target,off+i,cval))
-      case LGLSXP:  BODY(int,      LOGICAL,    SEXP, PROTECT(allocVector(LGLSXP, 1));LOGICAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),         SET_VECTOR_ELT(target,off+i,cval))
-      case INTSXP:  BODY(int,      INTEGER,    SEXP, PROTECT(allocVector(INTSXP, 1));INTEGER(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),         SET_VECTOR_ELT(target,off+i,cval))
-      case REALSXP: BODY(double,   REAL,       SEXP, PROTECT(allocVector(REALSXP, 1));REAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),           SET_VECTOR_ELT(target,off+i,cval))
-      case CPLXSXP: BODY(Rcomplex, COMPLEX,    SEXP, PROTECT(allocVector(CPLXSXP, 1));COMPLEX(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),        SET_VECTOR_ELT(target,off+i,cval))
-      case STRSXP:  BODY(SEXP,     STRING_PTR, SEXP, PROTECT(allocVector(STRSXP, 1));SET_STRING_ELT(cval, 0, val);copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
+      case RAWSXP:  BODY(Rbyte,    RAW,           SEXP, PROTECT(allocVector(RAWSXP, 1));RAW(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),             SET_VECTOR_ELT(target,off+i,cval))
+      case LGLSXP:  BODY(int,      LOGICAL,       SEXP, PROTECT(allocVector(LGLSXP, 1));LOGICAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),         SET_VECTOR_ELT(target,off+i,cval))
+      case INTSXP:  BODY(int,      INTEGER,       SEXP, PROTECT(allocVector(INTSXP, 1));INTEGER(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),         SET_VECTOR_ELT(target,off+i,cval))
+      case REALSXP: BODY(double,   REAL,          SEXP, PROTECT(allocVector(REALSXP, 1));REAL(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),           SET_VECTOR_ELT(target,off+i,cval))
+      case CPLXSXP: BODY(Rcomplex, COMPLEX,       SEXP, PROTECT(allocVector(CPLXSXP, 1));COMPLEX(cval)[0]=val;copyMostAttrib(source,cval);UNPROTECT(1),        SET_VECTOR_ELT(target,off+i,cval))
+      case STRSXP:  BODY(SEXP,     STRING_PTR_RO, SEXP, PROTECT(allocVector(STRSXP, 1));SET_STRING_ELT(cval, 0, val);copyMostAttrib(source,cval);UNPROTECT(1), SET_VECTOR_ELT(target,off+i,cval))
       case VECSXP:
       case EXPRSXP: BODY(SEXP,     SEXPPTR_RO, SEXP, val,                                                                                                   SET_VECTOR_ELT(target,off+i,cval))
       default: COERCE_ERROR("list");
diff --git a/src/between.c b/src/between.c
index 818fa0cbb..56f201fe2 100644
--- a/src/between.c
+++ b/src/between.c
@@ -160,9 +160,9 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAboundsArg, S
     break;
 
   case STRSXP: {
-    const SEXP *lp = STRING_PTR(lower);
-    const SEXP *up = STRING_PTR(upper);
-    const SEXP *xp = STRING_PTR(x);
+    const SEXP *lp = STRING_PTR_RO(lower);
+    const SEXP *up = STRING_PTR_RO(upper);
+    const SEXP *xp = STRING_PTR_RO(x);
     #define LCMP (strcmp(CHAR(ENC2UTF8(l)),CHAR(ENC2UTF8(elem)))<=-open)
     #define UCMP (strcmp(CHAR(ENC2UTF8(elem)),CHAR(ENC2UTF8(u)))<=-open)
     // TODO if all ascii can be parallel, otherwise ENC2UTF8 could allocate
diff --git a/src/bmerge.c b/src/bmerge.c
index 6cb20c683..351baff28 100644
--- a/src/bmerge.c
+++ b/src/bmerge.c
@@ -340,8 +340,8 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg
   case STRSXP : {
     // op[col]==EQ checked up front to avoid an if() here and non-thread-safe error()
     // not sure why non-EQ is not supported for STRSXP though as it seems straightforward (StrCmp returns sign to indicate GT or LT)
-    const SEXP *icv = STRING_PTR(ic);
-    const SEXP *xcv = STRING_PTR(xc);
+    const SEXP *icv = STRING_PTR_RO(ic);
+    const SEXP *xcv = STRING_PTR_RO(xc);
     const SEXP ival = ENC2UTF8(icv[ir]);
     #undef ISNAT
     #undef WRAP
diff --git a/src/chmatch.c b/src/chmatch.c
index 878699e6a..8f7855810 100644
--- a/src/chmatch.c
+++ b/src/chmatch.c
@@ -42,7 +42,7 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
   } else {
     xd = STRING_PTR(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
   }
-  const SEXP *td = STRING_PTR(PROTECT(coerceUtf8IfNeeded(table))); nprotect++;
+  const SEXP *td = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(table))); nprotect++;
   if (xlen==1) {
     ansd[0] = nomatch;
     for (int i=0; i<tablelen; ++i) {
diff --git a/src/cj.c b/src/cj.c
index 8e5b7e28e..0f9c940e5 100644
--- a/src/cj.c
+++ b/src/cj.c
@@ -62,7 +62,7 @@ SEXP cj(SEXP base_list) {
       }
     } break;
     case STRSXP: {
-      const SEXP *sourceP = STRING_PTR(source);
+      const SEXP *sourceP = STRING_PTR_RO(source);
       int start = 0;
       for (int i=0; i<ncopy; ++i) {
         for (int j=0; j<thislen; ++j) {
diff --git a/src/fifelse.c b/src/fifelse.c
index b5b06200c..72b7f2c01 100644
--- a/src/fifelse.c
+++ b/src/fifelse.c
@@ -146,9 +146,9 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b, SEXP na) {
     }
   } break;
   case STRSXP : {
-    const SEXP *restrict pa = na_a ? NULL : STRING_PTR(a);
-    const SEXP *restrict pb = na_b ? NULL : STRING_PTR(b);
-    const SEXP *restrict pna = na_n ? NULL : STRING_PTR(na);
+    const SEXP *restrict pa = na_a ? NULL : STRING_PTR_RO(a);
+    const SEXP *restrict pb = na_b ? NULL : STRING_PTR_RO(b);
+    const SEXP *restrict pna = na_n ? NULL : STRING_PTR_RO(na);
     const SEXP na = NA_STRING;
     for (int64_t i=0; i<len0; ++i) {
       SET_STRING_ELT(
@@ -358,8 +358,8 @@ SEXP fcaseR(SEXP na, SEXP rho, SEXP args) {
       }
     } break;
     case STRSXP: {
-      const SEXP *restrict pouts = STRING_PTR(outs);
-      const SEXP pna = nonna ? STRING_PTR(na)[0] : NA_STRING;
+      const SEXP *restrict pouts = STRING_PTR_RO(outs);
+      const SEXP pna = nonna ? STRING_PTR_RO(na)[0] : NA_STRING;
       for (int64_t j=0; j<len2; ++j) {
         const int64_t idx = imask ? j : p[j];
         if (pcons[idx]==1) {
diff --git a/src/fmelt.c b/src/fmelt.c
index 456c4f31c..51f4fbbb8 100644
--- a/src/fmelt.c
+++ b/src/fmelt.c
@@ -402,13 +402,13 @@ static SEXP combineFactorLevels(SEXP factorLevels, SEXP target, int * factorType
   SEXP ans = PROTECT(allocVector(INTSXP, nrow));
   SEXP *levelsRaw = (SEXP *)R_alloc(maxlevels, sizeof(SEXP));  // allocate for worst-case all-unique levels
   int *ansd = INTEGER(ans);
-  const SEXP *targetd = STRING_PTR(target);
+  const SEXP *targetd = STRING_PTR_RO(target);
   savetl_init();
   // no alloc or any fail point until savetl_end()
   int nlevel=0;
   for (int i=0; i<nitem; ++i) {
     const SEXP this = VECTOR_ELT(factorLevels, i);
-    const SEXP *thisd = STRING_PTR(this);
+    const SEXP *thisd = STRING_PTR_RO(this);
     const int thisn = length(this);
     for (int k=0; k<thisn; ++k) {
       SEXP s = thisd[k];
@@ -745,7 +745,7 @@ SEXP getidcols(SEXP DT, SEXP dtnames, Rboolean verbose, struct processData *data
           counter += thislen;
         }
       } else {
-        const SEXP *s = STRING_PTR(thiscol);  // to reduce overhead of STRING_ELT() inside loop below. Read-only hence const.
+        const SEXP *s = STRING_PTR_RO(thiscol);  // to reduce overhead of STRING_ELT() inside loop below. Read-only hence const.
         for (int j=0; j<data->lmax; ++j) {
           for (int k=0; k<data->nrow; ++k) {
             SET_STRING_ELT(target, j*data->nrow + k, s[k]);
diff --git a/src/forder.c b/src/forder.c
index 70744d091..7226f7e45 100644
--- a/src/forder.c
+++ b/src/forder.c
@@ -277,7 +277,7 @@ static void cradix(SEXP *x, int n)
   free(cradix_xtmp);   cradix_xtmp=NULL;
 }
 
-static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int *out_na_count)
+static void range_str(const SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int *out_na_count)
 // group numbers are left in truelength to be fetched by WRITE_KEY
 {
   int na_count=0;
@@ -323,7 +323,7 @@ static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int
     SEXP *ustr3 = (SEXP *)malloc(ustr_n * sizeof(SEXP));
     if (!ustr3)
       STOP(_("Failed to alloc ustr3 when converting strings to UTF8"));  // # nocov
-    memcpy(ustr3, STRING_PTR(ustr2), ustr_n*sizeof(SEXP));
+    memcpy(ustr3, STRING_PTR_RO(ustr2), ustr_n*sizeof(SEXP));
     // need to reset ustr_maxlen because we need ustr_maxlen for utf8 strings
     ustr_maxlen = 0;
     for (int i=0; i<ustr_n; i++) {
@@ -342,7 +342,7 @@ static void range_str(SEXP *x, int n, uint64_t *out_min, uint64_t *out_max, int
     int *tl = (int *)malloc(ustr_n * sizeof(int));
     if (!tl)
       STOP(_("Failed to alloc tl when converting strings to UTF8"));  // # nocov
-    SEXP *tt = STRING_PTR(ustr2);
+    const SEXP *tt = STRING_PTR_RO(ustr2);
     for (int i=0; i<ustr_n; i++) tl[i] = TRUELENGTH(tt[i]);   // fetches the o in ustr3 into tl which is ordered by ustr
     for (int i=0; i<ustr_n; i++) SET_TRUELENGTH(ustr3[i], 0);    // reset to 0 tl of the UTF8 (and possibly non-UTF in ustr too)
     for (int i=0; i<ustr_n; i++) SET_TRUELENGTH(ustr[i], tl[i]); // put back the o into ustr's tl
@@ -552,7 +552,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
       break;
     case STRSXP :
       // need2utf8 now happens inside range_str on the uniques
-      range_str(STRING_PTR(x), nrow, &min, &max, &na_count);
+      range_str(STRING_PTR_RO(x), nrow, &min, &max, &na_count);
       break;
     default:
       STOP(_("Column %d passed to [f]order is type '%s', not yet supported."), col+1, type2char(TYPEOF(x)));
@@ -693,7 +693,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
       }
       break;
     case STRSXP : {
-      SEXP *xd = STRING_PTR(x);
+      const SEXP *xd = STRING_PTR_RO(x);
       #pragma omp parallel for num_threads(getDTthreads(nrow, true))
       for (int i=0; i<nrow; i++) {
         uint64_t elem=0;
@@ -1326,7 +1326,7 @@ SEXP issorted(SEXP x, SEXP by)
       }
       break;
     case STRSXP : {
-      SEXP *xd = STRING_PTR(x);
+      const SEXP *xd = STRING_PTR_RO(x);
       i = 0;
       while (i<n && xd[i]==NA_STRING) i++;
       if (i==n) break; // xd consists only of NA_STRING #5070
@@ -1369,7 +1369,7 @@ SEXP issorted(SEXP x, SEXP by)
       break;
     case STRSXP:
       types[j] = 3;
-      ptrs[j] = (const char *)STRING_PTR(col);
+      ptrs[j] = (const char *)STRING_PTR_RO(col);
       break;
     default:
       STOP(_("type '%s' is not yet supported"), type2char(TYPEOF(col)));  // # nocov
diff --git a/src/frank.c b/src/frank.c
index 7148bdd38..425f21450 100644
--- a/src/frank.c
+++ b/src/frank.c
@@ -34,7 +34,7 @@ SEXP dt_na(SEXP x, SEXP cols) {
     }
       break;
     case STRSXP: {
-      const SEXP *sv = STRING_PTR(v);
+      const SEXP *sv = STRING_PTR_RO(v);
       for (int j=0; j<n; ++j) ians[j] |= (sv[j] == NA_STRING);
     }
       break;
@@ -209,7 +209,7 @@ SEXP anyNA(SEXP x, SEXP cols) {
       while(j<n && iv[j]!=NA_INTEGER) j++;
     } break;
     case STRSXP: {
-      const SEXP *sv = STRING_PTR(v);
+      const SEXP *sv = STRING_PTR_RO(v);
       while (j<n && sv[j]!=NA_STRING) j++;
     } break;
     case REALSXP:
diff --git a/src/fwriteR.c b/src/fwriteR.c
index 061654759..c179ea2c8 100644
--- a/src/fwriteR.c
+++ b/src/fwriteR.c
@@ -44,7 +44,7 @@ int getMaxStringLen(const SEXP *col, const int64_t n) {
 int getMaxCategLen(SEXP col) {
   col = getAttrib(col, R_LevelsSymbol);
   if (!isString(col)) error(_("Internal error: col passed to getMaxCategLen is missing levels"));
-  return getMaxStringLen( STRING_PTR(col), LENGTH(col) );
+  return getMaxStringLen( STRING_PTR_RO(col), LENGTH(col) );
 }
 
 const char *getCategString(SEXP col, int64_t row) {
diff --git a/src/gsumm.c b/src/gsumm.c
index 16cc228a3..35f13683b 100644
--- a/src/gsumm.c
+++ b/src/gsumm.c
@@ -763,8 +763,8 @@ static SEXP gminmax(SEXP x, SEXP narm, const bool min)
     break;
   case STRSXP: {
     ans = PROTECT(allocVector(STRSXP, ngrp));
-    const SEXP *ansd = STRING_PTR(ans);
-    const SEXP *xd = STRING_PTR(x);
+    const SEXP *ansd = STRING_PTR_RO(ans);
+    const SEXP *xd = STRING_PTR_RO(x);
     if (!LOGICAL(narm)[0]) {
       const SEXP init = min ? char_maxString : R_BlankString;  // char_maxString == "\xFF\xFF..." in init.c
       for (int i=0; i<ngrp; ++i) SET_STRING_ELT(ans, i, init);
@@ -978,7 +978,7 @@ static SEXP gfirstlast(SEXP x, const bool first, const int w, const bool headw)
            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;
-  case STRSXP:  DO(SEXP, STRING_PTR, NA_STRING,                 SET_STRING_ELT(ans,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:
     error(_("Type '%s' is not supported by GForce head/tail/first/last/`[`. Either add the namespace prefix (e.g. utils::head(.)) or turn off GForce optimization using options(datatable.optimize=1)"), type2char(TYPEOF(x)));
@@ -1266,7 +1266,7 @@ SEXP gshift(SEXP x, SEXP nArg, SEXP fillArg, SEXP typeArg) {
       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 STRSXP: { SHIFT(SEXP, STRING_PTR,                          SET_STRING_ELT(tmp,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:
         error(_("Type '%s' is not supported by GForce gshift. Either add the namespace prefix (e.g. data.table::shift(.)) or turn off GForce optimization using options(datatable.optimize=1)"), type2char(TYPEOF(x)));
diff --git a/src/rbindlist.c b/src/rbindlist.c
index 7d7578cde..68c3166e1 100644
--- a/src/rbindlist.c
+++ b/src/rbindlist.c
@@ -80,7 +80,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor
       if (isNull(li) || !LENGTH(li)) continue;
       const SEXP cn = getAttrib(li, R_NamesSymbol);
       if (!length(cn)) continue;
-      const SEXP *cnp = STRING_PTR(cn);
+      const SEXP *cnp = STRING_PTR_RO(cn);
       for (int j=0; j<thisncol; j++) {
         SEXP s = cnp[j];
         if (TRUELENGTH(s)<0) continue;  // seen this name before
@@ -111,7 +111,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor
       if (thisncol==0) continue;
       const SEXP cn = getAttrib(li, R_NamesSymbol);
       if (!length(cn)) continue;
-      const SEXP *cnp = STRING_PTR(cn);
+      const SEXP *cnp = STRING_PTR_RO(cn);
       memset(counts, 0, nuniq*sizeof(int));
       for (int j=0; j<thisncol; j++) {
         SEXP s = cnp[j];
@@ -151,7 +151,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor
       if (!length(cn)) {
         for (int j=0; j<thisncol; j++) colMapRaw[i*ncol + j] = j;
       } else {
-        const SEXP *cnp = STRING_PTR(cn);
+        const SEXP *cnp = STRING_PTR_RO(cn);
         memset(counts, 0, nuniq*sizeof(int));
         for (int j=0; j<thisncol; j++) {
           SEXP s = cnp[j];
@@ -372,7 +372,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor
         //      c( a<c<b, c<b, 'c,b'   ) => a<c<b  because the regular factor/character items c and b exist in the ordered levels
         //      c( a<c<b, c<b, 'c,d'   ) => a<c<b<d  'd' from non-ordered item added on the end of longest ordered levels
         //      c( a<c<b, c<b<d<e )  => regular factor because this case isn't yet implemented. a<c<b<d<e would be possible in future (extending longest at the beginning or end)
-        const SEXP *sd = STRING_PTR(longestLevels);
+        const SEXP *sd = STRING_PTR_RO(longestLevels);
         nLevel = allocLevel = longestLen;
         levelsRaw = (SEXP *)malloc(nLevel * sizeof(SEXP));
         if (!levelsRaw) {
@@ -392,7 +392,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor
           SEXP thisCol = VECTOR_ELT(li, w);
           if (isOrdered(thisCol)) {
             SEXP levels = getAttrib(thisCol, R_LevelsSymbol);
-            const SEXP *levelsD = STRING_PTR(levels);
+            const SEXP *levelsD = STRING_PTR_RO(levels);
             const int n = length(levels);
             for (int k=0, last=0; k<n; ++k) {
               SEXP s = levelsD[k];
@@ -431,7 +431,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor
           SEXP thisCol = VECTOR_ELT(li, w);
           SEXP thisColStr = isFactor(thisCol) ? getAttrib(thisCol, R_LevelsSymbol) : (isString(thisCol) ? thisCol : VECTOR_ELT(coercedForFactor, i));
           const int n = length(thisColStr);
-          const SEXP *thisColStrD = STRING_PTR(thisColStr);  // D for data
+          const SEXP *thisColStrD = STRING_PTR_RO(thisColStr);  // D for data
           for (int k=0; k<n; ++k) {
             SEXP s = thisColStrD[k];
             if (s==NA_STRING ||             // remove NA from levels; test 1979 found by package emil when revdep testing 1.12.2 (#3473)
@@ -498,7 +498,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor
               }
             }
           } else {
-            const SEXP *sd = STRING_PTR(thisColStr);
+            const SEXP *sd = STRING_PTR_RO(thisColStr);
             if (length(thisCol)<=1) {
               const int val = (length(thisCol)==1 && sd[0]!=NA_STRING) ? -TRUELENGTH(sd[0]) : NA_INTEGER;
               for (int r=0; r<thisnrow; ++r) targetd[ansloc+r] = val;
diff --git a/src/uniqlist.c b/src/uniqlist.c
index 48da706a2..cb01bb131 100644
--- a/src/uniqlist.c
+++ b/src/uniqlist.c
@@ -66,7 +66,7 @@ SEXP uniqlist(SEXP l, SEXP order)
       }
     } break;
     case STRSXP : {
-      const SEXP *vd=STRING_PTR(v);
+      const SEXP *vd=STRING_PTR_RO(v);
       SEXP prev, elem;
       if (via_order) {
         COMPARE1_VIA_ORDER && ENC2UTF8(elem)!=ENC2UTF8(prev) COMPARE2   // but most of the time they are equal, so ENC2UTF8 doesn't need to be called
@@ -224,7 +224,7 @@ SEXP rleid(SEXP l, SEXP cols) {
       }
     } break;
     case STRSXP : {
-      const SEXP *jd = STRING_PTR(jcol);
+      const SEXP *jd = STRING_PTR_RO(jcol);
       for (R_xlen_t i=1; i<nrow; i++) {
         bool same = jd[i]==jd[i-1];
         ians[i] = (grp+=!same);
diff --git a/src/utils.c b/src/utils.c
index 1b162ea62..28f3e3d3f 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -75,7 +75,7 @@ bool allNA(SEXP x, bool errorForBadType) {
     return true;
   }
   case STRSXP: {
-    const SEXP *xd = STRING_PTR(x);
+    const SEXP *xd = STRING_PTR_RO(x);
     for (int i=0; i<n; ++i)    if (xd[i]!=NA_STRING) {
       return false;
     }
@@ -229,7 +229,7 @@ SEXP copyAsPlain(SEXP x) {
     memcpy(COMPLEX(ans), COMPLEX(x), n*sizeof(Rcomplex));
     break;
   case STRSXP: {
-    const SEXP *xp=STRING_PTR(x);                                // covered by as.character(as.hexmode(1:500)) after test 642
+    const SEXP *xp=STRING_PTR_RO(x);                              // covered by as.character(as.hexmode(1:500)) after test 642
     for (int64_t i=0; i<n; ++i) SET_STRING_ELT(ans, i, xp[i]);
   } break;
   case VECSXP: {
@@ -312,7 +312,7 @@ SEXP islockedR(SEXP DT) {
 
 bool need2utf8(SEXP x) {
   const int xlen = length(x);
-  SEXP *xd = STRING_PTR(x);
+  const SEXP *xd = STRING_PTR_RO(x);
   for (int i=0; i<xlen; i++) {
     if (NEED2UTF8(xd[i]))
       return(true);
@@ -325,7 +325,7 @@ SEXP coerceUtf8IfNeeded(SEXP x) {
     return(x);
   const int xlen = length(x);
   SEXP ans = PROTECT(allocVector(STRSXP, xlen));
-  SEXP *xd = STRING_PTR(x);
+  const SEXP *xd = STRING_PTR_RO(x);
   for (int i=0; i<xlen; i++) {
     SET_STRING_ELT(ans, i, ENC2UTF8(xd[i]));
   }

From f5dcdb1f2ee24100e06511e0869fd989ce1bec9e Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Thu, 25 Jul 2024 22:27:59 -0700
Subject: [PATCH 2/3] Remaining cases compile as well

---
 src/chmatch.c  | 4 ++--
 src/coalesce.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/chmatch.c b/src/chmatch.c
index 8f7855810..88333119b 100644
--- a/src/chmatch.c
+++ b/src/chmatch.c
@@ -10,7 +10,7 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
   if (TYPEOF(x) == SYMSXP) {
     if (xlen!=1)
       error(_("Internal error: length of SYMSXP is %d not 1"), xlen); // # nocov
-    sym = PRINTNAME(x);  // so we can do &sym to get a length 1 (const SEXP *)STRING_PTR(x) and save an alloc for coerce to STRSXP
+    sym = PRINTNAME(x);  // so we can do &sym to get a length 1 (const SEXP *)STRING_PTR_RO(x) and save an alloc for coerce to STRSXP
   } else if (!isString(x) && !isSymbol(x) && !isNull(x)) {
     if (chin && !isVectorAtomic(x)) {
       return ScalarLogical(FALSE);
@@ -40,7 +40,7 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
   if (isSymbol(x)) {
     xd = &sym;
   } else {
-    xd = STRING_PTR(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
+    xd = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
   }
   const SEXP *td = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(table))); nprotect++;
   if (xlen==1) {
diff --git a/src/coalesce.c b/src/coalesce.c
index b747991c0..0c72f11d5 100644
--- a/src/coalesce.c
+++ b/src/coalesce.c
@@ -141,7 +141,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
     }
   } break;
   case STRSXP: {
-    const SEXP *xP = STRING_PTR(first);
+    const SEXP *xP = STRING_PTR_RO(first);
     SEXP finalVal=NA_STRING;
     int k=0;
     for (int j=0; j<nval; ++j) {
@@ -152,7 +152,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
         finalVal = tt;
         break;
       }
-      valP[k++] = STRING_PTR(item);
+      valP[k++] = STRING_PTR_RO(item);
     }
     const bool final = (finalVal!=NA_STRING);
     for (int i=0; i<nrow; ++i) {

From f14ec4bc04e4b9a0d49c25fdd164a9c3a3ca03fe Mon Sep 17 00:00:00 2001
From: Michael Chirico <michaelchirico4@gmail.com>
Date: Thu, 25 Jul 2024 22:37:22 -0700
Subject: [PATCH 3/3] Conditionally define STRING_PTR_RO on 3.3.0<=R<3.5.0

---
 src/data.table.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/data.table.h b/src/data.table.h
index cd9e40efa..812ad5f97 100644
--- a/src/data.table.h
+++ b/src/data.table.h
@@ -12,6 +12,9 @@
 #endif
 #include <Rinternals.h>
 #define SEXPPTR_RO(x) ((const SEXP *)DATAPTR_RO(x))  // to avoid overhead of looped STRING_ELT and VECTOR_ELT
+#ifndef STRING_PTR_RO
+#define STRING_PTR_RO STRING_PTR
+#endif
 #include <stdint.h>    // for uint64_t rather than unsigned long long
 #include <stdbool.h>
 #include "types.h"