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

str_to_float converts fractional values in reverse #1015

Closed
jdm opened this issue Oct 8, 2011 · 4 comments
Closed

str_to_float converts fractional values in reverse #1015

jdm opened this issue Oct 8, 2011 · 4 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@jdm
Copy link
Contributor

jdm commented Oct 8, 2011

The routine uses vec::pop_char instead of vec::shift_char.

@Yoric
Copy link
Contributor

Yoric commented Oct 11, 2011

Ok, I'll pick this issue as my first foray into rust.

@Yoric
Copy link
Contributor

Yoric commented Oct 11, 2011

Actually, it's a bit worse: str_to_float "1.5" returns 1.

@jdm
Copy link
Contributor Author

jdm commented Oct 11, 2011

Oh right, I remember coming across that before.

    let i = 1u;
    while (i < len) {
        total += dec_val(str::shift_char(right)) as float /
                 (int::pow(10, i) as float);
        i += 1u;
    }

Either start the loop at 0u and use i+1u in pow, or just make the loop

i <= len
. I think that should resolve it.

@Yoric Yoric mentioned this issue Oct 12, 2011
@Yoric
Copy link
Contributor

Yoric commented Oct 12, 2011

Well, eventually, I rewrote the whole function. This int::pow was also a problem as each call is linear.

@jdm jdm closed this as completed Oct 12, 2011
xFrednet pushed a commit to xFrednet/rust that referenced this issue May 21, 2022
… r=llogiq

Optionally allow `expect` and `unwrap` in tests

This addresses rust-lang#1015, except it makes the new behavior optional.

The reason for the msrv-related changes is as follows.

Rather than expand `check_methods` list of arguments, it seemed easier to make `check_methods` a method of `Methods`, so that `check_methods` could access `Methods`' fields.

`check_methods` had an `msrv` parameter, which I consequently made a field of `Methods`. But, to avoid adding a lifetime parameter to `Methods`, I made the field type `Option<RustcVersion>` instead of the parameter's existing type, `Option<&RustcVersion>`. This seemed sensible since `RustcVersion` implements `Copy`. But this broke a lot of code that expected an `Option<&RustcVersion>` or `&Option<RustcVersion>`. I changed all of those occurrences to `Option<RustcVersion>`. IMHO, the code is better as a result of these changes, though.

The msrv-related changes are in their own commit to (hopefully) ease review.

Closes rust-lang#1015

changelog: optionally allow `expect` and `unwrap` in tests

r? `@llogiq`
coastalwhite pushed a commit to coastalwhite/rust that referenced this issue Aug 5, 2023
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

2 participants