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

Surprising, possibly buggy behavior on &mut patterns in for loops #12250

Closed
bstrie opened this issue Feb 13, 2014 · 8 comments
Closed

Surprising, possibly buggy behavior on &mut patterns in for loops #12250

bstrie opened this issue Feb 13, 2014 · 8 comments

Comments

@bstrie
Copy link
Contributor

bstrie commented Feb 13, 2014

Here's the surprising program:

fn main() {
    let mut pixels = [0f32, ..8];

    for &mut pixel in pixels.mut_iter() {
        pixel = 2.0;
    }

    println!("{:?}", pixels);
}

Compiling produces the following output, which is a head-scratcher if you don't know what's going on:

um.rs:4:10: 4:19 warning: variable `pixel` is assigned to, but never used, #[warn(unused_variable)] on by default
um.rs:4     for &mut pixel in pixels.mut_iter() {
                 ^~~~~~~~~
um.rs:5:9: 5:14 warning: value assigned to `pixel` is never read, #[warn(dead_assignment)] on by default
um.rs:5         pixel = 2.0;
                ^~~~~

Running it prints the following:

[0f32, 0f32, 0f32, 0f32, 0f32, 0f32, 0f32, 0f32]

So even though the error messages are trying to warn you that you're not actually mutating the array, it doesn't answer the question as to why you're not mutating the array.

Intuitively, I expected this program to be equivalent to this:

fn main() {
    let mut pixels = [0f32, ..8];

    for pixel in pixels.mut_iter() {
        *pixel = 2.0;
    }

    println!("{:?}", pixels);
}

Which mutates the array as expected.

There's clearly some sort of implicit copying going on here that's very surprising, even if correct. @alexcrichton was also concerned that this might erroneously parsing &mut pixel as &(mut pixel).

So the question is, what behavior do we expect here?

@alexcrichton
Copy link
Member

Nominating, this could break existing code if we decide to change it.

@huonw
Copy link
Member

huonw commented Feb 13, 2014

The pattern for &mut pointers is &x, so &mut x is being parsed as &(mut x), however, even if &mut it did work as a pattern, it's the same as writing:

let foo = &mut ...;

let mut x = *foo;
x = 1.0;

i.e. it's not assigning to the reference, just assigning to the new variable with a value that's been copied out of foo.

@Aatch
Copy link
Contributor

Aatch commented Feb 13, 2014

I'm not sure what we could change here. Sure, the behaviour is slightly surprising, but it's just a consequence of how patterns and binding work.

I'm not convinced that this is anything other than an unexpected consequence and I don't think it's worth making any significant changes. (Since, in reality, the issue is actually the semantics of *foo as an rvalue vs. as an lvalue). The language is going to have a few cases like these and we can't remove every corner case from the syntax.

The only change that makes sense, to me, is making &mut foo equivalent to &foo, not that this would make the above code work, but it would throw an error instead (since pixel would be immutable).

@eddyb
Copy link
Member

eddyb commented Feb 15, 2014

Or make &mut foo an outright error, requiring parens, as in &(mut foo).

@huonw
Copy link
Member

huonw commented Feb 15, 2014

cc #11144 for parens.

@liigo
Copy link
Contributor

liigo commented Feb 16, 2014

How about mut& x, which read as "mut ref x"?
2014年2月16日 上午1:40于 "Eduard Burtescu" notifications@github.com写道:

Or make &mut foo an outright error, requiring parens, as in &(mut foo).


Reply to this email directly or view it on GitHubhttps://github.com//issues/12250#issuecomment-35162165
.

@nikomatsakis
Copy link
Contributor

cc me (@huonw's summary is precisely right -- and I think there is
nothing to be done beyond a possible lint without extra parens)

@pnkfelix
Copy link
Member

Closing, not a bug, just a slight footgun.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022

Verified

This commit was signed with the committer’s verified signature.
ChrisDenton Chris Denton
… r=Veykril

fix: special case base url of `BuiltinType` to core

fix rust-lang#12250
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 27, 2024

Verified

This commit was signed with the committer’s verified signature.
ChrisDenton Chris Denton
Add lint `manual_inspect`

fixes rust-lang#12250

A great example of a lint that sounds super simple, but has a pile of edge cases.

----

changelog: Add lint `manual_inspect`
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

8 participants