Skip to content

Commit

Permalink
gcc-12 always-false in fread.c (#5476)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattdowle committed Oct 9, 2022
1 parent 73a1f9d commit 2e691f5
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 24 deletions.
4 changes: 2 additions & 2 deletions .dev/CRAN_Release.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ grep asCharacter *.c | grep -v PROTECT | grep -v SET_VECTOR_ELT | grep -v setAtt

cd ..
R
cc(test=TRUE, clean=TRUE, CC="gcc-10") # to compile with -pedandic -Wall, latest gcc as CRAN: https://cran.r-project.org/web/checks/check_flavors.html
cc(test=TRUE, clean=TRUE, CC="gcc-12") # to compile with -pedandic -Wall, latest gcc as CRAN: https://cran.r-project.org/web/checks/check_flavors.html
saf = options()$stringsAsFactors
options(stringsAsFactors=!saf) # check tests (that might be run by user) are insensitive to option, #2718
test.data.table()
Expand Down Expand Up @@ -314,7 +314,7 @@ cd ~/build/R-devel-strict-clang
make

cd ~/build/R-devel-strict-gcc
# gcc-10 (in dev currently) failed to build R, so using regular gcc-9 (9.3.0 as per focal/Pop!_OS 20.04)
# gcc-10 failed to build R-devel at some point, so using regular gcc-9 (9.3.0 as per focal/Pop!_OS 20.04)
./configure --without-recommended-packages --disable-byte-compiled-packages --disable-openmp --enable-strict-barrier --disable-long-double CC="gcc-9 -fsanitize=undefined,address -fno-sanitize=float-divide-by-zero -fno-omit-frame-pointer"
make

Expand Down
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Package: data.table
Version: 1.14.2
Version: 1.14.4
Title: Extension of `data.frame`
Authors@R: c(
person("Matt","Dowle", role=c("aut","cre"), email="[email protected]"),
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ some:

.PHONY: clean
clean:
$(RM) data.table_1.14.2.tar.gz
$(RM) data.table_1.14.4.tar.gz
$(RM) src/*.o
$(RM) src/*.so

Expand All @@ -28,7 +28,7 @@ build:

.PHONY: install
install:
$(R) CMD INSTALL data.table_1.14.2.tar.gz
$(R) CMD INSTALL data.table_1.14.4.tar.gz

.PHONY: uninstall
uninstall:
Expand All @@ -40,7 +40,7 @@ test:

.PHONY: check
check:
_R_CHECK_CRAN_INCOMING_REMOTE_=false $(R) CMD check data.table_1.14.2.tar.gz --as-cran --ignore-vignettes --no-stop-on-test-error
_R_CHECK_CRAN_INCOMING_REMOTE_=false $(R) CMD check data.table_1.14.4.tar.gz --as-cran --ignore-vignettes --no-stop-on-test-error

.PHONY: revision
revision:
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

## NOTES

1. gcc 12.1 (May 2022) now detects and warns about an always-false condition in `fread` which caused a small efficiency saving never to be invoked. Thanks to CRAN for testing latest versions of compilers.

2. `update.dev.pkg()` has been renamed `update_dev_pkg()` to get out of the way of the `stats::update` generic function, [#5421](https://github.com/Rdatatable/data.table/pull/5421). This is a utility function which upgrades the version of `data.table` to the latest commit in development which has passed all tests. As such we don't expect any backwards compatibility concerns. Its manual page was causing an intermittent hang/crash from `R CMD check` on Windows-only on CRAN which we hope will be worked around by changing its name.


Expand Down
36 changes: 19 additions & 17 deletions src/fread.c
Original file line number Diff line number Diff line change
Expand Up @@ -1274,24 +1274,22 @@ int freadMain(freadMainArgs _args) {
while (*nastr) {
if (**nastr == '\0') {
blank_is_a_NAstring = true;
// if blank is the only one, as is the default, clear NAstrings so that doesn't have to be checked
if (nastr==NAstrings && nastr+1==NULL) NAstrings=NULL;
nastr++;
continue;
} else {
const char *ch = *nastr;
size_t nchar = strlen(ch);
if (isspace(ch[0]) || isspace(ch[nchar-1]))
STOP(_("freadMain: NAstring <<%s>> has whitespace at the beginning or end"), ch);
if (strcmp(ch,"T")==0 || strcmp(ch,"F")==0 ||
strcmp(ch,"TRUE")==0 || strcmp(ch,"FALSE")==0 ||
strcmp(ch,"True")==0 || strcmp(ch,"False")==0)
STOP(_("freadMain: NAstring <<%s>> is recognized as type boolean, this is not permitted."), ch);
if ((strcmp(ch,"1")==0 || strcmp(ch,"0")==0) && args.logical01)
STOP(_("freadMain: NAstring <<%s>> and logical01=%s, this is not permitted."), ch, args.logical01 ? "TRUE" : "FALSE");
char *end;
errno = 0;
(void)strtod(ch, &end); // careful not to let "" get to here (see continue above) as strtod considers "" numeric
if (errno==0 && (size_t)(end - ch) == nchar) any_number_like_NAstrings = true;
}
const char *ch = *nastr;
size_t nchar = strlen(ch);
if (isspace(ch[0]) || isspace(ch[nchar-1]))
STOP(_("freadMain: NAstring <<%s>> has whitespace at the beginning or end"), ch);
if (strcmp(ch,"T")==0 || strcmp(ch,"F")==0 ||
strcmp(ch,"TRUE")==0 || strcmp(ch,"FALSE")==0 ||
strcmp(ch,"True")==0 || strcmp(ch,"False")==0 ||
strcmp(ch,"1")==0 || strcmp(ch,"0")==0)
STOP(_("freadMain: NAstring <<%s>> is recognized as type boolean, this is not permitted."), ch);
char *end;
errno = 0;
(void)strtod(ch, &end); // careful not to let "" get to here (see continue above) as strtod considers "" numeric
if (errno==0 && (size_t)(end - ch) == nchar) any_number_like_NAstrings = true;
nastr++;
}
disabled_parsers[CT_BOOL8_N] = !args.logical01;
Expand All @@ -1314,6 +1312,10 @@ int freadMain(freadMainArgs _args) {
DTPRINT(_(" show progress = %d\n"), args.showProgress);
DTPRINT(_(" 0/1 column will be read as %s\n"), args.logical01? "boolean" : "integer");
}
if (*NAstrings==NULL || // user sets na.strings=NULL
(**NAstrings=='\0' && *(NAstrings+1)==NULL)) { // user sets na.strings=""
NAstrings=NULL; // clear NAstrings to save end_NA_string() dealing with these cases (blank_is_a_NAstring was set to true above)
}

stripWhite = args.stripWhite;
skipEmptyLines = args.skipEmptyLines;
Expand Down
2 changes: 1 addition & 1 deletion src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,6 @@ SEXP initLastUpdated(SEXP var) {

SEXP dllVersion() {
// .onLoad calls this and checks the same as packageVersion() to ensure no R/C version mismatch, #3056
return(ScalarString(mkChar("1.14.2")));
return(ScalarString(mkChar("1.14.4")));
}

0 comments on commit 2e691f5

Please sign in to comment.