cpp11 icon indicating copy to clipboard operation
cpp11 copied to clipboard

Fix errors resulting from `-Wconversion -Wno-sign-conversion`

Open paleolimbot opened this issue 1 year ago • 2 comments
trafficstars

The M1 runner apparently runs checks with -Wconversion -Wno-sign-conversion -Werror, which has caused the arrow package to have a very long list of compiler warnings/build failures on the package check page ( https://www.stats.ox.ac.uk/pub/bdr/M1mac/arrow.log ). Most of these are in the arrow package (fixed by https://github.com/apache/arrow/pull/39250 ), but a few are in the cpp11 headers.

The biggest change is for as_cpp<>(), templates for which we instantiate in arrow for int64_t, and uint8_t, both of which interact poorly with various pieces of that logic. I'm happy to update this and add some tests but I wanted to check first to see if there's consensus on what should actually happen. A few cases that were exposed by these warnings are:

as_cpp<int64_t>(NA_INTEGER); // static_cast<int64_t>(NA_INTEGER)?
as_cpp<int8_t>(NA_REAL); // error?
as_cpp<int8_t>(NA_INTEGER); // error?
as_cpp<int>(static_cast<double>(INT_MAX) + 1); // error?

paleolimbot avatar Dec 18 '23 19:12 paleolimbot

The warnings have gone away from the CRAN check page; however, I'm still happy to tighten up + test these changes if they're welcome!

paleolimbot avatar Dec 27 '23 01:12 paleolimbot

Thanks for taking a look! I will circle back to this when I get a break from my current project...you've raised excellent points and it would be nice to tighten up the automatic conversion for integral types other than just int.

is it still necessary?

It isn't! I believe that at least -Wconversion and/or -Wno-sign-conversion was turned off in the MacOS M1 runner, and thus the CRAN warning went away.

paleolimbot avatar May 01 '24 19:05 paleolimbot