New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Floating Point Parser Behavior Is Locale Dependent #3270

Closed
fundamental opened this Issue Nov 23, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@fundamental

Mruby's parse.y uses libc's strtod to parse floating point literals. For some locale configurations (those which use ',' in place of '.' as a decimal separator) this results in incorrectly parsed literals. Cruby appears to work around this with by using their own implementation, ruby_strtod which by default (unless USE_LOCALE is explicitly enabled) ignores locale settings when parsing ruby code.

A similar solution would be preferred here as setlocale cannot be safely used in applications where mruby is embedded within another program (e.g. for the case of plugins which make use of mruby).

@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Nov 24, 2016

Member

Hmm, difficult problem. The ideal solution must be reimplementing strtod(), but it would increase the size of mruby.

Member

matz commented Nov 24, 2016

Hmm, difficult problem. The ideal solution must be reimplementing strtod(), but it would increase the size of mruby.

@fundamental

This comment has been minimized.

Show comment
Hide comment
@fundamental

fundamental Nov 27, 2016

If the resulting binary size is the main concern it should only affect binaries that pull in the mruby-compiler gem, but if the size of the overall codebase is the concern I don't see much of a way to work around that.

If the resulting binary size is the main concern it should only affect binaries that pull in the mruby-compiler gem, but if the size of the overall codebase is the concern I don't see much of a way to work around that.

@dabroz

This comment has been minimized.

Show comment
Hide comment
@dabroz

dabroz Nov 27, 2016

Contributor

"1.25".to_f uses strtod as well.

Contributor

dabroz commented Nov 27, 2016

"1.25".to_f uses strtod as well.

@dabroz

This comment has been minimized.

Show comment
Hide comment
@dabroz

dabroz Nov 27, 2016

Contributor

Having said that, we could use strtod_l (or _strtod_l) on systems that are known to have it.

Contributor

dabroz commented Nov 27, 2016

Having said that, we could use strtod_l (or _strtod_l) on systems that are known to have it.

@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Nov 28, 2016

Member

I didn't know about strtod_l. I am sorry that it's not portable.
I found out that the locale insensitive strtod is not that big. I will merge it soon.

Member

matz commented Nov 28, 2016

I didn't know about strtod_l. I am sorry that it's not portable.
I found out that the locale insensitive strtod is not that big. I will merge it soon.

@matz matz closed this in a0fbc46 Dec 3, 2016

matz added a commit that referenced this issue Dec 3, 2016

@fundamental

This comment has been minimized.

Show comment
Hide comment
@fundamental

fundamental Dec 3, 2016

Thanks for the fix.

Thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment