32bit rdevel (attempt 2) (#2668)

CRAN_Release.cmd

@@ -76,6 +76,7 @@ grep "PROTECT *( *getAttrib" *.c  # attributes are already protected
 grep "\"starts\"" *.c     --exclude init.c
 grep "setAttrib(" *.c      # scan all setAttrib calls manually as a double-check
 grep "install(" *.c       --exclude init.c   # TODO: perhaps in future pre-install all constants
+grep mkChar *.c            # see comment in bmerge.c about passing this grep. I'm not sure char_ is really necessary, though.
 
 # ScalarInteger and ScalarString allocate and must be PROTECTed unless i) returned (which protects),
 # or ii) passed to setAttrib (which protects, providing leak-seals above are ok)
@@ -221,6 +222,12 @@ print(Sys.time()); started.at<-proc.time(); try(test.data.table()); print(Sys.ti
 # Running test id 1437.0331      Error : protect(): protection stack overflow
 
 
+#############################################################################
+# TODO: recompile without USE_RINTERNALS and recheck write barrier under ASAN
+#############################################################################
+There are some things to overcome to achieve compile without USE_RINTERNALS, though.
+
+
 ###############################################
 #  Single precision e.g. CRAN's Solaris Sparc
 ###############################################

DESCRIPTION

@@ -13,7 +13,7 @@ Authors@R: c(
 Depends: R (>= 3.1.0)
 Imports: methods
 Suggests: bit64, knitr, nanotime, chron, ggplot2 (>= 0.9.0), plyr, reshape, reshape2, testthat (>= 0.4), hexbin, fastmatch, nlme, xts, gdata, GenomicRanges, caret, curl, zoo, plm, rmarkdown, parallel
-Description: Fast aggregation of large data (e.g. 100GB in RAM), fast ordered joins, fast add/modify/delete of columns by group using no copies at all, list columns, a friendly and fast multithreaded file reader and writer. Offers a natural and flexible syntax, for faster development.
+Description: Fast aggregation of large data (e.g. 100GB in RAM), fast ordered joins, fast add/modify/delete of columns by group using no copies at all, list columns, friendly and fast character-separated-value read/write. Offers a natural and flexible syntax, for faster development.
 License: MPL-2.0 | file LICENSE
 URL: http://r-datatable.com
 BugReports: https://github.com/Rdatatable/data.table/issues

appveyor.yml

@@ -26,6 +26,10 @@ environment:
 
   matrix:
 
+  - R_VERSION: devel
+    R_ARCH: i386
+    GCC_PATH: mingw_32
+
   - R_VERSION: release
     R_ARCH: i386
     GCC_PATH: mingw_32
@@ -38,10 +42,6 @@ environment:
     R_ARCH: x64
     GCC_PATH: mingw_64
 
-#  Temp off, see PR #2665. Put first when enabled again.
-#  - R_VERSION: devel
-#    R_ARCH: i386
-#    GCC_PATH: mingw_32
 
 build_script:
   - set _R_CHECK_FORCE_SUGGESTS_=false

src/bmerge.c

@@ -142,10 +142,12 @@ SEXP bmerge(SEXP iArg, SEXP xArg, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SE
   // isorted arg
   o = NULL;
   if (!LOGICAL(isorted)[0]) {
-    SEXP order = PROTECT(int_vec_init(length(icolsArg), 1)); // rep(1L, length(icolsArg))
+    SEXP order = PROTECT(allocVector(INTSXP, length(icolsArg)));
+    protecti++;
+    for (int j=0; j<LENGTH(order); j++) INTEGER(order)[j]=1;   // rep(1L, length(icolsArg))
     SEXP oSxp = PROTECT(forder(i, icolsArg, ScalarLogical(FALSE), ScalarLogical(TRUE), order, ScalarLogical(FALSE)));
+    protecti++;
     // TODO - split head of forder into C-level callable
-    protecti += 2;   // order and oSxp
     if (!LENGTH(oSxp)) o = NULL; else o = INTEGER(oSxp);
   }
 
@@ -182,10 +184,10 @@ SEXP bmerge(SEXP iArg, SEXP xArg, SEXP icolsArg, SEXP xcolsArg, SEXP isorted, SE
   SET_VECTOR_ELT(ans, 3, allLen1Arg);
   SET_VECTOR_ELT(ans, 4, allGrp1Arg);
   SET_STRING_ELT(ansnames, 0, char_starts);  // changed from mkChar to char_ to pass the grep in CRAN_Release.cmd
-  SET_STRING_ELT(ansnames, 1, mkChar("lens"));
-  SET_STRING_ELT(ansnames, 2, mkChar("indices"));
-  SET_STRING_ELT(ansnames, 3, mkChar("allLen1"));
-  SET_STRING_ELT(ansnames, 4, mkChar("allGrp1"));
+  SET_STRING_ELT(ansnames, 1, char_lens);
+  SET_STRING_ELT(ansnames, 2, char_indices);
+  SET_STRING_ELT(ansnames, 3, char_allLen1);
+  SET_STRING_ELT(ansnames, 4, char_allGrp1);
   setAttrib(ans, R_NamesSymbol, ansnames);
   if (nqmaxgrp > 1 && mult == ALL) {
     Free(retFirst);

src/data.table.h

@@ -57,6 +57,10 @@ SEXP char_IDate;
 SEXP char_Date;
 SEXP char_POSIXct;
 SEXP char_nanotime;
+SEXP char_lens;
+SEXP char_indices;
+SEXP char_allLen1;
+SEXP char_allGrp1;
 SEXP sym_sorted;
 SEXP sym_index;
 SEXP sym_BY;

src/fcast.c

@@ -90,19 +90,6 @@ SEXP fcast(SEXP lhs, SEXP val, SEXP nrowArg, SEXP ncolArg, SEXP idxArg, SEXP fil
   return(ans);
 }
 
-// used in bmerge.c
-SEXP int_vec_init(R_len_t n, int val) {
-  // Matt removed the SEXP val input because the ScalarInteger() input wasn't PROTECTed in caller
-  // and was potentially leaking/crashing.
-  // This function only used once and only for int, so to remove untested lines (code-coverage)
-  // decided to simplify it rather than add the PROTECT and UNPROTECT in caller.
-  if (n < 0) error("Input argument 'n' to 'int_vec_init' must be >= 0");
-  SEXP ans = PROTECT(allocVector(INTSXP, n));
-  for (R_len_t i=0; i<n; i++) INTEGER(ans)[i] = val;
-  UNPROTECT(1);
-  return(ans);
-}
-
 // commenting all unused functions, but not deleting it, just in case
 
 // // internal functions that are not used anymore..

src/forder.c

@@ -1098,7 +1098,8 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrp, SEXP sortStrArg, SEXP orderArg, SEXP
 
   if (isNewList(DT)) {
     if (!length(DT)) error("DT is an empty list() of 0 columns");
-    if (!isInteger(by) || !length(by)) error("DT has %d columns but 'by' is either not integer or length 0", length(DT));  // seq_along(x) at R level
+    if (!isInteger(by) || !LENGTH(by)) error("DT has %d columns but 'by' is either not integer or is length 0", length(DT));  // seq_along(x) at R level
+    if (!isInteger(orderArg) || LENGTH(orderArg)!=LENGTH(by)) error("Either 'order' is not integer or its length (%d) is different to 'by's length (%d)", LENGTH(orderArg), LENGTH(by));
     n = length(VECTOR_ELT(DT,0));
     for (i=0; i<LENGTH(by); i++) {
       if (INTEGER(by)[i] < 1 || INTEGER(by)[i] > length(DT))
@@ -1109,6 +1110,7 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrp, SEXP sortStrArg, SEXP orderArg, SEXP
     x = VECTOR_ELT(DT,INTEGER(by)[0]-1);
   } else {
     if (!isNull(by)) error("Input is a single vector but 'by' is not NULL");
+    if (!isInteger(orderArg) || LENGTH(orderArg)!=1) error("Input is a single vector but 'order' is not a length 1 integer");
     n = length(DT);
     x = DT;
   }
@@ -1120,12 +1122,22 @@ SEXP forder(SEXP DT, SEXP by, SEXP retGrp, SEXP sortStrArg, SEXP orderArg, SEXP
   nalast = (LOGICAL(naArg)[0] == NA_LOGICAL) ? 0 : (LOGICAL(naArg)[0] == TRUE) ? 1 : -1; // 1=TRUE, -1=FALSE, 0=NA
   gsmaxalloc = n;  // upper limit for stack size (all size 1 groups). We'll detect and avoid that limit, but if just one non-1 group (say 2), that can't be avoided.
 
-  // TODO: check for 'orderArg'
+  if (n==0) {
+    // empty vector or 0-row DT is always sorted
+    SEXP ans = PROTECT(allocVector(INTSXP, 0));
+    if (LOGICAL(retGrp)[0]) {
+      setAttrib(ans, sym_starts, allocVector(INTSXP, 0));
+      setAttrib(ans, sym_maxgrpn, ScalarInteger(0));
+    }
+    UNPROTECT(1);
+    return ans;
+  }
+  // if n==1, the code is left to proceed below in case one or more of the 1-row by= columns are NA and na.last=NA. Otherwise it would be easy to return now.
 
   SEXP ans = PROTECT(allocVector(INTSXP, n)); // once for the result, needs to be length n.
   int *o = INTEGER(ans);                      // TO DO: save allocation if NULL is returned (isSorted==TRUE)
   o[0] = -1;                                  // so [i|c|d]sort know they can populate o directly with no working memory needed to reorder existing order
-                        // had to repace this from '0' to '-1' because 'nalast = 0' replace 'o[.]' with 0 values.
+                                              // using -1 rather than 0 because 'nalast = 0' replaces 'o[.]' with 0 values.
   xd = DATAPTR(x);
   stackgrps = length(by)>1 || LOGICAL(retGrp)[0];
   savetl_init();   // from now on use Error not error.

src/freadR.c

@@ -200,9 +200,9 @@ _Bool userOverride(int8_t *type, lenOff *colNames, const char *anchor, int ncol)
       if (colNames[i].len<=0) {
         char buff[12];
         sprintf(buff,"V%d",i+1);
-        this = mkChar(buff);
+        this = mkChar(buff);  // no PROTECT as passed immediately to SET_STRING_ELT
       } else {
-        this = mkCharLenCE(anchor+colNames[i].off, colNames[i].len, ienc);
+        this = mkCharLenCE(anchor+colNames[i].off, colNames[i].len, ienc);  // no PROTECT as passed immediately to SET_STRING_ELT
       }
       SET_STRING_ELT(colNamesSxp, i, this);
     }
@@ -422,9 +422,8 @@ void pushBuffer(ThreadLocalFreadParsingContext *ctx)
           for (int i=0; i<nRows; i++) {
             int strLen = source->len;
             if (strLen) {
-              SEXP thisStr = strLen<0 ? NA_STRING : mkCharLenCE(anchor + source->off, strLen, ienc);
               // stringLen == INT_MIN => NA, otherwise not a NAstring was checked inside fread_mean
-              SET_STRING_ELT(dest, DTi+i, thisStr);
+              SET_STRING_ELT(dest, DTi+i, strLen<0 ? NA_STRING : mkCharLenCE(anchor + source->off, strLen, ienc));
             } // else dest was already initialized with R_BlankString by allocVector()
             source += cnt8;
           }

src/init.c

@@ -236,6 +236,11 @@ void attribute_visible R_init_datatable(DllInfo *info)
   char_POSIXct =   PRINTNAME(install("POSIXct"));
   char_nanotime =  PRINTNAME(install("nanotime"));
   char_starts =    PRINTNAME(sym_starts = install("starts"));
+  char_lens =      PRINTNAME(install("lens"));
+  char_indices =   PRINTNAME(install("indices"));
+  char_allLen1 =   PRINTNAME(install("allLen1"));
+  char_allGrp1 =   PRINTNAME(install("allGrp1"));
+
   if (TYPEOF(char_integer64) != CHARSXP) {
     // checking one is enough in case of any R-devel changes
     error("PRINTNAME(install(\"integer64\")) has returned %s not %s",