Compilation issue: UINT64_MAX/UINT64_C not declared in scope

[snsansom]

R 3.6.0
dqrng 0.2.0
Centos 6 with gcc 5.3.0

I encountered the following compilation issue:

                 from dqrng.cpp:19:
../inst/include/xoshiro.h: In static member function ‘static constexpr dqrng::xoshiro<N, A, B, C>::result_type dqrng::xoshiro<N, A, B, C>::max()’:
../inst/include/xoshiro.h:80:53: error: ‘UINT64_MAX’ was not declared in this scope
   inline static constexpr result_type max() {return UINT64_MAX;};
                                                     ^
../inst/include/xoshiro.h: In member function ‘void dqrng::xoshiro<N, A, B, C>::jump() [with long unsigned int N = 2ul; signed char A = 24; signed char B = 16; signed char C = 37]’:
../inst/include/xoshiro.h:144:31: error: ‘UINT64_C’ was not declared in this scope
       if (JUMP[i] & UINT64_C(1) << b) {

Adding the following lines to inst/include/xoshiro.h (after the #include cstdint statement) fixes the issue:

#ifndef UINT64_MAX
#define UINT64_MAX ((uint64_t) -1)
#endif

#ifndef UINT64_C
#define UINT64_C(c) c
#endif

[rstub]

These are odd errors, since both these macros should be defined in /usr/include/stdint.h, which is included by cstdint. Probably due to an old glibc version, c.f. DerrickWood/kraken2#29 (comment). Can you have a look into stdint.h if that requires __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS to be defined?

[snsansom]

Hi, thanks for the comment.

Yes this is almost certainly due to an old glibc version:

ldd (GNU libc) 2.12
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Written by Roland McGrath and Ulrich Drepper.

/usr/include/stdint.h has these lines:

$ egrep -A1 -B3  "__STDC_LIMIT_MACROS|__STDC_CONSTANT_MACROS" /usr/include/stdint.h 

/* The ISO C99 standard specifies that in C++ implementations these
   macros should only be defined if explicitly requested.  */
#if !defined __cplusplus || defined __STDC_LIMIT_MACROS

--

/* The ISO C99 standard specifies that in C++ implementations these
   should only be defined if explicitly requested.  */
#if !defined __cplusplus || defined __STDC_CONSTANT_MACROS

[LTLA]

As it stands, the UINT64_C macro suggested by @snsansom actually doesn't do anything - which may be fine for some platforms, but exposes the code to UB on others if b is ever greater than 31.

If we're fixing it by adding macros, we should probably use a more C++ idiomatic approach:

// Not tested!
#ifndef UINT64_C
#define UINT64_C(c) std::static_cast<std::uint64_t>(c)
#endif

#ifndef UINT64_MAX
#define UINT64_MAX UINT64_C(-1)
#endif

[rstub]

Instead of defining our own macros, I would prefer to use those provided in stdint.h. Several possibilities come to mind:

1. Wrap all #include <cstdint> with code that (un)defines __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS, similar to what old cstdint do when including stdint.h.

2. Write a configure script that checks for the presence of the needed macros. If one is missing, -D__STDC_LIMIT_MACROS or -D__STDC_CONSTANT_MACROS is added to the compiler flags.

3. Like two, but also check that defining these macros does solve the issue.

Unfortunately I cannot reproduce the issue. A centos:6 docker container uses a very old g++ by default that wraps the #include <stdint.h>.

@snsansom How did you install gcc 5.3.0 on CentOS 6? Are you willing to test-drive a configure based solution?

[LTLA]

Whatever solution you decide on, keep in mind that the same solution will need to be used for all downstream packages that #include "xoshiro.h" in their C++ code. Something like 1 that could be placed in xoshiro.h directly would be simplest and transparent to downstream code.

[rstub]

I have now implemented 1 in a separate header file, since both xoshiro.h and dqrng_generator.h need that. That will not work if the cstdint or stdint.h is included before, though.

@snsansom Can you give https://github.com/daqana/dqrng/tree/issue/27 a try? E.g. via

remotes::install_github("daqana/dqrng#30")

[snsansom]

Many thanks both. Unfortunately

remotes::install_github("daqana/dqrng#30")

did not fix the issue for me: compile.messages.log

[rstub]

That's to bad. Can you tell me then how you installed gcc 5.3.0 on CentOS 6? This might help me reproducing the issue.

[snsansom]

Sure gcc was compiled with a custom install location (--prefix) and installed as a bash module with the "bin" location prepended to $PATH and the "lib" + "lib64" locations prepended to the $LD_LIBRARY_PATH.

So far we've no issues with other R packages (>400, including many from Bioconductor) on R 3.4, 3.5 or 3.6. With this said if this is not an issue for anyone else we can easily work around!

[rstub]

I think I have found the reason why my patch did not solve the issue: stdint.h already gets included via Rcpp.h. So either I define the macros in case they are missing, or go for the autoconf route described above, which is difficult with dependent packages as noted by @LTLA. Reordering the includes would be fragile and won't work with things like Rcpp:cppFunction.

What would your workaround entail? Since the issue here is missing C++11 compatibility I am inclined to mark this as "wontfix" for now. BTW, searching for UINT64_MAX and UINT64_C in https://github.com/cran/ gives a few hits, but mostly from C code.

[LTLA]

Can we just modify xoshiro.h to not use C-style code? The first macro can be replaced with numeric_limits<std::uint64_t>::max() and the second macro can be replaced with a static_cast.

[rstub]

Problem is that xoshiro.h is not the only file using these macros:

 $ grep UINT64 -r inst/include/
inst/include/pcg_uint128.hpp:#if UINT64_MAX == ULONG_MAX
inst/include/pcg_uint128.hpp:#elif UINT64_MAX == ULLONG_MAX
inst/include/pcg_uint128.hpp:#if UINT64_MAX == ULONG_MAX
inst/include/pcg_uint128.hpp:#elif UINT64_MAX == ULLONG_MAX
inst/include/dqrng_generator.h:  static constexpr result_type max() {return UINT64_MAX;};
inst/include/dqrng_generator.h:  static_assert(RNG::max() == UINT64_MAX, "Provided RNG has wrong maximum.");
inst/include/xoshiro.h:  inline static constexpr result_type max() {return UINT64_MAX;};
inst/include/xoshiro.h:      if (JUMP[i] & UINT64_C(1) << b) {
inst/include/xoshiro.h:                 if (LONG_JUMP[i] & UINT64_C(1) << b) {
inst/include/xoshiro.h:      if (JUMP[i] & UINT64_C(1) << b) {
inst/include/xoshiro.h:                 if (LONG_JUMP[i] & UINT64_C(1) << b) {
inst/include/dqrng_distribution.h:  return (x >> 11) * (1. / (UINT64_C(1) << 53));

It is just the first to be compiled.

[LTLA]

Hm, okay. It would be very unpalatable to have to edit the PCG headers, that would break our ability to just copy-and-paste updates of the PCG source.

I guess we could just manually define UINT64_MAX and friends in mystdint.h? Not particularly principled, but it should work in @snsansom's case and it shouldn't hurt anyone else.

I would also be content with leaving it as it is; but I would request adding a SystemRequirements: C++11 to the DESCRIPTION to make it clear that we need C++11. Incidentally, this would allow you to remove the corresponding line in the Makevars.

[rstub]

Makevars is prefered over DESCRIPTION, c.f. WRE. Anyway, I have now replaced the UINT64_C macro and defined a fallback UINT64_MAX. @snsansom Can you give remotes::install_github("daqana/dqrng#30") another shot, please?

[snsansom]

@rstub Brilliant that fixes the issue! Thanks all for the help with this.