Windows warnings with gcc about %lld

.dev/CRAN_Release.cmd

@@ -66,6 +66,17 @@ grep "PROTECT_PTR" ./src/*.c
 # No use of long long, instead use int64_t. TODO
 # grep "long long" ./src/*.c
 
+// No use of llu, lld, zd or zu
+grep -nE "(llu|lld|zd|zu)" src/*.[hc]
+// Comment moved here from fread.c on 19 Nov 2019
+// [Moved from fread.c on 19 Nov 2019] On Windows variables of type `size_t` cannot be printed
+// with "%zu" in the `snprintf()` function. For those variables we used to cast them into
+// `unsigned long long int` before printing, and defined (llu) to make the cast shorter.
+// We're now observing warnings from gcc-8 with -Wformat-extra-args, #4062. So
+// now we're more strict and cast to [u]int64_t and use PRIu64/PRId64 from <inttypes.h>
+// In many cases the format specifier is passed to our own macro (e.g. DTPRINT) or to Rprintf(),
+// error() etc, and even if they don't call sprintf() now, they could in future.
+
 # No tabs in C or R code (sorry, Richard Hendricks)
 grep -P "\t" ./R/*.R
 grep -P "\t" ./src/*.c

NEWS.md

@@ -16,6 +16,8 @@
 
 2. One test needed to be adjusted to pass R-devel (R 4.0.0) which will soon have reference counting turned on, [#4058](https://github.com/Rdatatable/data.table/issues/4058). This motivated early release to CRAN because every day CRAN tests every package using the previous day's changes in R-devel; a much valued feature of the R ecosystem. It helps R-core if packages can pass changes in R-devel as soon as possible. Thanks to Luke Tierney for the notice, and for implementing reference counting which we look forward to very much.
 
+3. C internals have been standardized to use `PRI[u|d]64` to print `[u]int64_t`. This solves new warnings from `gcc-8` on Windows with `%lld`, [#4062](https://github.com/Rdatatable/data.table/issues/4062), in many cases already working around `snprintf` on Windows not supporting `%zu`. Release procedures have been augmented to prevent any internal use of `llu`, `lld`, `zu` or `zd`.
+
 
 # data.table [v1.12.6](https://github.com/Rdatatable/data.table/milestone/18?closed=1)  (18 Oct 2019)
 

src/assign.c

@@ -864,7 +864,7 @@ const char *memrecycle(SEXP target, SEXP where, int start, int len, SEXP source,
       const char *sType = sourceIsI64 ? "integer64" : type2char(TYPEOF(source));                                        \
       const char *tType = targetIsI64 ? "integer64" : type2char(TYPEOF(target));                                        \
       int n = snprintf(memrecycle_message, MSGSIZE,                                                                     \
-               FMT" (type '%s') at RHS position %d "TO" when assigning to type '%s'", val, sType, i+1, tType);          \
+            "%"FMT" (type '%s') at RHS position %d "TO" when assigning to type '%s'", val, sType, i+1, tType);          \
       if (colnum>0 && n>0 && n<MSGSIZE)                                                                                 \
         snprintf(memrecycle_message+n, MSGSIZE-n, " (column %d named '%s')", colnum, colname);                          \
       /* string returned so that rbindlist/dogroups can prefix it with which item of its list this refers to  */        \
