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

tweak temporary lifetime rules for if and while conditions #12033

Closed
nikomatsakis opened this issue Feb 4, 2014 · 7 comments · Fixed by #12134
Closed

tweak temporary lifetime rules for if and while conditions #12033

nikomatsakis opened this issue Feb 4, 2014 · 7 comments · Fixed by #12134
Labels
A-lifetimes Area: lifetime related E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Milestone

Comments

@nikomatsakis
Copy link
Contributor

I think that if and while conditions should free any temporaries that occur within them (for while conditions it's mandatory, actually, since the condition is evaluated many times).

That is, if I write:

if my_set().borrow().contains(&key) { ... }

the temporary returned by borrow() should be freed once the condition is evaluated, before entering the body of the if.

The same holds for whiles.

What makes these expressions different from other compound forms is that the conditions are evaluated to scalar booleans, and hence can never create lasting aliases that might need to be preserved.

In the case of a while, this is needed because temporaries can never outlive conditional or repeated scopes, and the condition of a while is a repeated scope.

@flaper87
Copy link
Contributor

flaper87 commented Feb 4, 2014

cc @flaper87

@nikomatsakis
Copy link
Contributor Author

Marking as mentor with myself. This is a fairly small change (if you know what to change :)

@ghost ghost assigned flaper87 Feb 5, 2014
@flaper87
Copy link
Contributor

flaper87 commented Feb 5, 2014

I'll work on it :)

@pnkfelix
Copy link
Member

pnkfelix commented Feb 6, 2014

1.0 blocker, P-backcompat-lang.

@pnkfelix pnkfelix added this to the 1.0 milestone Feb 6, 2014
bors added a commit that referenced this issue Feb 10, 2014
…akis

Closes #12033

IR Before:

```llvm
normal-return:                                    ; preds = %while_body
  %113 = load i64* %i
  %114 = sub i64 %113, 1
  store i64 %114, i64* %i
  br label %while_cond
```

IR After:

```llvm
normal-return:                                    ; preds = %while_cond
  store i8 %11, i8* %0
  %18 = load i8* %0, !range !0
  call void @_ZN9Temporary9glue_drop19he4ee51d3c03b9cf4ajE(%struct.Temporary* %10)
  %19 = bitcast %struct.Temporary* %10 to i8*
  call void @_ZN2rt11global_heap14exchange_free_19h4fabdf24a2250163aj4v0.0E(i8* %19)
  %20 = icmp ne i8 %18, 0
  br i1 %20, label %while_body, label %while_exit
```
@eddyb eddyb reopened this Jul 25, 2016
@eddyb
Copy link
Member

eddyb commented Jul 25, 2016

The fix in #12134 was partial, if with an else is still broken:

use std::cell::RefCell;

fn main() {
    let x = RefCell::new(0);
    if *x.borrow() == 0 {} else {} // error: `x` does not live long enough
}

@nikomatsakis nikomatsakis added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 25, 2016
@nikomatsakis
Copy link
Contributor Author

Nominated. Seems clear to me that this is a bug we should fix -- the main question is whether to be concerned about backwards compatibility, and -- if so -- what to do about it.

I think this is mostly a question for @rust-lang/compiler team to decide, but I tagged this issue with T-Lang too in case @rust-lang/lang members want to weight in on whether the behavior is indeed a bug. But it seems clear that having if and if/else behave differently is bogus.

@nikomatsakis nikomatsakis added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 25, 2016
@nikomatsakis
Copy link
Contributor Author

Consensus in the compiler team meeting was that we should just fix this and add the resulting PR to the release notes.

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Jul 28, 2016
bors added a commit that referenced this issue 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 added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
…iny-me

fix: doc url link type

fix: rust-lang#12033

I did some debugging and found the cause looks like to be some doc links' `LinkType` are kept as `Shortcut` which don't make sense for url links.
This PR should resolve both problems in the origin issue, but aside this PR, more work are needed for doc_links.

about `LinkType`: https://github.com/raphlinus/pulldown-cmark/blob/f29bd1e228913690e5092c9594e4e607423ff0aa/src/lib.rs#L191-L210
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: lifetime related E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants