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 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

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Aug 27, 2016

r? @arielb1

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

@KiChjang KiChjang force-pushed the KiChjang:issue-12033 branch from 1cb52b2 to e67bc3b Aug 27, 2016
// 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

This comment has been minimized.

Copy link
@rkruppe

rkruppe Aug 27, 2016

Member

This doesn't seem to belong here.

@nagisa

This comment has been minimized.

Copy link
Contributor

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 force-pushed the KiChjang:issue-12033 branch from e67bc3b to 4853456 Aug 27, 2016
@KiChjang KiChjang changed the title Fix lifetime rules for 'if' and 'while' conditions Fix lifetime rules for 'if' conditions Aug 27, 2016
@KiChjang

This comment has been minimized.

Copy link
Member Author

KiChjang commented Aug 27, 2016

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

@arielb1

This comment has been minimized.

Copy link
Contributor

arielb1 commented Aug 28, 2016

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Aug 28, 2016

📌 Commit 4853456 has been approved by arielb1

@bors

This comment has been minimized.

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
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@eddyb

This comment has been minimized.

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.

@KiChjang KiChjang mentioned this pull request Aug 31, 2016
0 of 1 task complete
@KiChjang KiChjang deleted the KiChjang:issue-12033 branch Dec 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.