Warnings in vcfR's C++ code at CRAN

[knausb]

Became aware that we're throwing errors at vcfR on CRAN. This built cleanly the last time we submitted. So I'm going to guess they've increased the stringency of their tests. This does not throw errors when building locally, on Travis, or Appveyor. But I was able to reproduce it on WinBuilder R-release. So that will have to be our tool for checking.

[knausb]

And received the following e-mail (2018-03-25).

This concerns packages

ADM3 Amelia BAS Barycenter BoltzMM FastRCS FeatureHashing GERGM Libra
MCMCpack Matching ModelMetrics Mposterior PLordprob Phase123
PieceExpIntensity PwrGSD RRF Rlda Rttf2pt1 SemiCompRisks
StepSignalMargiLike SubTite TVsMiss ape bcp bifactorial bnpmr clusrank
coda coneproj corpustools cstab degreenet edci extracat fbati flam
gRbase glmmML gphmm graphicalVAR grpreg haplo hbmem iJRF imager interp
lfl lidR mcIRT mixAK mixR mlegp multinet mvnmle ncvreg pbdSLAP
pedantics permDep propr quadrupen reReg revealedPrefs rgenoud rlas rmp
s2 sparseSVM spatcounts spduration splusTimeDate spp tidyxl tranSurv
treeman validate vcfR weco

R-devel now reports warning from GCC (at least >= 7) like

ADM3.c:37:12: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]

as can be seen from the CRAN results page for your package.

However, in almost all the cases we looked at the actual cause is the
use of & where && would be better, or | instead of || .

Please correct your code to avoid the warnings, ASAP and before April
20 to safely retain the package on CRAN.

[knausb]

Appears to build cleanly on R-release at win-builder. At 18c276c1c5714db885ca76c4c6446d9a9810c554. Now testing R-devel.

[knausb]

Oops, still have the following.

* checking whether package 'vcfR' can be installed ... WARNING
Found the following significant warnings:
  Note: break used in wrong context: no loop is visible 
See 'd:/RCompile/CRANguest/R-devel/vcfR.Rcheck/00install.out' for details.
Information on the location(s) of code generating the 'Note's can be
obtained by re-running with environment variable R_KEEP_PKG_SOURCE set
to 'yes'.

[knausb]

This appears in R-devel but not R-release (3.4.4).

[knausb]

Using

R Under development (unstable) (2018-03-16 r74418) -- "Unsuffered Consequences"

In a docker session I now see the following.

* checking tests ...
  Running ‘testthat.R’ [25s/25s]
 ERROR
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  ══ testthat results  ═══════════════════════════════════════════════════════════
  OK: 426 SKIPPED: 6 FAILED: 47
  1. Failure: Jost's example works (@test_genetic_diff.R#35) 
  2. Failure: Jost's example works (@test_genetic_diff.R#36) 
  3. Failure: Jost's example works (@test_genetic_diff.R#37) 
  4. Failure: Jost's example works (@test_genetic_diff.R#38) 
  5. Failure: Nei's method works (@test_genetic_diff.R#55) 
  6. Failure: Nei's method works (@test_genetic_diff.R#56) 
  7. Failure: Nei's method works (@test_genetic_diff.R#57) 
  8. Failure: Nei's method works, Hedrick Table 1 (@test_genetic_diff.R#89) 
  9. Failure: Nei's method works, Hedrick Table 1 (@test_genetic_diff.R#90) 
  1. ...

  Error: testthat unit tests failed
  Execution halted
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... OK
* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* DONE

Status: 1 ERROR, 2 WARNINGs, 2 NOTEs
See
  ‘/RSource/vcfR.Rcheck/00check.log’
for details.

This does not appear to be the same ERROR reported on CRAN.

[knausb]

