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.vander...@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:ppe...@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]]
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
