marzer / tomlplusplus

Header-only TOML config file parser and serializer for C++17.
https://marzer.github.io/tomlplusplus/
MIT License
1.53k stars 146 forks source link

TOML parser toml::parse_file throws error when parsed floating point value is not IEEE-754 compliant #184

Closed aborisovich closed 1 year ago

aborisovich commented 1 year ago

Is your feature request related to a problem? Please describe.

I've tried to use TOML format files for some configuration files and stored random decimal float values there. Turned out that TOML format does not support decimal floating points but only binary floating points according to IEEE-754 specification. This topic had already been discussed in TOML language specification in https://github.com/toml-lang/toml/issues/572 . Please also note that "float" in C++ language is default understood to be a decimal representation.

Describe the solution you'd like toml::parse function should throw parsing error when parsed floating point is not IEEE-754 compliant and its value will be changed using rounding up for the nearest binary float representation. After all, end-user expects that value provided in the input file will be exactly the same as the value returned by toml parser. For more understanding have a look at https://www.h-schmidt.net/FloatConverter/IEEE754.html . Validation of floating point values compliance can be done using standard library https://en.cppreference.com/w/cpp/types/numeric_limits/is_iec559 .

Additional context Issue discovered in #179 .

marzer commented 1 year ago

Sorry but I disagree with this, chiefly because it's underpinned by a nonsensical assumption:

After all, end-user expects that value provided in the input file will be exactly the same as the value returned by toml parser.

I do not think that in the general case people expect floating-point values to be exactly represented, by their very nature. This property of them is well-known.

If that behaviour is mission-critical, then people will write their own validation code as you suggest. That's a feature, not a bug.

marzer commented 1 year ago

As an aside,

Validation of floating point values compliance can be done using standard library

I am already performing that check. It shouldn't even compile on platforms where double is not IEC 559:

https://github.com/marzer/tomlplusplus/blob/698285d9b2f3f6756fcdab8b93f60352325764e1/include/toml%2B%2B/impl/forward_declarations.h#L41

aborisovich commented 1 year ago

Well, I guess I need to my own TOML library, where authors will be keener on improving the code that closing feature requests. Toml::parse is meant to be "validation code", cause is obvious as the end-user is confused because the behavior is not what the developer would expect. Receiving different values that you entered in the file will lead to a painful investigation like mine. People waisting time just because toml library author does not want to provide them with meaningful explanation and error message.

marzer commented 1 year ago

authors will be keener on improving the code

I am very keen on improving the library, but I disagree that this is actually an improvement, and I explained my reasoning above. Your entitled attitude can fuck off.