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

Fix lifetime rules for 'if' conditions #36029

Merged
merged 1 commit into from
Aug 28, 2016
Merged

Conversation

KiChjang
Copy link
Member

@KiChjang KiChjang commented Aug 27, 2016

Fixes #12033.

Changes the temporary scope rules to make the condition of an if-then-else a terminating scope. This is a [breaking-change].

@rust-highfive
Copy link
Collaborator

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test an example where we fail to infer the type parameter H. This
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to belong here.

@nagisa
Copy link
Member

nagisa commented Aug 27, 2016

The issue title refers to fixing both if and while but there seem to be no changes or tests pertaining to the while expression. Is it intentional?

EDIT: reading the issue, its clear that only thing this PR ought to be fixing is if. Would be nice to have PR and commit titles updated correspondingly.

@KiChjang KiChjang changed the title Fix lifetime rules for 'if' and 'while' conditions Fix lifetime rules for 'if' conditions Aug 27, 2016
@KiChjang
Copy link
Member Author

Whoops, I made a couple of stupids, now it should look fine. Not sure if it behaves fine though.

@arielb1
Copy link
Contributor

arielb1 commented Aug 28, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Aug 28, 2016

📌 Commit 4853456 has been approved by arielb1

@arielb1 arielb1 added the relnotes Marks issues that should be documented in the release notes of the next release. label Aug 28, 2016
@bors
Copy link
Contributor

bors commented Aug 28, 2016

⌛ Testing commit 4853456 with merge 312734c...

bors added a commit that referenced this pull request Aug 28, 2016
Fix lifetime rules for 'if' conditions

Fixes #12033.

Changes the temporary scope rules to make the condition of an if-then-else a terminating scope. This is a [breaking-change].
@bors bors merged commit 4853456 into rust-lang:master Aug 28, 2016
@eddyb
Copy link
Member

eddyb commented Aug 31, 2016

To explain how this can be a breaking-change, we can look at the RefCell-based examples: with if-else, temporaries in the conditions can live longer than variables above the if-else, whereas with just if, the "terminating" aspect means that temporaries live shorter than the variables they borrow from.

The opposite can be produced by having a local variable borrow the temporary instead:

struct Foo;

impl Foo {
    fn save<'a>(&'a self, dest: &mut &'a Foo) -> bool {
        *dest = self;
        true
    }
}

fn main() {
    let mut dest = &Foo;
    if Foo.save(&mut dest) {} else {}
}

This example compiles before this PR, but not if we remove else {}. With this PR, the if-else case behaves like the if case, so neither will compile, the temporary being to short to be borrowed by dest.

EDIT: The other breaking change here is destructor ordering, which affects all the code using if-else in the tail of a block, even if it continues to compile.
Temporaries in the condition expression of if and if-else will now always be dropped before branching based on the condition, and thus before all of the variables in the enclosing block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants