[wip], use to/from_chars in attribute_value and lexical_cast

[gfgtdf]

This makes attribute_value and lexical_cast use the "new" to/from_chars api.

Its main advantages are:

• It's guaranteed to be locale independent, hopefully fixing all cases of #3945 and similar
• It's faster

So far afaik the only compiler which has a complete and proper to/from_chars implementation is msvc, gccs implementation of from_chars sometimes uses strtod under the hood and clang imply hasn't implemented from_chars for floating point numbers yet at all. Luckily for us, there is now also boost::charconv that can be used. So this raises to minimum build requirement to have at least one of:

• msvc 2019 update 5
• gcc 11
• clang 14 (i have added a fallback implementation of from_chars for this case, that doesn't support all of its features, and is probably certainly not as fast as the boost version, but supports the features that we use from it)
• boost 1.85

Since in particular the gcc implementation isn't that good (at least it on gcc11), boost charconv is the preferred implementation that is used if available. TODO:

• [ ] the macosx ci buildfails, , so it will need to be updated , preferably to use boost 1.85
• [x] the steam runtime ci build fails, , so it will need to be updated , preferably to use boost 1.85
• [x] The test also haven't been updated yet for the changes in lexical_cast
• [ ] makefile updates for boost charconv
• [x] figure out when exactly -lquadmathis needed
• [ ] commits cleanup

[gfgtdf]

@hrubymar10 Could you update https://github.com/hrubymar10/MacCompileStuff to use boost 1.85 with the charconv library enabled ?

[Vultraz]

This would replace the minimal wrapper I added (utils/from_chars) and its use in image_modifications.cpp, I take it?

[gfgtdf]

This would replace the minimal wrapper I added (utils/from_chars) and its use in image_modifications.cpp, I take it?

I haven't thought about this yet, it has a slightly different interface than lexical_cast so it might be useful? Not sure.

[CelticMinstrel]

This would replace the minimal wrapper I added (utils/from_chars) and its use in image_modifications.cpp, I take it?

I would say yes.

[Vultraz]

FTR, it used to use (in most cases) lexical_cast_default, and it seemed to make sense to have an API which used optional::value_or.

[gfgtdf]

Also utils::from_chars is slightly different in what it supports,as it doesn't support leading plus signs, not sure whether that's intentional or not.

[gfgtdf]

FTR, it used to use (in most cases) lexical_cast_default, and it seemed to make sense to have an API which used optional::value_or.

Looking at the image_modifications.cpp code i wonder whether most of the case shoudl print an error instead of just using '0' when the argument doesn't parse to a number.

[Pentarctagon]

Regarding boost 1.85 on macOS - I asked hrubymar and that will take a while to do since updating MCS is always tricky.

[gfgtdf]

yeah, this still waits for boost 1.85 or better 1.86 for the macos compile stuff

[Vultraz]

Currently this code will fail if added to the tests:

	c["x"] = "9.87654321";
	BOOST_CHECK_EQUAL(c["x"], 9.87654321);

saying that check c["x"] == 9.87654321 has failed [9.87654321 != 9.8765432099999995]. I imagine this PR would fix that?

[CelticMinstrel]

I imagine this PR would fix that?

No. I don't think it would. Round-trip conversion means that a string that was converted from a double is guaranteed to convert back to the same double. There is no guarantee that an arbitrary string you compose will convert to exactly the double it looks like.

So this PR would make the following code work (if it doesn't already, I didn't test it):

	c["x"] = 9.87654321;
	BOOST_CHECK_EQUAL(c["x"], 9.87654321);

[gfgtdf]

I imagine this PR would fix that?

it's quite possible, the to_chars functions are a bit stricter in its guarantees in how numbers are represtented as chars, especially compared to whatever else we are currentls using. But i cannot guarantee that it fixes this one we'd have to test it then

[Pentarctagon]

This is still waiting on boost 1.85+ on macOS, I assume?

[gfgtdf]

This is still waiting on boost 1.85+ on macOS, I assume?

Yes

[Pentarctagon]

@hrubymar10 or @soliton- can this be done?

[hrubymar10]

@hrubymar10 or @soliton- can this be done?

Yes, nearly done

[hrubymar10]

@gfgtdf please rebase onto latest master. The new MCS is there now.

CC @Pentarctagon

[soliton-]

gfgtdf please rebase onto latest master. The new MCS is there now.

It's getting annoying that you ignore all issues and PRs on MCS.

[gfgtdf]

@gfgtdf please rebase onto latest master. The new MCS is there now.

I rebased onto latest master but it seems like its still not working, the error is

Ld /Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/Release/wesnothd normal (in target 'wesnothd' from project 'The Battle for Wesnoth')
    cd /Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode
    /Applications/Xcode_15.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -Xlinker -reproducible -target x86_64-apple-macos10.13 -isysroot /Applications/Xcode_15.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk -Os -L/Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/EagerLinkingTBDs/Release -L/Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/Release -L/Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/lib -F/Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/EagerLinkingTBDs/Release -F/Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/Release -F/Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/lib -filelist /Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/The\ Battle\ for\ Wesnoth.build/Release/wesnothd.build/Objects-normal/x86_64/wesnothd.LinkFileList -Xlinker -rpath -Xlinker @loader_path/../Frameworks -Xlinker -rpath -Xlinker @loader_path/The\ Battle\ for\ Wesnoth.app/Contents/Frameworks -Xlinker -rpath -Xlinker @loader_path/Frameworks -Xlinker -object_path_lto -Xlinker /Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/The\ Battle\ for\ Wesnoth.build/Release/wesnothd.build/Objects-normal/x86_64/wesnothd_lto.o -stdlib\=libc++ -lz -lbz2 -Wl -ld_classic -framework CoreFoundation -lboost_locale-mt -lboost_coroutine-mt -lssl.1.1 -lboost_random-mt -lboost_system-mt -lpcre.1 -lboost_filesystem-mt -lcrypto.1.1 -lboost_iostreams-mt -lboost_context-mt -Xlinker -no_adhoc_codesign -Xlinker -dependency_info -Xlinker /Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/The\ Battle\ for\ Wesnoth.build/Release/wesnothd.build/Objects-normal/x86_64/wesnothd_dependency_info.dat -o /Users/runner/work/wesnoth/wesnoth/projectfiles/Xcode/build/Release/wesnothd
Undefined symbols for architecture x86_64:
  "boost::charconv::from_chars(char const*, char const*, double&, boost::charconv::chars_format)", referenced from:
      config_attribute_value::operator=(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&) in config_attribute_value.o
      implementation::lexical_caster<double, std::__1::basic_string_view<char, std::__1::char_traits<char>>, void, void>::operator()(std::__1::basic_string_view<char, std::__1::char_traits<char>>, boost::optional<double>) const in config_attribute_value.o
  "boost::charconv::to_chars(char*, char*, double, boost::charconv::chars_format)", referenced from:
      bool (anonymous namespace)::str_equals_number<double>(std::__1::basic_string_view<char, std::__1::char_traits<char>>, double) in config_attribute_value.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

there is also no mention of charconv (like there is lboost_filesystem for filesytem in the linker command)

[gfgtdf]

saying that check c["x"] == 9.87654321 has failed [9.87654321 != 9.8765432099999995]. I imagine this PR would fix that?

I enabled that test on this pr and it seems to pass.

[gfgtdf]

@gfgtdf please rebase onto latest master. The new MCS is there now.

It seems to work now after i added it to the wesnothd build, Thx

[hrubymar10]

@gfgtdf please rebase onto latest master. The new MCS is there now.

It seems to work now after i added it to the wesnothd build, Thx

I indeed didn’t include it. Thx 👍🏻

[soliton-]

An update of INSTALL.md is missing.