Improve c++ compatibility of htslib header files

[daviesrob]

Allow room in kputuw_dig2r for the nul byte on the end of the string used to initialize it. gcc doesn't mind it being dropped but g++ complains.

Fix missing stdint.h limit macros when including htslib/kstring.h and htslib/vcf.h in a c++ compilation. The C99 standard says c++ implementations should only define them if __STDC_LIMIT_MACROS
is also defined. C++11 later put them back, but older versions of libc may not provide them without this.

As we can't be sure we're the first to include stdint.h, defining it in our header files may be too late. Instead suitable definitions are added for the macros that are used by each header.

Fixes #771 (kstring.h error: initializer-string for array of chars is too long)

[jmarshall]

The C99 standard was full of crap when it tried to tell the C++ committee what to do, so the comment doesn't need to be so detailed — at this point, disabling these macros in the absence of __STDC_LIMIT_MACROS is just a bug in old compilers/C libraries.

What about just avoiding using INTx_MAX etc in HTSlib's public header files? Certainly for the vcf.h ones, just writing out the numerical values in #define bcf_int8_vector_end etc wouldn't be much of a loss.

Defining SIZE_MAX itself will lead to ill-formed programs if something includes <stdint.h> after "htslib/kstring.h". It's a somewhat theoretical problem (you currently get warnings only if you're using -Wsystem-headers, which most people won't do), but could be avoided by instead defining a local name within the header:

#ifdef SIZE_MAX
#define SIZE_MAX_KS SIZE_MAX
#else
#define SIZE_MAX_KS ((size_t) -1)
#endif
…
// Use SIZE_MAX_KS within htslib/kstring.h
…
#undef SIZE_MAX_KS  // Undef it at the end of the header file

(Undeffing it afterwards so that other code (especially third-party code) just uses the standard nomenclature.)

[daviesrob]

Yes, the vcf.h bit is quite ugly, so it might be better if it defined its own MAX and MIN macros.

I don't see why including stdint.h again should be a problem. htslib/kstring.h will already have included it, and we only create our own SIZE_MAX if stdint.h failed to make one. As we've already included stdint.h, it shouldn't do anything if included again later.

I'd rather not have to jump through hoops in the rest of the file, just because of an ancient c/c++ incompatibility.

[jmarshall]

I don't see why including stdint.h again should be a problem. htslib/kstring.h will already have included it

Oops — you are quite right that <stdint.h> was never likely to operate like <assert.h>/NDEBUG despite the C people asserting that it had a somewhat similar control macro.

It would make me nervous in general to define names that are reserved to the compiler/standard library, especially in a public header, but YMMV.

[daviesrob]

Mk.2 version (just pushed) avoids the problem by not using SIZE_MAX at all. According to Godbolt, the new version of the overflow checks in kstring.h may actually be a bit more efficient.

[jmarshall]

Nice 😄

Now that they exist, you might as well use BCF_MIN/MAX_BT_INT* in vcf.c's bcf_enc_vint() too.

[jmarshall]

These are boundaries on the unencoded int32_t values, so should just be

int32_t max = INT32_MIN, min = INT32_MAX;

and on the lines below you need to lose the + 8.

[valeriuo]

Thanks for spotting that. I was under the impression they were match for match.

[jmarshall]

Thanks — looks good to me now.

[valeriuo]

Closed by 57fe419 and e85b599.