".".parse() returns Ok(0.0)

[hanna-kruppe]

This program prints Ok(0) where I would expect an Err(..):

fn main() {
    println!("{:?}", ".".parse::<f64>());
}

If I remember correctly, this behavior has been around since pre-1.0 but it would be good if someone who has multirust available could verify that (at least for 1.0 final if earlier builds are harder to get).

Unfortunately even if we all agree that this behavior is wrong, it may not be worth the potential breakage to fix. For example, recent changes to the integer FromStr format (allowing leading plus) did, surprisingly, break real code. It's quite possible that fixing this wart is not worth the risk.

[bluss]

Travis testing says it's present in all versions since 1.0.0 (builds) (code)

[Stebalien]

I'd rather not go down the PHP route and document bugs as features, even minor ones like this. Can we get a crater run? Can't be cratered. Crap.

[alexcrichton]

triage: I-nominated

[alexcrichton]

The libs team discussed this during triage yesterday and the conclusion is that this is pretty reasonable to include in the future as a bugfix, so P-medium for now.

[hanna-kruppe]

This is an ideal E-easy bug, and although it's quite small I can mentor for this if desired. So, someone please tag this E-easy E-mentor.

Details for someone looking to fix this bug:
You'll have to find the correct code path in the parse_decimal function in src/libcore/num/dec2flt/parse.rs and change it to return an error. There is a whole if dedicated to this case, but perhaps there is a more elegant way to rewrite the surrounding code. There's also a test in libcoretest that expect the current (wrong) behavior, it's probably best to change this test to expect an error instead.

[yati-sagade]

E-easy -- can I take this one?

[sfackler]

Go for it!

[hanna-kruppe]

@yati-sagade

Absolutely! If you have questions or run into problems, you can ping me on IRC (my IRC nick is the same as my username here) or if I'm not online on IRC , comment here.

[yati-sagade]

@rkruppe Thanks :) I'm playing with the code, and this indeed looks easy. However, the build time is insane for such a small change (maybe because libcore compilation forces almost all other modules to compile?). I'm just doing make -j5 -- is there a faster way or I'm stuck with this? :)

Also, do we want to spit a warning at this point or just return Invalid?

[hanna-kruppe]

@yati-sagade

There's no way to make build times good, but you can make it less insane by rebuilding less. make -jN check-stage1-coretest will only build the first stage of the bootstrapping process and then enough to run the libcore tests. Since this misses all tests outside of libcore, and AFAIK some test in libstd use parse, you might still want to run make -jN check-stage1-std or even make -jN check if you have the patience. There is also a way to build a full stage1 compiler (to make one-off experiments that need not or should not go into tests), but I keep forgetting what it's called. If you want/need that, hop into #rust-internals and ask how to do it.

As for warning: It would be nice to have a grace period, but I don't see a good way to do it. Since this is a runtime condition, the warning would be shown to the users, not to the developers. Also, how would we even report the warning? Just printing to a running process's stderr is extremely rude — and also not really possibly in libcore. @alexcrichton what do you think?

[ghost]

Sorry that I just jumped in and did this @yati-sagade, I didn't realise someone else was doing it until I was opening the PR.

[yati-sagade]

@Toby-s No problem. I didn't realize that returning Err was what we were going to do (per above discussion).