vroom icon indicating copy to clipboard operation
vroom copied to clipboard

Consider rchk issues

Open jennybc opened this issue 2 years ago • 2 comments

https://raw.githubusercontent.com/kalibera/cran-checks/master/rchk/results/vroom.out

Package vroom version 1.5.7
Package built using 82396/R 4.3.0; x86_64-pc-linux-gnu; 2022-05-24 14:34:41 UTC; unix   
Checked with rchk version 35d5d01a98903a5f3e9ec166dcdb2f790ecb784f LLVM version 12.0.1
More information at https://github.com/kalibera/cran-checks/blob/master/rchk/PROTECT.md
For rchk in docker image see https://github.com/kalibera/rchk/blob/master/doc/DOCKER.md

Suspicious call (two or more unprotected arguments) to Rf_lang3 at vroom_errors::warn_for_errors() const vroom/src/./vroom_errors.h:89

Function read_chr(vroom_vec_info*)::$_1::operator()() const
  [UP] unprotected variable val while calling allocating function vroom::iterator::filename() const vroom/src/vroom_chr.cc:31
  [UP] unprotected variable val while calling allocating function vroom::iterator::index() const vroom/src/vroom_chr.cc:31
  [UP] unprotected variable val while calling allocating function cpp11::writable::r_vector<cpp11::r_string>::operator SEXPREC*() const vroom/src/vroom_chr.cc:34

Function vroom_errors::warn_for_errors() const
  [UP] calling allocating function Rf_findFun(S:warn,?) with argument allocated using Rf_findVarInFrame(?,S:rlang) vroom/src/./vroom_errors.h:86

jennybc avatar May 31 '22 04:05 jennybc

https://builder.r-hub.io/status/vroom_1.5.7.9000.tar.gz-29d57b6da0a344cea75d32def6adb631

This report is dominated by stuff that pertains to cpp11 and, to a much lesser extent, RProgress.

As for the vroom results, there appears to be zero overlap with what I show above from CRAN's current rchk results, despite the fact that, for example, I think read_chr() has not changed between the CRAN version of vroom and what I submitted to r-hub today. warn_for_errors() definitely has and there is no mention of it in my r-hub results from today. That suggests that maybe the recent changes have also incidentally improved things in terms of protection (actual improvements or making the protection more intelligible to rchk).

In terms of vroom-specific results, I only see many instances of this:

#> Function vroom_big_int::Make(vroom_vec_info*) (https://builder.r-hub.io/status/vroom_1.5.7.9000.tar.gz-29d57b6da0a344cea75d32def6adb631#L4436)
#> [PB] has negative depth /home/docker/R-svn/packages/build/NG3k0BAS/vroom/src/./vroom_big_int.h:46 (https://builder.r-hub.io/status/vroom_1.5.7.9000.tar.gz-29d57b6da0a344cea75d32def6adb631#L4437)
#> [UP] attempt to unprotect more items (1) than protected (0), results will be incomplete /home/docker/R-svn/packages/build/NG3k0BAS/vroom/src/./vroom_big_int.h:46 (https://builder.r-hub.io/status/vroom_1.5.7.9000.tar.gz-29d57b6da0a344cea75d32def6adb631#L4438)
#> [PB] has possible protection stack imbalance /home/docker/R-svn/packages/build/NG3k0BAS/vroom/src/./vroom_big_int.h:51

All told, we get that for

  • vroom_big_int::Make()
  • vroom_chr::Make()
  • vroom_chr::Val()
  • vroom_convert()
  • vroom_date::Make()
  • vroom_dbl::Make()
  • vroom_dttm::Make()
  • vroom_fct::Make()
  • vroom_int::Make()
  • vroom_materialize()
  • vroom_num::Make()
  • vroom_rle::Materialize()
  • vroom_time::Make()

I'm guessing those don't appear in CRAN's results because of

In the provided outputs from the tools, some reports were filtered out in the interest of focusing on reports that are most likely true errors. Sometimes the tool knows it is confused by the code (it reports "results will be incomplete" for a function which it does not sufficiently understand). While this may be also because of a true error in the code, these reports were still excluded.

from https://github.com/kalibera/cran-checks/blob/master/rchk/PROTECT.md

jennybc avatar May 31 '22 17:05 jennybc

Upon the release of v1.6.0 I was nudged to address rchk issues (but ultimately it was not a blocker), so remember to re-check for official rchk results once v1.6.0 has been on CRAN a while.

jennybc avatar Sep 30 '22 15:09 jennybc