HaxeFoundation / hxcpp

Runtime files for c++ backend for haxe
Other
298 stars 189 forks source link

Make Std.parseFloat() locale independant? #762

Open jeremyfa opened 5 years ago

jeremyfa commented 5 years ago

Currently, Std.parseFloat() in HXCPP relies on strtod():

https://github.com/HaxeFoundation/hxcpp/blob/31500dc4bdc1b78729f6865c5a419a4ec5e2acaf/src/hx/StdLibs.cpp#L669-L683

The problem with strtod() is that its parsing of floats is dependant on the current locale. I learnt that the hard way when updating SDL to version 2.0.9, which is changing the locale in one of its dependencies (hidapi), making, for instance, Std.parseFloat('3.5') return 3 instead of 3.5 because the new locale was accepting , instead of ., breaking my whole project code which relies on Std.parseFloat to parse large JSON file (for Spine animations).

My current workaround is to ensure the locale for parsing numbers is:

setlocale(LC_NUMERIC, "C");

But ideally, it may be nice to ensure Std.parseFloat() doesn't depend at all on the locale. What are your thoughs on this?

EricBishton commented 5 years ago

The API docs for Std.parseFloat() are clear (https://api.haxe.org/Std.html):

Additionally, decimal notation may contain a single . to denote the start of the fractions.

Therefore, the implementation using strtod() is incorrect.

hughsando commented 5 years ago

I think setting the numeric locale looks like the best solution. Does that workaround do the job? I can look to insert that code in some appropriate place.

jeremyfa commented 7 months ago

I'm following up with this issue posted a long time ago, sorry about replying only now.

Yes, using setlocale(LC_NUMERIC, "C"); at regular interval solved it for me, although I'm not very comfortable with setting a global flag to handle this issue, as it could cause problem when creating a Haxe/C++ library that is intended to be distributed to other developers who might rely on another setting of that flag.

jeremyfa commented 7 months ago

Looking for alternative to setting locale globally, I found 2 other options:

  1. Provide a locale-independent strtod by caching a "C" locale: https://gist.github.com/tknopp/9271244 . Sounds like a good approach that can be thread safe as well as you could cache the locale once hxcpp inits and just reuse it after.

  2. Use a fully re-implemented strtod. The one from ruby source seems the most accurate: https://github.com/apple-open-source/macos/blob/master/ruby/ruby/util.c#L2024 . But that looks like an overkill solution if the first option works.

I'll explore the first solution with cached locale and see how it goes. Might make a pull request later if results are good

Shallowmallow commented 1 month ago

Looking for alternative to setting locale globally, I found 2 other options:

1. Provide a locale-independent strtod by caching a "C" locale: https://gist.github.com/tknopp/9271244 . Sounds like a good approach that can be thread safe as well as you could cache the locale once hxcpp inits and just reuse it after.

2. Use a fully re-implemented strtod. The one from ruby source seems the most accurate: https://github.com/apple-open-source/macos/blob/master/ruby/ruby/util.c#L2024 . But that looks like an overkill solution if the first option works.

I'll explore the first solution with cached locale and see how it goes. Might make a pull request later if results are good

Did it work out with the first solution?