@@ -876,28 +876,28 @@ const char *memrecycle(SEXP target, SEXP where, int start, int len, SEXP source,
     switch(TYPEOF(target)) {
     case LGLSXP:
       switch (TYPEOF(source)) {
-      case RAWSXP:  CHECK_RANGE(Rbyte, RAW,      val!=0 && val!=1,                                        "%d",   "taken as TRUE")
-      case INTSXP:  CHECK_RANGE(int, INTEGER,    val!=0 && val!=1 && val!=NA_INTEGER,                     "%d",   "taken as TRUE")
+      case RAWSXP:  CHECK_RANGE(Rbyte, RAW,    val!=0 && val!=1,                                        "d",    "taken as TRUE")
+      case INTSXP:  CHECK_RANGE(int, INTEGER,  val!=0 && val!=1 && val!=NA_INTEGER,                     "d",    "taken as TRUE")
       case REALSXP: if (sourceIsI64)
-                    CHECK_RANGE(long long, REAL, val!=0 && val!=1 && val!=NA_INTEGER64,                   "%lld", "taken as TRUE")
-              else  CHECK_RANGE(double, REAL,    !ISNAN(val) && val!=0.0 && val!=1.0,                     "%f",   "taken as TRUE")
+                    CHECK_RANGE(int64_t, REAL, val!=0 && val!=1 && val!=NA_INTEGER64,                   PRId64, "taken as TRUE")
+              else  CHECK_RANGE(double, REAL,  !ISNAN(val) && val!=0.0 && val!=1.0,                     "f",    "taken as TRUE")
       } break;
     case RAWSXP:
       switch (TYPEOF(source)) {
-      case INTSXP:  CHECK_RANGE(int, INTEGER,    val<0 || val>255,                                        "%d",   "taken as 0")
+      case INTSXP:  CHECK_RANGE(int, INTEGER,  val<0 || val>255,                                        "d",    "taken as 0")
       case REALSXP: if (sourceIsI64)
-                    CHECK_RANGE(long long, REAL, val<0 || val>255,                                        "%lld", "taken as 0")
-              else  CHECK_RANGE(double, REAL,    !R_FINITE(val) || val<0.0 || val>256.0 || (int)val!=val, "%f",   "either truncated (precision lost) or taken as 0")
+                    CHECK_RANGE(int64_t, REAL, val<0 || val>255,                                        PRId64, "taken as 0")
+              else  CHECK_RANGE(double, REAL,  !R_FINITE(val) || val<0.0 || val>256.0 || (int)val!=val, "f",    "either truncated (precision lost) or taken as 0")
       } break;
     case INTSXP:
       if (TYPEOF(source)==REALSXP) {
         if (sourceIsI64)
-                    CHECK_RANGE(long long, REAL, val!=NA_INTEGER64 && (val<=NA_INTEGER || val>INT_MAX),   "%lld",  "out-of-range (NA)")
-        else        CHECK_RANGE(double, REAL,    !ISNAN(val) && (!R_FINITE(val) || (int)val!=val),        "%f",    "truncated (precision lost)")
+                    CHECK_RANGE(int64_t, REAL, val!=NA_INTEGER64 && (val<=NA_INTEGER || val>INT_MAX),   PRId64,  "out-of-range (NA)")
+        else        CHECK_RANGE(double, REAL,  !ISNAN(val) && (!R_FINITE(val) || (int)val!=val),        "f",     "truncated (precision lost)")
       } break;
     case REALSXP:
       if (targetIsI64 && isReal(source) && !sourceIsI64) {
-                    CHECK_RANGE(double, REAL,    !ISNAN(val) && (!R_FINITE(val) || (int)val!=val),        "%f",    "truncated (precision lost)")
+                    CHECK_RANGE(double, REAL,  !ISNAN(val) && (!R_FINITE(val) || (int)val!=val),        "f",     "truncated (precision lost)")
       }
     }
   }

src/between.c

@@ -92,7 +92,7 @@ SEXP between(SEXP x, SEXP lower, SEXP upper, SEXP incbounds, SEXP NAboundsArg, S
       if (check) for (int i=0; i<longestBound; ++i) {
         const int64_t l=lp[i & lowMask], u=up[i & uppMask];
         if (l!=NA_INTEGER64 && u!=NA_INTEGER64 && l>u)
-          error("Item %d of lower (%lld) is greater than item %d of upper (%lld)", (i&lowMask)+1, l, (i&uppMask)+1, u);
+          error("Item %d of lower (%"PRId64") is greater than item %d of upper (%"PRId64")", (i&lowMask)+1, l, (i&uppMask)+1, u);
       }
       if (NAbounds) {
         #pragma omp parallel for num_threads(getDTthreads())

src/chmatch.c

@@ -57,7 +57,7 @@ static SEXP chmatchMain(SEXP x, SEXP table, int nomatch, bool chin, bool chmatch
       // # nocov start
       for (int i=0; i<tablelen; i++) SET_TRUELENGTH(td[i], 0);
       savetl_end();
-      error("Failed to allocate %lld bytes working memory in chmatchdup: length(table)=%d length(unique(table))=%d", (tablelen*2+nuniq)*sizeof(int), tablelen, nuniq);
+      error("Failed to allocate %"PRIu64" bytes working memory in chmatchdup: length(table)=%d length(unique(table))=%d", ((uint64_t)tablelen*2+nuniq)*sizeof(int), tablelen, nuniq);
       // # nocov end
     }
     for (int i=0; i<tablelen; ++i) counts[-TRUELENGTH(td[i])-1]++;

src/data.table.h

@@ -3,7 +3,8 @@
 #include <Rinternals.h>
 // #include <signal.h> // the debugging machinery + breakpoint aidee
 // raise(SIGINT);
-#include <stdint.h> // for uint64_t rather than unsigned long long
+#include <stdint.h>    // for uint64_t rather than unsigned long long
+#include <inttypes.h>  // for PRId64 and PRIu64
 #include <stdbool.h>
 #include "myomp.h"
 #include "types.h"

src/fifelse.c

@@ -35,9 +35,9 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b, SEXP na) {
   }
 
   if (len1!=1 && len1!=len0)
-    error("Length of 'yes' is %lld but must be 1 or length of 'test' (%lld).", len1, len0);
+    error("Length of 'yes' is %"PRId64" but must be 1 or length of 'test' (%"PRId64").", len1, len0);
   if (len2!=1 && len2!=len0)
-    error("Length of 'no' is %lld but must be 1 or length of 'test' (%lld).", len2, len0);
+    error("Length of 'no' is %"PRId64" but must be 1 or length of 'test' (%"PRId64").", len2, len0);
   const int64_t amask = len1>1 ? INT64_MAX : 0; // for scalar 'a' bitwise AND will reset iterator to first element: pa[i & amask] -> pa[0]
   const int64_t bmask = len2>1 ? INT64_MAX : 0;
 
@@ -48,7 +48,7 @@ SEXP fifelseR(SEXP l, SEXP a, SEXP b, SEXP na) {
   bool nonna = !isNull(na);
   if (nonna) {
     if (xlength(na) != 1)
-      error("Length of 'na' is %lld but must be 1", xlength(na));
+      error("Length of 'na' is %"PRId64" but must be 1", (int64_t)xlength(na));
     SEXPTYPE tn = TYPEOF(na);
     if (tn == LGLSXP && LOGICAL(na)[0]==NA_LOGICAL) {
       nonna = false;

src/forder.c

@@ -500,7 +500,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
   int keyAlloc = (ncol+n_cplx)*8 + 1;         // +1 for NULL to mark end; calloc to initialize with NULLs
   key = calloc(keyAlloc, sizeof(uint8_t *));  // needs to be before loop because part II relies on part I, column-by-column.
   if (!key)
-    STOP("Unable to allocate %llu bytes of working memory", (unsigned long long)(keyAlloc*sizeof(uint8_t *)));  // # nocov
+    STOP("Unable to allocate %"PRId64" bytes of working memory", (uint64_t)keyAlloc*sizeof(uint8_t *));  // # nocov
   nradix=0; // the current byte we're writing this column to; might be squashing into it (spare>0)
   int spare=0;  // the amount of bits remaining on the right of the current nradix byte
   bool isReal=false;
@@ -569,7 +569,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
     }
 
     uint64_t range = max-min+1 +1/*NA*/ +isReal*3/*NaN, -Inf, +Inf*/;
-    // Rprintf("range=%llu  min=%llu  max=%llu  na_count==%d\n", range, min, max, na_count);
+    // Rprintf("range=%"PRIu64"  min=%"PRIu64"  max=%"PRIu64"  na_count==%d\n", range, min, max, na_count);
 
     int maxBit=0;
     while (range) { maxBit++; range>>=1; }
@@ -604,7 +604,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
       if (key[nradix+b]==NULL) {
         uint8_t *tt = calloc(nrow, sizeof(uint8_t));  // 0 initialize so that NA's can just skip (NA is always the 0 offset)
         if (!tt)
-          STOP("Unable to allocate %llu bytes of working memory", (unsigned long long)(nrow * sizeof(uint8_t))); // # nocov
+          STOP("Unable to allocate %"PRIu64" bytes of working memory", (uint64_t)nrow*sizeof(uint8_t)); // # nocov
         key[nradix+b] = tt;
       }
     }
@@ -622,7 +622,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
 
     const uint64_t naval = ((nalast==1) == asc) ? max+1+isReal*2 : min-1-isReal*2;
     const uint64_t nanval = ((nalast==1) == asc) ? max+2 : min-2;  // only used when isReal
-    // Rprintf("asc=%d  min2=%llu  max2=%llu  naval==%llu  nanval==%llu\n", asc, min2, max2, naval, nanval);
+    // Rprintf("asc=%d  min2=%"PRIu64"  max2=%"PRIu64"  naval==%"PRIu64"  nanval==%"PRIu64"\n", asc, min2, max2, naval, nanval);
 
     // several columns could squash into 1 byte. due to this bit squashing is why we deal
     // with asc|desc here, otherwise it could be done in the ugrp sorting by reversing the ugrp insert sort
@@ -794,7 +794,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrpArg, SEXP sortGroupsArg, SEXP ascArg, S
       Rprintf("Timing block %2d%s = %8.3f   %8d\n", i, (i>=17&&i<=19)?"(*)":"   ", tblock[i], nblock[i]);
     }
     for (int i=0; i<=256; i++) {
-      if (stat[i]) Rprintf("stat[%03d]==%10zd\n", i, stat[i]);
+      if (stat[i]) Rprintf("stat[%03d]==%20"PRIu64"\n", i, (uint64_t)stat[i]);
     }
   }
   #endif