Skip to content

An variable assigned -1 can be inferred as uint #9037

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

Closed
novalis opened this issue Sep 7, 2013 · 7 comments
Closed

An variable assigned -1 can be inferred as uint #9037

novalis opened this issue Sep 7, 2013 · 7 comments

Comments

@novalis
Copy link
Contributor

novalis commented Sep 7, 2013

fn bar () -> uint {
    return 5;
}

fn foo (use_bar: bool) -> ~str {
    let mut a = -1;
    if use_bar {
        a = bar();
    }
    if a >= 0 {
        ~"a"
    } else {
        ~"b"
    }
}

fn main() {
    printfln!("%s", foo(false));
    printfln!("%s", foo(true));
}
bad-infer.rs:10:7: 10:13 warning: comparison is useless due to type limits [-W type-limits (default)]
bad-infer.rs:10     if a >= 0 {
@novalis novalis mentioned this issue Sep 7, 2013
@huonw
Copy link
Member

huonw commented Sep 7, 2013

Isn't this inference correct? One needs to explicitly cast the result of bar to int if one wants it to be an int, i.e.

fn foo() -> uint { 1 }
fn main() {
    let x: int = foo(); // should be `foo() as int`
}
9037.rs:3:17: 3:23 error: mismatched types: expected `int` but found `uint` (expected int but found uint)
9037.rs:3     let x: int = foo();
                           ^~~~~~
error: aborting due to previous error

I guess it's a little unintuitive that -1 can be a uint; although it makes sense mathematically: it's the number the you can add to 1 which gives 0 (e.g., for a 64-bit platform this is 18446744073709551615, since this is 2^64 - 1).

@thestinger
Copy link
Contributor

This is correct, since overflow is perfectly well defined. It makes sense to have a lint warning but not to change the type system.

@novalis
Copy link
Contributor Author

novalis commented Sep 7, 2013

I think this actually is pretty surprising behavior. Sure, I understand the math and all that. But I bet 99% of uses of literal negative numbers are intended to be ints as opposed to uints. If, in my example code, I hadn't had the if statement, I wouldn't have noticed this until it bit me. And one of the reviewers of my actual code where this actually came up didn't notice it either (@alexcrichton). Alex asked me to file this bug report, actually. So I think the literal negatives should be treated as signed, and users who want unsigned should have to do -1u. That's optimizing for the common case / the principle of least surprise.

@thestinger
Copy link
Contributor

There's a bug open about making a lint check for it already, it doesn't make sense to remove a valid use case for wrapping from the language by making it more complex.

@alexcrichton
Copy link
Member

Yeah I was just unsure on what the intended behavior was, sounds like it's working as intended, though, and I don't have a strong opinion one way or the other.

@Skrylar
Copy link

Skrylar commented Jan 8, 2014

@alexcrichton Well, I have one at least.

Negative values are outside the bounds of the type and cannot be stored. Note that assigning 256 to a u8 is not allowed (you get an out of bounds error, why not wrap back to 0?) but assigning -1 to a u8 is allowed. This behavior is inconsistent (it wraps in one direction but not the other.)

I already had one issue where I passed a negative value to a function I wrote which expects an unsigned value, and Rust silently accepted it. However, in other code it refused to compile because the literals were too large. For consistency, both need to be accepted or both need to be rejected; half-assing it can give someone the impression the language won't allow such voodoo without warning, and then it quietly allows it somewhere else.

@nikomatsakis
Copy link
Contributor

I'm surprised the existing type-overflow lint doesn't catch this case.

flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2022
…, r=dswij

Fix `extra_unused_lifetimes` false positive

This PR fixes rust-lang#9014.

I confirmed the FP on the `crates.io` source as `@JohnTitor` mentioned, and confirmed that the FP is no longer present following this change.

I did not include a test in this PR because I think constructing one would be complicated, and the fix is pretty simple. But please let me know if this is unacceptable.

changelog: fix `extra_unused_lifetimes` FP
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2022
This commit adds test for a `extra_unused_lifetimes` false positive from derive (rust-lang#9014).

The fix for the FP is introduced in rust-lang#9037.
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2022
`extra_unused_lifetimes` add FP test case emitting from derived attributes.

Add test to cover for rust-lang#9014 which is fixed in rust-lang#9037.

changelog: [`extra_unused_lifetimes`] Add FP test case emitting from derived attributes.

---

Seeing the FP from the test:

```sh
$ git revert -m 1 1d1ae10
$ TESTNAME=extra_unused_lifetime cargo uitest
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants