Skip to content
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

restrict keyword used in header breaks some C++ inclusion #88

Closed
mitchblank opened this issue Sep 2, 2016 · 3 comments
Closed

restrict keyword used in header breaks some C++ inclusion #88

mitchblank opened this issue Sep 2, 2016 · 3 comments

Comments

@mitchblank
Copy link

in xxhash.h:

#if !(defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L))   /* ! C99 */
#  define restrict   /* disable restrict */
#endif

restrict is technically only a keyword in C, not C++ yet this header needs to be includible from either. Most C++ compilers I guess either accept it as an extension or doesn't set __STDC_VERSION__ incorrectly. However, one host in my build farm (a Solaris x86_64 machine with gcc 4.7.3) is getting a compile error when this header is included from C++ code:

include/xxhash.h:233:61: error: expected ',' or '...' before 'dst_state'
include/xxhash.h:234:61: error: expected ',' or '...' before 'dst_state'

According to wikipedia https://en.wikipedia.org/wiki/Restrict

C++ does not have standard support for restrict, but many compilers have equivalents that usually work in both C++ and C, such as the GNU Compiler Collection's and Clang's restrict, and Visual C++'s __restrict and __declspec(restrict).

I'll try fixing it tomorrow, but I'll probably do something like:

#ifdef __cplusplus
#  if defined(__GNUC__) || defined(__clang__)
#    define XXH_RESTRICT __restrict__
#  elif defined (_MSC_VER)
#    define XXH_RESTRICT  __restrict
#  endif
#else
#  if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L)   /* C99 */
#    define XXH_RESTRICT restrict
#  endif
#endif
#ifndef XXH_RESTRICT
#  define restrict   /* disable restrict */
#endif

I think defining an xxhash-specific macro like XXH_RESTRICT is less intrusive than potentially defining restrict which might affect how system headers, etc, are interpreted.

@Cyan4973
Copy link
Owner

Cyan4973 commented Sep 2, 2016

Thanks for feedback.

It's also possible to remove restrict which use case within xxhash is extremely limited.
That would reduce complexity.

@Cyan4973
Copy link
Owner

Cyan4973 commented Sep 2, 2016

restrict removed from source in "dev" branch

@mitchblank
Copy link
Author

I can confirm my previously failing environment builds after the 386a634 change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants