From 2070c852774781af803e697c5a937e826cb65ba1 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 22 Jul 2019 02:56:40 +0800 Subject: [PATCH 1/3] add default to all switch statements in src add coverage tests of touched files fix test vestigial msg --- inst/tests/tests.Rraw | 14 ++++ src/assign.c | 4 +- src/bmerge.c | 12 ++- src/fcast.c | 10 +-- src/frank.c | 5 +- src/frollR.c | 7 +- src/ijoin.c | 168 ++++++++++++++++++++++-------------------- src/nafill.c | 3 +- 8 files changed, 128 insertions(+), 95 deletions(-) diff --git a/inst/tests/tests.Rraw b/inst/tests/tests.Rraw index 03fecf3da6..5c1131240c 100644 --- a/inst/tests/tests.Rraw +++ b/inst/tests/tests.Rraw @@ -15403,6 +15403,20 @@ DT = data.table(d=sample(seq(as.Date("2015-01-01"), as.Date("2015-01-05"), by="d test(2070.01, typeof(DT$d), "double") test(2070.02, DT[, .N, keyby=d, verbose=TRUE], output="Column 1.*date.*8 byte double.*no fractions are present.*4 byte integer.*to save space and time") +# coverage along with switch+default pairing +test(2071.01, dcast(data.table(id=1, grp=1, e=expression(1)), id ~ grp, value.var='e'), error="Unsupported column type in fcast val: 'expression'") +test(2071.02, is_na(data.table(expression(1))), error="Unsupported column type 'expression'") +test(2071.03, is_na(data.table(1L), 2L), error="Item 1 of 'cols' is 2 which is outside") +test(2071.04, is_na(list(1L, 1:2)), error="Column 2 of input list x is length 2, inconsistent") +test(2071.05, any_na(data.table(1L), 2L), error="Item 1 of 'cols' is 2 which is outside") +test(2071.06, any_na(list(1L, 1:2)), error="Column 2 of input list x is length 2, inconsistent") +test(2071.07, any_na(data.table(as.raw(0L))), FALSE) +test(2071.08, any_na(data.table(c(1+1i, NA)))) +test(2071.09, any_na(data.table(expression(1))), error="Unsupported column type 'expression'") +test(2071.10, dcast(data.table(a=1, b=1, l=list(list(1))), a ~ b, value.var='l'), + data.table(a=1, `1`=list(list(1)), key='a')) +test(2071.11, dcast(data.table(a = 1, b = 2, c = 3), a ~ b, value.var = 'c', fill = '2'), + data.table(a=1, `2`=3, key='a')) ################################### # Add new tests above this line # diff --git a/src/assign.c b/src/assign.c index d7f5d45785..01a6147ec4 100644 --- a/src/assign.c +++ b/src/assign.c @@ -1021,8 +1021,8 @@ void writeNA(SEXP v, const int from, const int n) // If there's ever a way added to R API to pass NA_STRING to allocVector() to tell it to initialize with NA not "", would be great for (int i=from; i<=to; ++i) SET_STRING_ELT(v, i, NA_STRING); break; - case VECSXP : - // list columns already have each item initialized to NULL + case VECSXP : case EXPRSXP : + // list & expression columns already have each item initialized to NULL break; default : error("Internal error: writeNA passed a vector of type '%s'", type2char(TYPEOF(v))); // # nocov diff --git a/src/bmerge.c b/src/bmerge.c index ec003ff547..d85f4dd08a 100644 --- a/src/bmerge.c +++ b/src/bmerge.c @@ -265,10 +265,11 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg } if (col>-1 && op[col] != EQ) { switch (op[col]) { - case LE : xlow = xlowIn; break; - case LT : xupp = xlow + 1; xlow = xlowIn; break; - case GE : if (ival.i != NA_INTEGER) xupp = xuppIn; break; - case GT : xlow = xupp - 1; if (ival.i != NA_INTEGER) xupp = xuppIn; break; + case LE : xlow = xlowIn; break; + case LT : xupp = xlow + 1; xlow = xlowIn; break; + case GE : if (ival.i != NA_INTEGER) xupp = xuppIn; break; + case GT : xlow = xupp - 1; if (ival.i != NA_INTEGER) xupp = xuppIn; break; + default : error("Internal error in bmerge_r for '%s' column. Unrecognized value op[col]=%d", type2char(TYPEOF(xc)), op[col]); // #nocov } // for LE/LT cases, we need to ensure xlow excludes NA indices, != EQ is checked above already if (op[col] <= 3 && xlow 0 && type_count[from[i]-1]) ? type_count[from[i]-1] : 1; - break; + } break; - case EQUAL: + case EQUAL: { for (i=0; i0) ? from[i] : 1; @@ -275,9 +282,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr if (len == totlen) ++totlen; } - break; + } break; - case ANY: + case ANY: { for (i=0; i 0) ? from[i] : 1; @@ -289,9 +296,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr if (len == totlen) ++totlen; } - break; + } break; - case WITHIN: + case WITHIN: { for (i=0; i 0) { @@ -352,9 +360,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr ++thislen; } } - break; + } break; - case EQUAL : + case EQUAL : { for (i=0; i 0 && to[i] > 0) { @@ -388,9 +396,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr ++thislen; } } - break; + } break; - case ANY : + case ANY : { for (i=0; i0) ? from[i] : 1; @@ -418,9 +426,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr ++thislen; } } - break; + } break; - case WITHIN : + case WITHIN : { for (i=0; i Date: Tue, 23 Jul 2019 06:26:03 +0800 Subject: [PATCH 2/3] removed curlies? --- src/fcast.c | 8 +-- src/frollR.c | 2 +- src/ijoin.c | 156 +++++++++++++++++++++++++-------------------------- 3 files changed, 83 insertions(+), 83 deletions(-) diff --git a/src/fcast.c b/src/fcast.c index e24cb0f112..5ef9f70d90 100644 --- a/src/fcast.c +++ b/src/fcast.c @@ -59,7 +59,7 @@ SEXP fcast(SEXP lhs, SEXP val, SEXP nrowArg, SEXP ncolArg, SEXP idxArg, SEXP fil } } } break; - case STRSXP: { + case STRSXP: for (int j=0; j 0 && type_count[from[i]-1]) ? type_count[from[i]-1] : 1; - } break; + break; - case EQUAL: { + case EQUAL: for (i=0; i0) ? from[i] : 1; @@ -282,9 +282,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr if (len == totlen) ++totlen; } - } break; + break; - case ANY: { + case ANY: for (i=0; i 0) ? from[i] : 1; @@ -296,9 +296,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr if (len == totlen) ++totlen; } - } break; + break; - case WITHIN: { + case WITHIN: for (i=0; i 0) { @@ -360,9 +360,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr ++thislen; } } - } break; + break; - case EQUAL : { + case EQUAL : for (i=0; i 0 && to[i] > 0) { @@ -396,9 +396,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr ++thislen; } } - } break; + break; - case ANY : { + case ANY : for (i=0; i0) ? from[i] : 1; @@ -426,9 +426,9 @@ SEXP overlaps(SEXP ux, SEXP imatches, SEXP multArg, SEXP typeArg, SEXP nomatchAr ++thislen; } } - } break; + break; - case WITHIN : { + case WITHIN : for (i=0; i Date: Mon, 22 Jul 2019 16:57:45 -0700 Subject: [PATCH 3/3] whitespace only --- src/bmerge.c | 3 +-- src/ijoin.c | 71 ++++++++++++++++++++++++++-------------------------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/src/bmerge.c b/src/bmerge.c index d85f4dd08a..a7246786a8 100644 --- a/src/bmerge.c +++ b/src/bmerge.c @@ -548,8 +548,7 @@ void bmerge_r(int xlowIn, int xuppIn, int ilowIn, int iuppIn, int col, int thisg if (iupp 0 && type_count[from[i]-1]) ? type_count[from[i]-1] : 1; - break; + break; case EQUAL: for (i=0; i