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

dropck seems overconservative when dealing with matches at the end of a block #22252

Closed
sfackler opened this Issue Feb 13, 2015 · 12 comments

Comments

Projects
None yet
6 participants
@sfackler
Copy link
Member

sfackler commented Feb 13, 2015

#![feature(unsafe_destructor)]
struct A;

impl Drop for A {
    fn drop(&mut self) {}
}

struct B<'a> {
    a: &'a A,
}

#[unsafe_destructor]
impl<'a> Drop for B<'a> {
    fn drop(&mut self) {}
}

impl A {
    fn b(&self) -> B {
        B { a: self }
    }
}

fn main() {
    let a = A;
    match a.b() {
        _ => {}
    }
}
test.rs:25:11: 25:12 error: `a` does not live long enough
test.rs:25     match a.b() {
                     ^
test.rs:23:11: 28:2 note: reference must be valid for the destruction scope surrounding block at 23:10...
test.rs:23 fn main() {
test.rs:24     let a = A;
test.rs:25     match a.b() {
test.rs:26         _ => {}
test.rs:27     }
test.rs:28 }
test.rs:24:14: 28:2 note: ...but borrowed value is only valid for the block suffix following statement 0 at 24:13
test.rs:24     let a = A;
test.rs:25     match a.b() {
test.rs:26         _ => {}
test.rs:27     }
test.rs:28 }
error: aborting due to previous error

Adding anything after the match (like ; or ()) causes the error to go away.

cc @pnkfelix

@sfackler

This comment has been minimized.

Copy link
Member Author

sfackler commented Feb 13, 2015

This may be a dup of #21114

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Feb 14, 2015

I think this might be a consequence of our rules for r-value temporaries, where the temporaries in the expression at the end of a block are assigned a lifetime longer than that of the let-bindings within the block itself.

But then again, that answer is not very satisfying without an example of what kind of unsoundness this is supposedly preventing. (I do not have an example ready on hand, but you've given me a place to start.)

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Feb 14, 2015

cc #22321

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Feb 14, 2015

also, cc #12032 ;)

@theemathas

This comment has been minimized.

Copy link

theemathas commented Feb 23, 2015

The problem does not require a match

Simpler reproduction code:

#![feature(unsafe_destructor)]

struct Foo;
struct FooRef<'a>(&'a Foo);

impl<'a> FooRef<'a> {
    fn borrow(&self) {}
}

#[unsafe_destructor]
impl<'a> Drop for FooRef<'a> {
    fn drop(&mut self) {}
}

fn main() {
    let x = Foo;
    FooRef(&x).borrow()
}

playpen

The error:

<anon>:17:13: 17:14 error: `x` does not live long enough
<anon>:17     FooRef(&x).borrow()
                      ^
<anon>:15:11: 18:2 note: reference must be valid for the destruction scope surrounding block at 15:10...
<anon>:15 fn main() {
<anon>:16     let x = Foo;
<anon>:17     FooRef(&x).borrow()
<anon>:18 }
<anon>:16:16: 18:2 note: ...but borrowed value is only valid for the block suffix following statement 0 at 16:15
<anon>:16     let x = Foo;
<anon>:17     FooRef(&x).borrow()
<anon>:18 }

worr added a commit to worr/yup-oauth2 that referenced this issue May 11, 2015

Provide workaround for lifetime bug in rustlang
Due to rust-lang/rust#22252, r-value temporaries
outlive the lifetimes of variables bound with let statements in a function
body. Because of this, device.rs fails to compile against newer rustc
nightlies.

This implements a simple workaround that binds the result of the match in
`request_code` to a local variable before returning it. This allows it to have
the same lifetime as `req` in one of the previous let bindings.

worr added a commit to worr/yup-oauth2 that referenced this issue May 11, 2015

fix(rustup): workaround rustlang bug
Due to rust-lang/rust#22252, r-value temporaries
outlive the lifetimes of variables bound with let statements in a function
body. Because of this, device.rs fails to compile against newer rustc
nightlies.

This implements a simple workaround that binds the result of the match in
`request_code` to a local variable before returning it. This allows it to have
the same lifetime as `req` in one of the previous let bindings.

worr added a commit to worr/yup-oauth2 that referenced this issue May 11, 2015

fix(rustup): workaround rustlang bug
Due to rust-lang/rust#22252, r-value
temporaries outlive the lifetimes of variables bound with let
statements in a function body. Because of this, device.rs fails to
compile against newer rustc nightlies.

This implements a simple workaround that binds the result of the match
in `request_code` to a local variable before returning it. This allows
it to have the same lifetime as `req` in one of the previous let
bindings.
@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented May 24, 2016

Triage: no change, at least for the example provided by @theemathas

@arielb1

This comment has been minimized.

Copy link
Contributor

arielb1 commented May 24, 2016

This looks like a duplicate of #33490 - on MIR it would be a segfault, on trans it (incorrectly) isn't.

@arielb1 arielb1 closed this May 24, 2016

@sfackler

This comment has been minimized.

Copy link
Member Author

sfackler commented May 24, 2016

@arielb1 This is a bug about a compilation error, not a runtime failure, so I don't think this is a duplicate of that issue (which was also filed a year and a half after this one).

@nikomatsakis nikomatsakis referenced this issue Jun 6, 2016

Closed

Error Message Overhaul #33240

1 of 10 tasks complete
@arielb1

This comment has been minimized.

Copy link
Contributor

arielb1 commented Jun 11, 2016

@sfackler

The issues have the same root cause - borrowck/MIR have a notion of destruction order that makes this code obviously wrong. Trans uses a different destruction order, but that is problematic.

@mbrubeck

This comment has been minimized.

Copy link
Contributor

mbrubeck commented Sep 14, 2016

#33490 is closed but this issue still exists in nightly. Without the unsafe_destructor flags, the test case above still generates the same compiler error. The similar cases from #31439 and #21114 (comment) also still fail to compile in nightly. Should this be re-opened?

@sfackler

This comment has been minimized.

Copy link
Member Author

sfackler commented Sep 14, 2016

#33490 seems open to me?

@mbrubeck

This comment has been minimized.

Copy link
Contributor

mbrubeck commented Sep 14, 2016

Oops, I meant that comments like #33490 (comment) claimed that MIR now behaves correctly and that only old trans was still broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.