Skip to content

Using by-value self with an immediate, implicity-copyable struct should pass a copy as the self param. #10615

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
luqmana opened this issue Nov 22, 2013 · 8 comments
Labels
A-codegen Area: Code generation
Milestone

Comments

@luqmana
Copy link
Member

luqmana commented Nov 22, 2013

This is from a code sample sent to the mailing list: https://mail.mozilla.org/pipermail/rust-dev/2013-November/006924.html

struct Value {
    n: int
}

impl Value {
    fn squared(mut self) -> Value {
        self.n *= self.n;
        self
    }
}

fn main() {
    let x = Value { n: 3 };
    let y = x.squared();
    println!("{} {}", x.n, y.n);
}

Prints out:

-> % rustc foo.rs && ./foo
9 9

The call to squared should be receiving a copy of x as the self param. This bug is only exhibited in the case of implicity copyable struct. If you add something like ~int to Value the compiler will rightly complain that x has been moved.

Also, if you add another field (like m: int) making Value an immediate no longer, we see the right behaviour:

-> % rustc foo.rs && ./foo
3 9
@huonw
Copy link
Member

huonw commented Nov 22, 2013

Presumably this would be fixed if we handled self as a normal argument, rather than passing it in the environment pointer (which means "by-val" self is particularly strange)?

@alexcrichton
Copy link
Member

This seems very bad, nominating.

@thestinger
Copy link
Contributor

@huonw: Yes, lets please fix this issue once and for all rather than working around the symptoms. We shouldn't be passing self as a environment pointer - it's just wrong.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 9, 2014

Accepted for 1.0, P-backcompat-lang

@adrientetar
Copy link
Contributor

This should be closed now. (@luqmana)

@eddyb
Copy link
Member

eddyb commented Jan 11, 2014

I put "Fixes #7411, #10615." in the commit message, but for some reason only the first one was closed.

@adrientetar
Copy link
Contributor

Well, the GitHub parser doesn't take stacked declarations:
https://help.github.com/articles/closing-issues-via-commit-messages

@alexcrichton
Copy link
Member

Thanks @eddyb!

flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 23, 2023
[arithmetic_side_effects] Fix rust-lang#10590

Fix rust-lang#10590

changelog: [`arithmetic_side_effects`]: Detect integer methods that can introduce side effects
flip1995 pushed a commit to flip1995/rust that referenced this issue May 20, 2023
Rename `integer_arithmetic`

The lack of official feedback in rust-lang#10200 made me give up on pursuing the matter but after yet another use-case that is not handled by `integer_arithmetic` (rust-lang#10615), I think it is worth trying again.

---

changelog: Move/Deprecation: Rename `integer_arithmetic` to `arithmetic_side_effects`
[rust-lang#10674](rust-lang/rust-clippy#10674)
<!-- changelog_checked -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation
Projects
None yet
Development

No branches or pull requests

7 participants