Xxhash: ISO C90 / ANSI C compatability

[Leandros]

It's great you try to keep it compatible with ISO C90 (ANSI C89), however, the typedefs for fixed types is not C90 compatible, while you do not include stdint.h (a header added in C99), the type long long was also added in C99 and does not exist in C90.

This line is here has the non compatible definition.

[Cyan4973]

Unfortunately, I see no way of supporting XXH64 while preserving C90 compatibility without stdint.h support, precisely because of long long.

Should you have a better solution, I'm all hear.

[Leandros]

You can't. C90 doesn't natively know 64-Bit types.

long long is only supported in C90 through a GNU extension. At the current state, compiling xxHash as C99 emits a bunch of:

warning: 'long long' is an extension when C99 mode is not enabled [-Wlong-long]

The only solution I think is feasible is to drop 64-Bit support on native C90, and enable it when compiled with C99 or later or when GNU C is detected.

[petervaro]

First things first: I don't think anyone should support C89/90 anymore, I mean c'mon, we are living in 2016, we already have two new standards, which are not only better and safer, but also widely supported nowadays as well. Of course, backward compatibility is important, but at this particular situation, I have to say, no, please stop supporting systems, which should not be here with us now, and only limiting us. (For example, most of the variable names should be shorter in C89/90, so you should rewrite everything, to get external token names with 6 significant characters only!)

The other thing is, the absence of long long as you two mentioned earlier. Actually, there is a solution for that: you can always implement your own type, which of course will be a hell lot slower, and will not support any of the arithmetic operations by default, and I even have to say, at some level you can not guarantee, that your implementation will work on any exotic platforms..

Long story short: I don't think a C89/C90 support is necessary in 2016, IMO the contrary is true: let's push the industry to use C11 or later standards, and let's get rid of the unnecessary ballasts, we are carrying for almost five decades now!

[Leandros]

I totally agree to @petervaro, that we can _should_ stop supporting ISO C90.

But you have to keep in mind that dropping support for ISO C90 means you also drop support for any C++ before C++11, since the C++ standard only required the compiler to support the ISO C90 subset of C. long long was an extension in C++03 and earlier, and was only added to C++ with C++11.

Unfortunately, there are still compilers in use today which only support ANSI C89 (Hello, Microsoft).

[t-mat]

Since we don't have feature definition macros in standard, I'd like to propose macro XXHASH_STRICT_C90 or XXHASH_DISABLE_XXH64 which disables functionalities which doesn't match C90 standard :

...
...
typedef   signed int       S32;
#if !defined(XXHASH_STRICT_C90)
typedef unsigned long long U64;
#endif
...

#if !defined(XXHASH_DISABLE_XXH64)
XXH_PUBLIC_API unsigned long long XXH64 (const void* input, size_t len, unsigned long long seed);
#endif

• This macro may introduce DLL incompatibility. One possible solution is we restrict it only static link.
• We'll be able to use XXHASH_DISABLE_XXH64 style macro for XXH128.

Since there are many compilers which has non-standard 64bit support, personally I don't recommend automatic feature detection for compilers. But just an FYI:

• MSVC 2003 has long long but doesn't support unsigned long long
• MSVC 2005 supports long long and unsigned long long
  
  
  
  • MSVC 2005 or greater has predefined macro _MSC_VER >= 1400
  
  

---

Personally I strongly :+1: to @petervaro's opinion, but @Leandros described our bitter situation. Older Linux distributions, many embedded platforms (including console gaming ones) and MSVC don't fully support C11/C++11.
Sadly and aside from this issue, but I also must say even GCC doesn't support some C11 functionality yet. So I can't say we can use C11 freely :cry:

[Cyan4973]

I can appreciate the need for a specific version without long long dependency, limited to 32-bits only.
However, is it the best solution to support such scenario directly within the reference implementation, as opposed to a dedicated fork ?

I tend to accept portability patches even to support rare, esoteric or dying environments,
but on the condition that impact is "limited".
Then it requires to define "limited", which I translate into
typically a few lines in a single located place.

If the patch needs to modify many lines, scattered all around the source file, it's no longer a "light adaptation". Therefore it becomes harder to justify its continuous maintenance burden if the beneficiary system isn't exactly mainstream or relevant.

Long story short :

• OK, why not a version without long long dependency, but how to do it with limited impact on code source ?
• Note : there is one trap here, even XXH32 needs long long support, since the streaming version can deal with > 4 GB combined source length in multiple rounds. Though it's certainly possible to replace it with a more convoluted version requiring two 32-bits variables instead. That's part of the difficulties such patch introduces.

[Leandros]

Why does XXH32 require long long? Isn't a simple size_t enough? It'll be 64bit if files larger than 4GB are supported.

[Cyan4973]

It needs to track input sizes > 4 GB.

size_t would be enough if input could only be provided in a single block.
In streaming mode though, input can be provided in multiple successive blocks,
so it's no longer limited by size_t.

[t-mat]

@Leandros, @Cyan4973 may reference XXH32_state_s::total_len. We can support it relatively easily for now, but he may be worrying that we'll care every arithmetic operation eventually.

OK, why not a version without long long dependency, but how to do it with limited impact on code source ?

One possible solution is struct+macro:

struct U64_s { U32 u32[2]; };
void set_U64(U64_s* pu64, U32 high, U32 low);
void add_U64_U32(U64_s* pu64, U32 u32);
int compare_U64_U32(const U64_s* pu64, U32 u32);

but obviously it decrease readability a lot.

So as a beginning, I'd like to support limited functions and only static-linking for 32bit-only platforms.
I think the following functions would be enough for over 50% of their applications:

typedef unsigned int XXH32_hash_t;
XXH_PUBLIC_API XXH32_hash_t XXH32 (const void* input, size_t length, unsigned int seed);

I suppose we can leave streaming hash for now because we don't know actual their demands and limitations.

[Cyan4973]

That's a pretty small scope, so yes it's doable.

So the routing between "limited" and "full" version would be manual, with "full" by default.

Now the name of the macro should be selected.
Both those who you proposed are good @t-mat .

I suspect XXH_STRICT_C90 to be the more explicit (the only reason one would need to compile this mode is to be strictly C90 compliant). Unavailability of XXH64*() in this mode should be underlined in comments.

[Bulat-Ziganshin]

1. There is http://www.azillionmonkeys.com/qed/pstdint.h
2. In small projects people usually just check whether modern C/C++ standars are supported and use stdint.h depending on that:

#ifdef _MSC_VER
  typedef  unsigned __int64 uint64;
  typedef  unsigned __int32 uint32;
  typedef  unsigned __int16 uint16;
  typedef  unsigned __int8  uint8;
#elseif ((defined(__STDC__) && __STDC__ && __STDC_VERSION__ >= 199901L) || (defined (__WATCOMC__) && (defined (_STDINT_H_INCLUDED) || __WATCOMC__ >= 1250)) || (defined(__GNUC__) && (defined(_STDINT_H) || defined(_STDINT_H_)) )) && !defined (_PSTDINT_H_INCLUDED)
# include <stdint.h>
  typedef  uint64_t  uint64;
  typedef  uint32_t  uint32;
  typedef  uint16_t  uint16;
  typedef  uint8_t   uint8;
#else
 ...
#endif

[Bulat-Ziganshin]

there is one trap here, even XXH32 needs long long support

It's required only for total_len and only for the check if (state->total_len >= 16)

By replacing update with

    state->total_len += len;
    state->over4g ||= (state->total_len < len);

and the check with if (state->over4g || state->total_len >= 16) we can make total_len 32-bit.

[Bulat-Ziganshin]

And overall, may be this topic calls for "transposing" the sources - i.e. make a section with XXXH32 and a section with XXH64. It should make simpler adding of XXH128, simd hash and so. One can develop it as separate xxh128.[hc] files and after final approval, easily add new code to the end of xxhash.*

[Cyan4973]

Latest update in the "dev" branch introduces a new macro, XXH_NO_LONG_LONG,
which disables XXH64() section for systems without 64-bits arithmetic support,
such as strict ansi C89.

[Cyan4973]

Implemented in v0.6.2