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
Closed
Labels
A-destructors Area: destructors (Drop, ..)

Comments

@sfackler
Copy link
Member

#![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 sfackler added the A-destructors Area: destructors (Drop, ..) label Feb 13, 2015
@sfackler
Copy link
Member Author

This may be a dup of #21114

@pnkfelix
Copy link
Member

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
Copy link
Member

cc #22321

@pnkfelix
Copy link
Member

also, cc #12032 ;)

@theemathas
Copy link
Contributor

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
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
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
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
Copy link
Member

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

@arielb1
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 as completed May 24, 2016
@sfackler
Copy link
Member Author

@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).

@arielb1
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
Copy link
Contributor

#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
Copy link
Member Author

#33490 seems open to me?

@mbrubeck
Copy link
Contributor

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
Labels
A-destructors Area: destructors (Drop, ..)
Projects
None yet
Development

No branches or pull requests

6 participants