A recent change to r-devel causes an R CMD check warning when a C file
includes a "#pragma GCC diagnostic ignored" pragma:
https://github.com/wch/r-source/commit/b76c8fd355a0f5b23d42aaf44a879cac0fc31fa4
. This causes the CRAN checks for the "corpus" package to emit a
warning:
https://www.r-project.org/nosvn/R.check/r-devel-linux-x86_64-fedora-clang/corpus-00check.html
.
The offending code is in an upstream library bundled with the package:
https://github.com/patperry/corpus/blob/master/src/table.c#L118
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wtype-limits"
// gcc emits a warning if sizeof(size_t) > sizeof(unsigned)
if ((size_t)size > SIZE_MAX / sizeof(*items)) {
#pragma GCC diagnostic pop
This is code appears in the "corpus" library that gets bundled with
the
corpus r-package but can also be installed by itself. I am the
maintainer for both projects but in theory the library is independent
from the r package (the latter depends on the former). I put the pragma
there in the first place because this is the cleanest way I know of to
remove the gcc compiler warning "comparison is always false due to
limited range of data type" which appears whenever sizeof(unsigned) <
sizeof(size_t); the warning does not appear for clang.
Does anyone have recommendations for what I should do to remove the R
CMD check warning? Is it possible to do this while simultaneously
removing the gcc warning? Note that the package does not use autoconf.
Fortunately, I am the maintainer for the included library, so I can
potentially remove the pragma. However, I can imagine that there are
many other cases of R packages bundling C libraries where R package
maintainers do not have control over the downstream source. Perhaps
there is a compelling case for this new CRAN check that I'm not seeing,
but it seems to me that this additional CRAN check will cause extra work
for package developers without providing any additional safety for R
users. Package developers that do not control bundled upstream libraries
will have to resort to `sed s/^#pragma.*//` or manually patch unfamiliar
code to remove the CRAN warning, potentially introducing bugs in the
process.
Patrick
Hi Patrick, It was recently added as a cran policy (thanks Dirk's cran policy watch: https://twitter.com/markvdloo/status/935810241190617088). It seems to be a general stricter policy on keeping to the C(++) standard. Warnings are there for a reason and should usually not be ignored. I'm not familiar with the warning you are suppressing but it seems likely that your code might assume type size that is not guaranteed by the standard and thus may differ a cross systems/compilers. (An example is wchar_t which has typically 16b on Windows as guaranteed by the standard and 32b on Unix) Best, Mark On Mon, Dec 11, 2017, 4:33 PM Patrick Perry <pperry at stern.nyu.edu> wrote:> A recent change to r-devel causes an R CMD check warning when a C file > includes a "#pragma GCC diagnostic ignored" pragma: > > https://github.com/wch/r-source/commit/b76c8fd355a0f5b23d42aaf44a879cac0fc31fa4 > . This causes the CRAN checks for the "corpus" package to emit a > warning: > > https://www.r-project.org/nosvn/R.check/r-devel-linux-x86_64-fedora-clang/corpus-00check.html > . > > The offending code is in an upstream library bundled with the package: > https://github.com/patperry/corpus/blob/master/src/table.c#L118 > > #pragma GCC diagnostic push > #pragma GCC diagnostic ignored "-Wtype-limits" > // gcc emits a warning if sizeof(size_t) > sizeof(unsigned) > > if ((size_t)size > SIZE_MAX / sizeof(*items)) { > #pragma GCC diagnostic pop > > This is code appears in the "corpus" library that gets bundled with the > corpus r-package but can also be installed by itself. I am the > maintainer for both projects but in theory the library is independent > from the r package (the latter depends on the former). I put the pragma > there in the first place because this is the cleanest way I know of to > remove the gcc compiler warning "comparison is always false due to > limited range of data type" which appears whenever sizeof(unsigned) < > sizeof(size_t); the warning does not appear for clang. > > Does anyone have recommendations for what I should do to remove the R > CMD check warning? Is it possible to do this while simultaneously > removing the gcc warning? Note that the package does not use autoconf. > > Fortunately, I am the maintainer for the included library, so I can > potentially remove the pragma. However, I can imagine that there are > many other cases of R packages bundling C libraries where R package > maintainers do not have control over the downstream source. Perhaps > there is a compelling case for this new CRAN check that I'm not seeing, > but it seems to me that this additional CRAN check will cause extra work > for package developers without providing any additional safety for R > users. Package developers that do not control bundled upstream libraries > will have to resort to `sed s/^#pragma.*//` or manually patch unfamiliar > code to remove the CRAN warning, potentially introducing bugs in the > process. > > > Patrick > > ______________________________________________ > R-devel at r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-devel >[[alternative HTML version deleted]]
Dear Mark,
Thank you for the reply. I am not questioning the utility of compiler
warnings in general. If you look at the code snippet, you can see that I
am only disabling one warning ("-Wtype-limits"), in one line of code.
This particular warning is spurious, so I want to ignore it, but I want
warnings to be active for the rest of the project.
The specific issue is that I have a dynamic array of integers with an
int length. When I grow the array, I want to make sure that the size
doesn't overflow a size_t. I have an overflow check of the form
int n; // array size
if ((size_t)n > SIZE_MAX / sizeof(int)) {
// handle overflow
}
The condition will never be true on a 64-bit architecture with
sizeof(int) == 4 and sizeof(size_t) == 8 but it might be true on a
32-bit architecture with sizeof(int) == sizeof(size_t). When compiled on
a 64-bit architecture with "-Wextra", gcc emits a warning. Clang does
not. I added the pragmas to disable this warning.
You might think that I could fix things by doing something like
#if sizeof(size_t) == sizeof(int)
/// check for overflow
#endif
but that won't work, because you can't use sizeof in a static #if. The
standard work-around is to use autoconf to define SIZEOF_SIZE_T and
SIZEOF_INT as in
https://stackoverflow.com/questions/1921295/x64-compatible-c-source ,
but I'd rather not add a dependency on autoconf.
Patrick> Mark van der Loo <mailto:mark.vanderloo at gmail.com>
> December 11, 2017 at 3:42 PM
>
> Hi Patrick,
>
> It was recently added as a cran policy (thanks Dirk's cran policy
> watch: https://twitter.com/markvdloo/status/935810241190617088).
>
> It seems to be a general stricter policy on keeping to the C(++)
> standard. Warnings are there for a reason and should usually not be
> ignored. I'm not familiar with the warning you are suppressing but it
> seems likely that your code might assume type size that is not
> guaranteed by the standard and thus may differ a cross
> systems/compilers. (An example is wchar_t which has typically 16b on
> Windows as guaranteed by the standard and 32b on Unix)
>
> Best,
> Mark
>
>
> Patrick Perry <mailto:pperry at stern.nyu.edu>
> December 11, 2017 at 10:32 AM
> A recent change to r-devel causes an R CMD check warning when a C file
> includes a "#pragma GCC diagnostic ignored" pragma:
>
https://github.com/wch/r-source/commit/b76c8fd355a0f5b23d42aaf44a879cac0fc31fa4
> . This causes the CRAN checks for the "corpus" package to emit a
> warning:
>
https://www.r-project.org/nosvn/R.check/r-devel-linux-x86_64-fedora-clang/corpus-00check.html
> .
>
> The offending code is in an upstream library bundled with the package:
> https://github.com/patperry/corpus/blob/master/src/table.c#L118
>
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wtype-limits"
> // gcc emits a warning if sizeof(size_t) > sizeof(unsigned)
>
> if ((size_t)size > SIZE_MAX / sizeof(*items)) {
> #pragma GCC diagnostic pop
>
> This is code appears in the "corpus" library that gets bundled
with
> the corpus r-package but can also be installed by itself. I am the
> maintainer for both projects but in theory the library is independent
> from the r package (the latter depends on the former). I put the
> pragma there in the first place because this is the cleanest way I
> know of to remove the gcc compiler warning "comparison is always false
> due to limited range of data type" which appears whenever
> sizeof(unsigned) < sizeof(size_t); the warning does not appear for
clang.
>
> Does anyone have recommendations for what I should do to remove the R
> CMD check warning? Is it possible to do this while simultaneously
> removing the gcc warning? Note that the package does not use autoconf.
>
> Fortunately, I am the maintainer for the included library, so I can
> potentially remove the pragma. However, I can imagine that there are
> many other cases of R packages bundling C libraries where R package
> maintainers do not have control over the downstream source. Perhaps
> there is a compelling case for this new CRAN check that I'm not
> seeing, but it seems to me that this additional CRAN check will cause
> extra work for package developers without providing any additional
> safety for R users. Package developers that do not control bundled
> upstream libraries will have to resort to `sed s/^#pragma.*//` or
> manually patch unfamiliar code to remove the CRAN warning, potentially
> introducing bugs in the process.
>
>
> Patrick
[[alternative HTML version deleted]]