localize cumSum vars (#7239)

* localize reused object cumSum + general vicinity cleanup

* reverting to old draft for testing

* gradual reintroduction of older changes

* formatting

* fix formatting warning

* add coverage test case

* change testnum

* fix testnum

---------

Co-authored-by: Benjamin Schwendinger <[email protected]>

Author: badasahog

inst/tests/tests.Rraw

@@ -13701,6 +13701,10 @@ test(1962.054, forder(DT, ), 3:1)
 
 test(1962.055, fsort(as.double(DT$a), internal = TRUE),
      error = 'Internal code should not be being called on type double')
+# coverage for fsort #7239
+set.seed(1)
+x = as.double(sample(0:(2^20), size = 1e6, replace = TRUE))
+test(1962.0551, sort(x), fsort(x))
 
 l = as.list(DT)
 test(1962.056, setorder(l, a), error = 'x must be a data.frame or data.table')

src/fsort.c

@@ -1,6 +1,6 @@
 #include "data.table.h"
 
-#define INSERT_THRESH 200  // TODO: expose via api and test
+static const int INSERT_THRESH = 200;  // TODO: expose via api and test
 
 static void dinsert(double *x, const int n) {   // TODO: if and when twiddled, double => ull
   if (n<2) return;
@@ -43,17 +43,16 @@ static void dradix_r(  // single-threaded recursive worker
     return;
   }
 
-  uint64_t cumSum=0;
-  for (uint64_t i=0; cumSum<n; ++i) { // cumSum<n better than i<width as may return early
-    uint64_t tmp;
-    if ((tmp=counts[i])) {  // don't cumulate through 0s, important below to save a wasteful memset to zero
+  for (uint64_t i = 0, cumSum = 0; cumSum < n; i++) { // cumSum<n better than i<width as may return early
+    uint64_t tmp = counts[i];
+    if (tmp) {  // don't cumulate through 0s, important below to save a wasteful memset to zero
       counts[i] = cumSum;
       cumSum += tmp;
     }
   } // leaves cumSum==n && 0<i && i<=width
 
   tmp=in;
-  for (uint64_t i=0; i<n; ++i) {  // go forwards not backwards to give cpu pipeline better chance
+  for (uint64_t i = 0; i<n; ++i) {  // go forwards not backwards to give cpu pipeline better chance
     int thisx = (*(uint64_t *)tmp - minULL) >> fromBit & mask;
     working[ counts[thisx]++ ] = *tmp;
     tmp++;
@@ -71,8 +70,7 @@ static void dradix_r(  // single-threaded recursive worker
     return;
   }
 
-  cumSum=0;
-  for (int i=0; cumSum<n; ++i) {   // again, cumSum<n better than i<width as it can return early
+  for (uint64_t i = 0, cumSum = 0; cumSum < n; i++) {   // again, cumSum<n better than i<width as it can return early
     if (counts[i] == 0) continue;
     uint64_t thisN = counts[i] - cumSum;  // undo cummulate; i.e. diff
     if (thisN <= INSERT_THRESH) {