The above problem appears to be due to how .gt_to_popsum() concatenates the allele counts into a comma delimited string. I had been using sprintf to concatenate the allele counts into a char. Not sure why I took that route. I have circumvented this issue by using a std::string which has + to facilitate the concatenation. Commit at 7fc3618b294a44cd732b852dea656ccd242ff789.

[knausb]

We're now down to 2 WARNINGS.

* checking whether package ‘vcfR’ can be installed ... WARNING
Found the following significant warnings:
  Note: break used in wrong context: no loop is visible 
See ‘/RSource/vcfR.Rcheck/00install.out’ for details.
Information on the location(s) of code generating the ‘Note’s can be
obtained by re-running with environment variable R_KEEP_PKG_SOURCE set
to ‘yes’.

CRAN has asked me to fix this by April 20.

* checking compilation flags used ... WARNING
Compilation used the following non-portable flag(s):
  ‘-Wdate-time’ ‘-Werror=format-security’ ‘-Wformat’

I haven't seen this second one before, so I'll have to do some homework.

[knausb]

I'm working in Docker so that I can use R-devel, so parts of my typical workflow are not available to me. At the R prompt I can determine my HOME directory as follows.

> Sys.getenv()

For my its at /root. Again, I'm working in Docker. I would not suggest using this home for a regular system. I created a file at /root/.Rprofile that includes the following line.

Sys.setenv(R_KEEP_PKG_SOURCE="yes")

This shoulld be executed at the start of an R session so that when I query environment variables, as above, it should be set. Now, hopefully, my build will tell me where my bad break is.

[knausb]

Too easy! After building my vcfR.Rcheck/00install.out file reports the following.

Note: break used in wrong context: no loop is visible at proc_chromR.R:194

Which is absolutely correct. I deleted the break and eliminated the WARNING.

[knausb]

Down to one WARNING on R-devel.

* checking compilation flags used ... WARNING
Compilation used the following non-portable flag(s):
  ‘-Wdate-time’ ‘-Werror=format-security’ ‘-Wformat’

I checked my Makevars file but I do not set these options.

The Docker container is running:

# g++ --version
g++ (Debian 7.3.0-11) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

While my system is running the following.

$ g++ --version
g++ (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Not sure that matters, but it might.

[knausb]

https://cran.r-project.org/web/checks/check_results_vcfR.html Additional issues valgrind

Reports:

==46== Conditional jump or move depends on uninitialised value(s)
==46==    at 0x5B5778D: internal_utf8_loop (loop.c:298)
==46==    by 0x5B5778D: __gconv_transform_internal_utf8 (skeleton.c:609)
==46==    by 0x5BD3844: wcsrtombs (wcsrtombs.c:110)
==46==    by 0x5B69210: wcstombs (wcstombs.c:34)
==46==    by 0x509BA3C: wcstombs (stdlib.h:154)
==46==    by 0x509BA3C: tre_parse_bracket_items (tre-parse.c:336)
==46==    by 0x509BA3C: tre_parse_bracket (tre-parse.c:453)
==46==    by 0x509BA3C: tre_parse (tre-parse.c:1380)
==46==    by 0x5093438: tre_compile (tre-compile.c:1920)
==46==    by 0x5090BA0: tre_regcompb (regcomp.c:150)
==46==    by 0x4F94D42: do_gsub (grep.c:1828)
==46==    by 0x4F66809: bcEval (eval.c:6771)
==46==    by 0x4F74D0F: Rf_eval (eval.c:624)
==46==    by 0x4F768AE: R_execClosure (eval.c:1764)
==46==    by 0x4F74ED9: Rf_eval (eval.c:747)
==46==    by 0x4F793A7: do_set (eval.c:2774)
==46==  Uninitialised value was created by a stack allocation
==46==    at 0x509A06D: tre_parse (tre-parse.c:943)

I believe this is due to the software tre. https://github.com/laurikari/tre

vcfR does not directly call this software.

[knausb]

vcfR v1.8.0 addressed most of this. We still appear to have valgrind issue. But I think I'll address that in a new issue.