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

Warn on accidental copying of arrays #1474

Closed
aochagavia opened this issue Jan 26, 2017 · 4 comments
Closed

Warn on accidental copying of arrays #1474

aochagavia opened this issue Jan 26, 2017 · 4 comments
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types

Comments

@aochagavia
Copy link
Contributor

aochagavia commented Jan 26, 2017

The code below compiles correctly, but makes no sense at all. I think this violates the principle of least surprise (we had a nice discussion about it on #rust). See if you can spot the error ;)

extern crate scoped_threadpool;
fn main() {
    let mut array = [0; 3];
    let mut pool = scoped_threadpool::Pool::new(2);
    pool.scoped(|scope| {
        scope.execute(move||{
            array[0] = 1;
        });
        scope.execute(move||{
            array[0] = 2;
        });
    });
    // Prints 0, since the array is copied into each thread
    println!("{:?}",array[0]);
}

Would it be possible to add a lint for this? Just warning that a captured array of Clone elements will be copied and not moved.

Credits go to @arianvp for coming up with the code (and needing 5 days to discover why it was wrong)

@mcarton
Copy link
Member

mcarton commented Jan 26, 2017

we had a nice discussion about it on #rust

Do you have a link? Time of discussion? 😄

@mcarton mcarton added L-correctness Lint: Belongs in the correctness lint group E-medium Call for participation: Medium difficulty level problem and requires some initial experience. A-lint Area: New lints T-middle Type: Probably requires verifiying types labels Jan 26, 2017
@aochagavia
Copy link
Contributor Author

According to the botbot logs it was on 9:10 am today (January 26th 2017)

@camsteffen
Copy link
Contributor

Just looking at the closures independently, you could lint that the (copy of the) array is modified and then dropped without ever observing the array after it was modified.

Otherwise, I don't think it is realistic to lint "accidental copy" because there is no reliable way of knowing if the copying is "accidental".

@camsteffen
Copy link
Contributor

Just looking at the closures independently, you could lint that the (copy of the) array is modified and then dropped without ever observing the array after it was modified.

This is rust-lang/rust#65467

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

3 participants