uint64_t values cause compiler errors

[HBaghdadi1995]

Environment

toml++ version and/or commit hash:

Version: 2.6.0
Hash: 47216c8

Compiler:

MSVC v142 with 10.0.19041.0 for windows SDK

C++ standard mode:

std:c++17

Target arch:

x64

Library configuration overrides:

N/A

Relevant compilation flags:

N/A

Describe the bug

Attempting to use uint64_t or unsigned long long returns the following compile-time error Value initializers must be (or be promotable to) one of the TOML value types

Steps to reproduce (or a small repro code sample)

#include <toml++/toml.h>
#include <cstdint>

int main()
{
    uint64_t foo = 100;

    auto tbl = toml::table{{
        {"example", foo}
    }};
}

Additional information

It looks like this error is caused by line 391 in common.h

integer_value_limits<T>::max <= 9223372036854775807ULL;

This could be fixed by changing it to

integer_value_limits<T>::max <= 18446744073709551615ULL;

I'll see if I can create a PR for this issue.

[marzer]

This could be fixed by changing it to <...>

The trait you're referring to is is_losslessly_convertible_to_native; changing the upper limit to the uint64_t range would be incorrect because TOML integers are 64-bit signed ints. Per the spec:

Arbitrary 64-bit signed integers (from −2^63 to 2^63−1) should be accepted and handled losslessly. If an integer cannot be represented losslessly, an error must be thrown

So that means that C++ uint64_t's are a bit tricky - we can't support them losslessly in TOML. If you know the value is already within the int64_t range, the solution here is for you to explicitly cast the source value beforehand. The alternative solution is to add an implicit cast (with potential integer overflow), which feels like a much worse option to me.

[HBaghdadi1995]

Okay thanks I see the issue now. I wasn't aware of this part of the toml specification.

And having looked more deeply into the code the result would be converted to int64_t in the end anyway, resullting in an integer_overflow.

That being said it would be nice to have a precompile flag to allow such a change to be made.

Thanks for your clarifacation I'll close this issue now.

[englercj]

changing the upper limit to the uint64_t range would be incorrect because TOML integers are 64-bit signed ints

My understanding is that the spec is stating that implementation should support the 64-bit signed integer range, but are not limited to that range. This library can represent uint64 values losslessly, so it doesn't need to throw an error for those values.

I also think the "cannot be represented losslessly" comment is separate from the recommended supported range. It isn't saying to throw an error if the value doesn't fit into the signed integer range, but rather if your implementation can't represent the value losslessly. For example JS can't represent that range losslessly (without BigInt) so it can throw an error for values outside the supported native range. Here in C++ full uint64_t range could certainly be supported, and still be compliant.

Discussion on the spec seems to have others come to the same conclusion. Go-toml seems to have come to the same conclusion and is thinking of implementing full uint64 range support.

[marzer]

Well that's a reasonable case, but I won't be changing my implementation. To do so would mean I'd have to add a separate value type for uint64, or do some union shenanigans internally, both of which would massively complicate things.

The spec mandates that I support 64 bit signed integers, so I do. It doesn't mandate that I support all integer types supported by the host platform, so I don't.

Besides, personally I think those other implementations are making a huge mistake. If someone used them to serialise some TOML containing, say, UINT64_MAX, then used a different lib on a different platform to read it, there's good chance it will fail (at best) or silently wrap it around as a negative signed integer (at worst). Adding room for this scenario is a bad idea IMO.

[pelletier]

@englercj Just to be clear about go-toml, we are still thinking it through, leaning towards not supporting uint64, for reasons similar to @marzer's.

[englercj]

Understood, it is of course your prerogative to decide what your library supports. I'm shoving my uint64's into strings currently, but will likely evolve towards my own parser/writer to support my use case. Thanks!