Skip to content
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

float_of_string accepts underscore + whitespace + numerical part #7188

Closed
vicuna opened this issue Mar 19, 2016 · 3 comments
Closed

float_of_string accepts underscore + whitespace + numerical part #7188

vicuna opened this issue Mar 19, 2016 · 3 comments

Comments

@vicuna
Copy link

vicuna commented Mar 19, 2016

Original bug ID: 7188
Reporter: skrah
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2017-09-24T15:32:06Z)
Resolution: won't fix
Priority: low
Severity: minor
Version: 4.02.1
Category: standard library

Bug description

The first of these two should probably also raise:

float_of_string "___ 1.2";;

  • : float = 1.2

float_of_string "1.2 ___";;

Exception: Failure "float_of_string".

@vicuna
Copy link
Author

vicuna commented Mar 19, 2016

Comment author: @gasche

The current implementation of float_of_string can be found there:
https://github.com/ocaml/ocaml/blob/78293a0/byterun/floats.c#L258-L296

it removes all underscores (accepted inside OCaml literals, eg. 123_456) before calling the C strtod function. The only difference between those two inputs (after underscore removal) is that whitespace is either before or after the input. (On my system?) strtod is specified to ignore optional whitespace before the string, but not after, which explains the behavior.

I don't think it is necessary for OCaml to add complex workarounds for the limitations or surprising parts of those functions implementation or specification.

@vicuna
Copy link
Author

vicuna commented Mar 19, 2016

Comment author: skrah

The problem is that underscores outside OCaml literals are accepted here.
I'm happy with "won't fix", but seems to me that underscores should only be
removed when they are actually part of a literal, like "___1.2__e__222".

@vicuna
Copy link
Author

vicuna commented Mar 19, 2016

Comment author: @gasche

My impression is that although this behavior is indeed quirky, it is not a bug either (it parses float just fine), and trying to fix it would risk introducing regressions. However, if someone proposes a patch that is nice (simple and readable) that fixes this and is easy to analyze for absence of regressions, why not?

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

No branches or pull requests

2 participants