New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alternate way to grab 'long long' type #829

Merged
merged 4 commits into from Mar 8, 2018

Conversation

Projects
None yet
2 participants
@kevinushey
Contributor

kevinushey commented Mar 7, 2018

This PR is another attempt to grab 64-bit integer types in C++98 mode in a way that won't trigger pedantic compiler warnings. Fortunately, seems to work on my Windows box using the latest Rtools toolchain.

@kevinushey

This comment has been minimized.

Contributor

kevinushey commented Mar 7, 2018

For context, the Rtools toolchain has a c++config.h file with the contents:

/* Define if int64_t is available in <stdint.h>. */
#define _GLIBCXX_HAVE_INT64_T 1

/* Define if int64_t is a long. */
/* #undef _GLIBCXX_HAVE_INT64_T_LONG */

/* Define if int64_t is a long long. */
#define _GLIBCXX_HAVE_INT64_T_LONG_LONG 1

which I'm making use of in choosing the int64_t and uint64_t types as 'aliases' for long long and unsigned long long.

@kevinushey

This comment has been minimized.

Contributor

kevinushey commented Mar 7, 2018

This also makes me think there might be a chance that we could sneak in long long support on other platforms with C++98. I feel like venturing there will open some old scars but it sure would sure be nice to just have long long everywhere.

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Mar 7, 2018

That is a bit of a "riskier" approach as it touches the old part. Oh I see you just mod'ed that.

@kevinushey

This comment has been minimized.

Contributor

kevinushey commented Mar 7, 2018

I think this should be ready of merge. Just to summarize the changes:

  1. We explicitly enable long long and unsigned long long when the compiler reports it's compiling in C++11 (or above) mode. (Previously I think this was caught implicitly based on whether C++0x extensions were enabled, which might change in the future)

  2. We also enable long long for older gcc compilers where long long is just an alias is int64_t (which is true with the Windows toolchain, currently). Fortunately, glibc reports this to us with the _GLIBCXX_HAVE_INT64_T_LONG_LONG define;

  3. Finally, we allow the old logic wherein long long is brought in if we detect C++0x extensions are enabled (which appears to be true for clang by default, and for some flavors of older gcc)

I think this effectively lets us use long long on 64bit Windows (and other 64bit platforms) in a CRAN-compliant way, even if C++11 support has not been explicitly enabled.

If we find out this doesn't work as expected or we end up with warnings on some other platform we can of course back it out, but thus far things look good on my Windows VM, macOS machine and Ubuntu 16.04 VM (with gcc-5).

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Mar 8, 2018

This is good stuff, and I had it go over a rev.dep run as well.

@eddelbuettel eddelbuettel merged commit 1a0d3c5 into master Mar 8, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@eddelbuettel eddelbuettel deleted the feature/windows-long-long branch Mar 8, 2018

@kevinushey

This comment has been minimized.

Contributor

kevinushey commented Mar 8, 2018

Thanks!

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Mar 8, 2018

Shall we call it a day and ship this now as Rcpp 0.12.16?

@kevinushey

This comment has been minimized.

Contributor

kevinushey commented Mar 8, 2018

Sounds good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment