New compiler warnings about modifying 'const' object (STRING_PTR_RO)

[MichaelChirico]

data.table/src/coalesce.c

Line 159 in a599557

valP[k++] = STRING_PTR_RO(item);

data.table/src/chmatch.c

Line 43 in a599557

xd = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;

The compiler (on GitHub codespaces) warns at these two places:

chmatch.c: In function ‘chmatchMain’:
gcc -I"/usr/local/lib/R/include" -DNDEBUG   -I/usr/local/include    -fpic  -fopenmp -std=c11 -O3 -pipe -Wall -pedantic -Wstrict-prototypes -isystem /usr/share/R/include  -fno-common -c subset.c -o subset.o
coalesce.c: In function ‘coalesce’:
chmatch.c:43:8: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   43 |     xd = STRING_PTR_RO(PROTECT(coerceUtf8IfNeeded(x))); nprotect++;
      |        ^
gcc -I"/usr/local/lib/R/include" -DNDEBUG   -I/usr/local/include    -fpic  -fopenmp -std=c11 -O3 -pipe -Wall -pedantic -Wstrict-prototypes -isystem /usr/share/R/include  -fno-common -c transpose.c -o transpose.o
coalesce.c:159:17: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  159 |       valP[k++] = STRING_PTR_RO(item);
      |                 ^

The chmatch.c one is easy to fix (by instantiating xd as const), I'm less clear on how to fix the coalesce issue.

[aitap]

For coalesce, pepper a lot more consts as a pinky promise to the compiler that the buffer contents will not be modified?

diff --git a/src/coalesce.c b/src/coalesce.c
index c6e18da9..5853fac5 100644
--- a/src/coalesce.c
+++ b/src/coalesce.c
@@ -53,7 +53,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
     first = PROTECT(copyAsPlain(first)); nprotect++;
     if (verbose) Rprintf(_("coalesce copied first item (inplace=FALSE)\n"));
   }
-  void **valP = (void **)R_alloc(nval, sizeof(void *));
+  const void **valP = (const void **)R_alloc(nval, sizeof(const void *));
   switch(TYPEOF(first)) {
   case LGLSXP:
   case INTSXP: {
@@ -66,14 +66,14 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
         finalVal = tt;
         break;  // stop early on the first singleton that is not NA; minimizes deepest loop body below
       }
-      valP[k++] = INTEGER(item);
+      valP[k++] = (const void*)INTEGER(item);
     }
     const bool final=(finalVal!=NA_INTEGER);
     #pragma omp parallel for num_threads(getDTthreads(nrow, true))
     for (int i=0; i<nrow; ++i) {
       int val = xP[i];
       if (val!=NA_INTEGER) continue;
-      int j=0; while (val==NA_INTEGER && j<k) val=((int *)valP[j++])[i];  // k is the number of non-singletons
+      int j=0; while (val==NA_INTEGER && j<k) val=((const int *)valP[j++])[i];  // k is the number of non-singletons
       if (val!=NA_INTEGER) xP[i]=val; else if (final) xP[i]=finalVal;
     }
   } break;
@@ -89,14 +89,14 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
           finalVal = tt;
           break;
         }
-        valP[k++] = REAL(item);
+        valP[k++] = (const void *)REAL(item);
       }
       const bool final = (finalVal!=NA_INTEGER64);
       #pragma omp parallel for num_threads(getDTthreads(nrow, true))
       for (int i=0; i<nrow; ++i) {
         int64_t val=xP[i];
         if (val!=NA_INTEGER64) continue;
-        int j=0; while (val==NA_INTEGER64 && j<k) val=((int64_t *)valP[j++])[i];
+        int j=0; while (val==NA_INTEGER64 && j<k) val=((const int64_t *)valP[j++])[i];
         if (val!=NA_INTEGER64) xP[i]=val; else if (final) xP[i]=finalVal;
       }
     } else {
@@ -110,14 +110,14 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
           finalVal = tt;
           break;
         }
-        valP[k++] = REAL(item);
+        valP[k++] = (const void *)REAL(item);
       }
       const bool final = !ISNAN(finalVal);
       #pragma omp parallel for num_threads(getDTthreads(nrow, true))
       for (int i=0; i<nrow; ++i) {
         double val=xP[i];
         if (!ISNAN(val)) continue;
-        int j=0; while (ISNAN(val) && j<k) val=((double *)valP[j++])[i];
+        int j=0; while (ISNAN(val) && j<k) val=((const double *)valP[j++])[i];
         if (!ISNAN(val)) xP[i]=val; else if (final) xP[i]=finalVal;
       }
     }
@@ -133,14 +133,14 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
         finalVal = tt;
         break;
       }
-      valP[k++] = COMPLEX(item);
+      valP[k++] = (const void *)COMPLEX(item);
     }
     const bool final = !ISNAN(finalVal.r) && !ISNAN(finalVal.i);
     #pragma omp parallel for num_threads(getDTthreads(nrow, true))
     for (int i=0; i<nrow; ++i) {
       Rcomplex val=xP[i];
       if (!ISNAN(val.r) && !ISNAN(val.i)) continue;
-      int j=0; while (ISNAN(val.r) && ISNAN(val.i) && j<k) val=((Rcomplex *)valP[j++])[i];
+      int j=0; while (ISNAN(val.r) && ISNAN(val.i) && j<k) val=((const Rcomplex *)valP[j++])[i];
       if (!ISNAN(val.r) || !ISNAN(val.i)) xP[i]=val; else if (final) xP[i]=finalVal;
     }
   } break;
@@ -156,13 +156,13 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
         finalVal = tt;
         break;
       }
-      valP[k++] = STRING_PTR_RO(item);
+      valP[k++] = (const void *)STRING_PTR_RO(item);
     }
     const bool final = (finalVal!=NA_STRING);
     for (int i=0; i<nrow; ++i) {
       SEXP val = xP[i];
       if (val!=NA_STRING) continue;
-      int j=0; while (val==NA_STRING && j<k) val=((SEXP *)valP[j++])[i];
+      int j=0; while (val==NA_STRING && j<k) val=((const SEXP *)valP[j++])[i];
       if (val!=NA_STRING) SET_STRING_ELT(first, i, val); else if (final) SET_STRING_ELT(first, i, finalVal);
     }
   } break;

Alternatively, the warning will be silenced even by a single cast to void *, even though it means later laundering a pointer that was given to us as a const SEXP * back into a writable SEXP *:

diff --git a/src/coalesce.c b/src/coalesce.c
index c6e18da9..616c2acf 100644
--- a/src/coalesce.c
+++ b/src/coalesce.c
@@ -156,7 +156,7 @@ SEXP coalesce(SEXP x, SEXP inplaceArg) {
         finalVal = tt;
         break;
       }
-      valP[k++] = STRING_PTR_RO(item);
+      valP[k++] = (void*)STRING_PTR_RO(item);
     }
     const bool final = (finalVal!=NA_STRING);
     for (int i=0; i<nrow; ++i) {