Cyan4973 / xxHash

Extremely fast non-cryptographic hash algorithm
http://www.xxhash.com/
Other
9.16k stars 777 forks source link

Functions not in use under MSVC+clang-cl #556

Closed TheNicker closed 2 years ago

TheNicker commented 3 years ago

The compiler gets irritated due the declaration of the following functions when not in use

XXH3_accumulate_512_scalar
XXH3_scrambleAcc_scalar
XXH3_initCustomSecret_scalar

These may change according to the value of XXH_VECTOR

The issue is that these are annotated with XXH_FORCE_INLINE which resolves to static which makes them local.

Why XXH_FORCE_INLINE resolves to static, is it intentionally or a mistake ?

There are several solutions, one of them is to exclude these entirely according to the XXH_VECTOR compilation constant.

Cyan4973 commented 3 years ago

Why XXH_FORCE_INLINE resolves to static, is it intentionally or a mistake ?

It's intentional. These symbols should not be accessible / visible outside of their unit.

The compiler gets irritated due the declaration of the following functions when not in use

These warnings are enabled by high level of compiler warnings. But they are supposed to be already taken care of thanks to the bundled __attribute__((unused)) statement. Note that this statement is only enabled for _GNUC_, aka gcc, and it also applies to other compilers defining _GNUC_, such as clang.

TheNicker commented 3 years ago

Why XXH_FORCE_INLINE resolves to static, is it intentionally or a mistake ?

I meant by that to the conjunction of static and inline using a single constant XXH_FORCE_INLINE which seems intentional to save some typing work and for a cleaner look, but a bit misleading.

I believe force inline and unused should be separated, another constant can define both e.g:

#define XXH_MAYBE_UNUSED
#define XXH_FORCE_INLINE
#define XXH_PRIVATE_CALL XXH_FORCE_INLINE XXH_MAYBE_UNUSED

Then replace XXH_FORCE_INLINE with XXH_PRIVATE_CALL