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

Compound assignment (+=) on uninitialized variables is allowed #12527

Closed
ben0x539 opened this issue Feb 24, 2014 · 13 comments · Fixed by #12733
Closed

Compound assignment (+=) on uninitialized variables is allowed #12527

ben0x539 opened this issue Feb 24, 2014 · 13 comments · Fixed by #12733

Comments

@ben0x539
Copy link
Contributor

This compiles:

fn main() {
    let i: int;
    i += 2;
}

Now, I can't seem to get that anything reads the value again to compile, but it seems like bad times anyway.

Thought to try this because of #12452.

@flaper87
Copy link
Contributor

cc @flaper87

@huonw huonw added the I-wrong label Feb 24, 2014
@nikomatsakis
Copy link
Contributor

cc me

@edwardw
Copy link
Contributor

edwardw commented Mar 6, 2014

If this were to be fixed as use of possibly uninitialized variable, it will void the following two tests from compile-fail/liveness-unused.rs:

fn f3() {
    let mut x = 3;
    //~^ ERROR variable `x` is assigned to, but never used
    x += 4;
    //~^ ERROR value assigned to `x` is never read
}

fn f3b() {
    let mut z = 3;
    //~^ ERROR variable `z` is assigned to, but never used
    loop {
        z += 4;
    }
}

But it makes sense, doesn't it?

@nikomatsakis
Copy link
Contributor

@edwardw actually, that analysis (that gives those warnings) is completely separate, and I remember it drew some subtle distinctions regarding assignment vs use etc

@edwardw
Copy link
Contributor

edwardw commented Mar 6, 2014

Very subtle indeed. I think I find the right compromise.

@nikomatsakis
Copy link
Contributor

No, this is the wrong approach. liveness.rs shouldn't be responsible for reporting errors anymore. I thought I had removed the span_err but I guess I was wrong about that! Anyway, I believe there should be changes to borrowck.

@nikomatsakis
Copy link
Contributor

Well, let me review the code, but I think liveness should just have the job of warnings (in general I'd like to be able to rm liveness.rs without affecting correctness of the compiler).

@edwardw
Copy link
Contributor

edwardw commented Mar 6, 2014

Whoops, the patch has been half way through bors.

@nikomatsakis
Copy link
Contributor

@edwardw just to be clear, the patch you made is pretty reasonable, it's just that I've been moving responsibility for these sorts of checks to the borrow checker, which is probably not clear from the code. I think I didn't do a good job of finishing the job of simplifying liveness.

Anyway the problem there is that the borrow checker classifies all expressions into assignees or uses, but this is both, so we need to change the move data to have a 3-way classification.

It'd also be nice to remove all span_err's from liveness and make sure that my assertion that liveness is purely about warnings is actually true. Maybe there are other oversights in the borrow checker!

@edwardw
Copy link
Contributor

edwardw commented Mar 7, 2014

I see. Let's see what I can do about that.

@edwardw
Copy link
Contributor

edwardw commented Mar 8, 2014

r?

@nikomatsakis
Copy link
Contributor

Yeah, this looks good. I made a few minor suggestions.

@edwardw
Copy link
Contributor

edwardw commented Mar 8, 2014

All comments addressed. r?

bors added a commit that referenced this issue Mar 9, 2014
- Repurposes `MoveData.assignee_ids` to mean only `=` but not `+=`, so
  that borrowck effectively classifies all expressions into assignees,
  uses or both.
- Removes two `span_err` in liveness analysis, which are now borrowck's
  responsibilities.
    
Closes #12527.
bors added a commit that referenced this issue Mar 10, 2014
- Repurposes `MoveData.assignee_ids` to mean only `=` but not `+=`, so
  that borrowck effectively classifies all expressions into assignees,
  uses or both.
- Removes two `span_err` in liveness analysis, which are now borrowck's
  responsibilities.
    
Closes #12527.
@bors bors closed this as completed in abfde39 Mar 10, 2014
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this issue Mar 21, 2024
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

Successfully merging a pull request may close this issue.

5 participants