From fa98a7d8b3c0d865660ef127736283acaa952fb1 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 23 Sep 2024 22:46:38 -0700 Subject: [PATCH 1/3] Refactor CHECK_RANGE to be i18n friendly --- src/assign.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/assign.c b/src/assign.c index 4402608ff..e70f5d96d 100644 --- a/src/assign.c +++ b/src/assign.c @@ -744,6 +744,12 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) #define MSGSIZE 1000 static char memrecycle_message[MSGSIZE+1]; // returned to rbindlist so it can prefix with which one of the list of data.table-like objects +static char column_desc[MSGSIZE+1]; // can contain column names, hence relatively large allocation. +const char *columnDesc(int colnum, const char *colname) { + snprintf(column_desc, MSGSIZE, _("column %d named '%s'"), colnum, colname); + return column_desc; +} + const char *memrecycle(const SEXP target, const SEXP where, const int start, const int len, SEXP source, const int sourceStart, const int sourceLen, const int colnum, const char *colname) // like memcpy but recycles single-item source // 'where' a 1-based INTEGER vector subset of target to assign to, or NULL or integer() @@ -932,18 +938,16 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con // inside BODY that cater for 'where' or not. Maybe there's a way to merge the two macros in future. // The idea is to do these range checks without calling coerceVector() (which allocates) - #define CHECK_RANGE(STYPE, RFUN, COND, FMT, TO, FMTVAL) {{ \ + #define CHECK_RANGE(STYPE, RFUN, COND, FMTVAL, FMT) {{ \ const STYPE *sd = (const STYPE *)RFUN(source); \ for (int i=0; i255, "d", "taken as 0", val) + case INTSXP: CHECK_RANGE(int, INTEGER, val<0 || val>255, val, _("%d (type '%s' at RHS position %d taken as 0 when assigning to type '%s' (%s)")) case REALSXP: if (sourceIsI64) - CHECK_RANGE(int64_t, REAL, val<0 || val>255, PRId64, "taken as 0", val) - 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", val) + CHECK_RANGE(int64_t, REAL, val<0 || val>255, val, _("%"PRId64" (type '%s') at RHS position %d taken as 0 when assigning to type '%s' (%s)")) + else CHECK_RANGE(double, REAL, !R_FINITE(val) || val<0.0 || val>256.0 || (int)val!=val, val, _("%f (type '%s') at RHS position %d either truncated (precision lost) or taken as 0 when assigning to type '%s' (%s)")) } break; case INTSXP: switch (TYPEOF(source)) { case REALSXP: if (sourceIsI64) - CHECK_RANGE(int64_t, REAL, val!=NA_INTEGER64 && (val<=NA_INTEGER || val>INT_MAX), PRId64, "out-of-range (NA)", val) - else CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int32_repres(val) || (int)val!=val), "f", "out-of-range(NA) or truncated (precision lost)", val) + CHECK_RANGE(int64_t, REAL, val!=NA_INTEGER64 && (val<=NA_INTEGER || val>INT_MAX), val, _("%"PRId64" (type '%s') at RHS position %d out-of-range (NA) when assigning to type '%s' (%s)")) + else CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int32_repres(val) || (int)val!=val), val, _("%f (type '%s') at RHS position %d out-of-range(NA) or truncated (precision lost) when assigning to type '%s' (%s)")) case CPLXSXP: CHECK_RANGE(Rcomplex, COMPLEX, !((ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)) && - (ISNAN(val.r) || (within_int32_repres(val.r) && (int)val.r==val.r))), "f", "either imaginary part discarded or real part truncated (precision lost)", val.r) + (ISNAN(val.r) || (within_int32_repres(val.r) && (int)val.r==val.r))), val.r, _("%f (type '%s') at RHS position %d either imaginary part discarded or real part truncated (precision lost) when assigning to type '%s' (%s)")) } break; case REALSXP: switch (TYPEOF(source)) { case REALSXP: if (targetIsI64 && !sourceIsI64) - CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int64_repres(val) || (int64_t)val!=val), "f", "out-of-range(NA) or truncated (precision lost)", val) + CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int64_repres(val) || (int64_t)val!=val), val, _("%f (type '%s') at RHS position %d out-of-range(NA) or truncated (precision lost) when assigning to type '%s' (%s)")) break; case CPLXSXP: if (targetIsI64) CHECK_RANGE(Rcomplex, COMPLEX, !((ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)) && - (ISNAN(val.r) || (R_FINITE(val.r) && (int64_t)val.r==val.r))), "f", "either imaginary part discarded or real part truncated (precision lost)", val.r) - else CHECK_RANGE(Rcomplex, COMPLEX, !(ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)), "f", "imaginary part discarded", val.i) + (ISNAN(val.r) || (R_FINITE(val.r) && (int64_t)val.r==val.r))), val.r, _("%f (type '%s') at RHS position %d either imaginary part discarded or real part truncated (precision lost) when assigning to type '%s' (%s)")) + else CHECK_RANGE(Rcomplex, COMPLEX, !(ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)), val.r, _("%f (type '%s') at RHS position %d imaginary part discarded when assigning to type '%s' (%s)")) } } } From 54827614362735bb58b135809828292937d5b7aa Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Tue, 24 Sep 2024 09:14:41 -0700 Subject: [PATCH 2/3] Move column_desc buffer inside columnDesc routine --- src/assign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/assign.c b/src/assign.c index e70f5d96d..07e24ff0e 100644 --- a/src/assign.c +++ b/src/assign.c @@ -744,8 +744,8 @@ SEXP assign(SEXP dt, SEXP rows, SEXP cols, SEXP newcolnames, SEXP values) #define MSGSIZE 1000 static char memrecycle_message[MSGSIZE+1]; // returned to rbindlist so it can prefix with which one of the list of data.table-like objects -static char column_desc[MSGSIZE+1]; // can contain column names, hence relatively large allocation. const char *columnDesc(int colnum, const char *colname) { + static char column_desc[MSGSIZE+1]; // can contain column names, hence relatively large allocation. snprintf(column_desc, MSGSIZE, _("column %d named '%s'"), colnum, colname); return column_desc; } From 7b4065367c1b01e5b072c6d5e0f3d4852cad6d0e Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Tue, 24 Sep 2024 10:59:16 -0700 Subject: [PATCH 3/3] make nominative case clearer --- src/assign.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/assign.c b/src/assign.c index 07e24ff0e..4fc05d992 100644 --- a/src/assign.c +++ b/src/assign.c @@ -746,7 +746,7 @@ static char memrecycle_message[MSGSIZE+1]; // returned to rbindlist so it can pr const char *columnDesc(int colnum, const char *colname) { static char column_desc[MSGSIZE+1]; // can contain column names, hence relatively large allocation. - snprintf(column_desc, MSGSIZE, _("column %d named '%s'"), colnum, colname); + snprintf(column_desc, MSGSIZE, _("(column %d named '%s')"), colnum, colname); return column_desc; } @@ -947,7 +947,8 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con const char *tType = targetIsI64 ? "integer64" : type2char(TYPEOF(target)); \ snprintf(memrecycle_message, MSGSIZE, FMT, \ FMTVAL, sType, i+1, tType, \ - colnum == 0 ? _("target vector") : columnDesc(colnum, colname)); \ + /* NB: important for () to be part of the translated string as a signal of nominative case to translators */ \ + colnum == 0 ? _("(target vector)") : columnDesc(colnum, colname)); \ /* string returned so that rbindlist/dogroups can prefix it with which item of its list this refers to */ \ break; \ } \ @@ -957,36 +958,36 @@ const char *memrecycle(const SEXP target, const SEXP where, const int start, con switch(TYPEOF(target)) { case LGLSXP: switch (TYPEOF(source)) { - case RAWSXP: CHECK_RANGE(Rbyte, RAW, val!=0 && val!=1, val, _("%d (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' (%s)")) - case INTSXP: CHECK_RANGE(int, INTEGER, val!=0 && val!=1 && val!=NA_INTEGER, val, _("%d (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' (%s)")) + case RAWSXP: CHECK_RANGE(Rbyte, RAW, val!=0 && val!=1, val, _("%d (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' %s")) + case INTSXP: CHECK_RANGE(int, INTEGER, val!=0 && val!=1 && val!=NA_INTEGER, val, _("%d (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' %s")) case REALSXP: if (sourceIsI64) - CHECK_RANGE(int64_t, REAL, val!=0 && val!=1 && val!=NA_INTEGER64, val, _("%"PRId64" (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' (%s)")) - else CHECK_RANGE(double, REAL, !ISNAN(val) && val!=0.0 && val!=1.0, val, _("%f (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' (%s)")) + CHECK_RANGE(int64_t, REAL, val!=0 && val!=1 && val!=NA_INTEGER64, val, _("%"PRId64" (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' %s")) + else CHECK_RANGE(double, REAL, !ISNAN(val) && val!=0.0 && val!=1.0, val, _("%f (type '%s') at RHS position %d taken as TRUE when assigning to type '%s' %s")) } break; case RAWSXP: switch (TYPEOF(source)) { - case INTSXP: CHECK_RANGE(int, INTEGER, val<0 || val>255, val, _("%d (type '%s' at RHS position %d taken as 0 when assigning to type '%s' (%s)")) + case INTSXP: CHECK_RANGE(int, INTEGER, val<0 || val>255, val, _("%d (type '%s' at RHS position %d taken as 0 when assigning to type '%s' %s")) case REALSXP: if (sourceIsI64) - CHECK_RANGE(int64_t, REAL, val<0 || val>255, val, _("%"PRId64" (type '%s') at RHS position %d taken as 0 when assigning to type '%s' (%s)")) - else CHECK_RANGE(double, REAL, !R_FINITE(val) || val<0.0 || val>256.0 || (int)val!=val, val, _("%f (type '%s') at RHS position %d either truncated (precision lost) or taken as 0 when assigning to type '%s' (%s)")) + CHECK_RANGE(int64_t, REAL, val<0 || val>255, val, _("%"PRId64" (type '%s') at RHS position %d taken as 0 when assigning to type '%s' %s")) + else CHECK_RANGE(double, REAL, !R_FINITE(val) || val<0.0 || val>256.0 || (int)val!=val, val, _("%f (type '%s') at RHS position %d either truncated (precision lost) or taken as 0 when assigning to type '%s' %s")) } break; case INTSXP: switch (TYPEOF(source)) { case REALSXP: if (sourceIsI64) - CHECK_RANGE(int64_t, REAL, val!=NA_INTEGER64 && (val<=NA_INTEGER || val>INT_MAX), val, _("%"PRId64" (type '%s') at RHS position %d out-of-range (NA) when assigning to type '%s' (%s)")) - else CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int32_repres(val) || (int)val!=val), val, _("%f (type '%s') at RHS position %d out-of-range(NA) or truncated (precision lost) when assigning to type '%s' (%s)")) + CHECK_RANGE(int64_t, REAL, val!=NA_INTEGER64 && (val<=NA_INTEGER || val>INT_MAX), val, _("%"PRId64" (type '%s') at RHS position %d out-of-range (NA) when assigning to type '%s' %s")) + else CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int32_repres(val) || (int)val!=val), val, _("%f (type '%s') at RHS position %d out-of-range(NA) or truncated (precision lost) when assigning to type '%s' %s")) case CPLXSXP: CHECK_RANGE(Rcomplex, COMPLEX, !((ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)) && - (ISNAN(val.r) || (within_int32_repres(val.r) && (int)val.r==val.r))), val.r, _("%f (type '%s') at RHS position %d either imaginary part discarded or real part truncated (precision lost) when assigning to type '%s' (%s)")) + (ISNAN(val.r) || (within_int32_repres(val.r) && (int)val.r==val.r))), val.r, _("%f (type '%s') at RHS position %d either imaginary part discarded or real part truncated (precision lost) when assigning to type '%s' %s")) } break; case REALSXP: switch (TYPEOF(source)) { case REALSXP: if (targetIsI64 && !sourceIsI64) - CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int64_repres(val) || (int64_t)val!=val), val, _("%f (type '%s') at RHS position %d out-of-range(NA) or truncated (precision lost) when assigning to type '%s' (%s)")) + CHECK_RANGE(double, REAL, !ISNAN(val) && (!within_int64_repres(val) || (int64_t)val!=val), val, _("%f (type '%s') at RHS position %d out-of-range(NA) or truncated (precision lost) when assigning to type '%s' %s")) break; case CPLXSXP: if (targetIsI64) CHECK_RANGE(Rcomplex, COMPLEX, !((ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)) && - (ISNAN(val.r) || (R_FINITE(val.r) && (int64_t)val.r==val.r))), val.r, _("%f (type '%s') at RHS position %d either imaginary part discarded or real part truncated (precision lost) when assigning to type '%s' (%s)")) - else CHECK_RANGE(Rcomplex, COMPLEX, !(ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)), val.r, _("%f (type '%s') at RHS position %d imaginary part discarded when assigning to type '%s' (%s)")) + (ISNAN(val.r) || (R_FINITE(val.r) && (int64_t)val.r==val.r))), val.r, _("%f (type '%s') at RHS position %d either imaginary part discarded or real part truncated (precision lost) when assigning to type '%s' %s")) + else CHECK_RANGE(Rcomplex, COMPLEX, !(ISNAN(val.i) || (R_FINITE(val.i) && val.i==0.0)), val.r, _("%f (type '%s') at RHS position %d imaginary part discarded when assigning to type '%s' %s")) } } }