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

Temporary does not live long enough in let chains #103476

Closed
Alexendoo opened this issue Oct 24, 2022 · 13 comments · Fixed by #133093
Closed

Temporary does not live long enough in let chains #103476

Alexendoo opened this issue Oct 24, 2022 · 13 comments · Fixed by #133093
Labels
C-bug Category: This is a bug. F-let_chains `#![feature(let_chains)]`

Comments

@Alexendoo
Copy link
Member

I tried this code, playground:

#![feature(let_chains)]

use syn::{Attribute, Meta};

fn f(attr: &Attribute) -> bool {
    if let Meta::NameValue(name_value) = attr.parse_meta().ok().unwrap()
        && let path_idents = name_value.path.segments.iter()
        /* other stuff */
    {
        
    }
    
    true
}

I expected to see this happen: code compiles

Instead, this happened: errors due to name_value.path.segments does not live long enough

It compiles when expressed as nested ifs, playground:

#![feature(let_chains)]

use syn::{Attribute, Meta};

fn f(attr: &Attribute) -> bool {
    if let Meta::NameValue(name_value) = attr.parse_meta().ok().unwrap() {
        if let path_idents = name_value.path.segments.iter() {}
    }

    true
}

The code compiled previously, but no longer does after #103034 (confirmed with a cargo bisect-rustc)

cc @nathanwhit

@Alexendoo Alexendoo added C-bug Category: This is a bug. F-let_chains `#![feature(let_chains)]` labels Oct 24, 2022
@est31
Copy link
Member

est31 commented Oct 30, 2022

Minimized:

#![feature(let_chains)]

struct Pd<T>(std::marker::PhantomData<T>);

impl<T> Pd<T> {
    fn iter(&self) -> Iter<T> {
        todo!()
    }
}

pub struct Iter<'a, T: 'a> {
    inner: Box<dyn IterTrait<'a, T> + 'a>,
}

trait IterTrait<'a, T: 'a> {
    fn clone_box(&self) -> Box<dyn IterTrait<'a, T> + 'a>;
}

fn f(m: Option<Pd<()>>) -> bool {
    if let Some(n) = m
        && let it = n.iter()
        /* other stuff */
    {
        
    }
    
    true
}
A bit further minimized:
#![feature(let_chains)]

struct Pd;

impl Pd {
    fn it(&self) -> It {
        todo!()
    }
}

pub struct It<'a>(Box<dyn Tr<'a>>);

trait Tr<'a> {}

fn f(m: Option<Pd>) {
    if let Some(n) = m && let it = n.iter() {};
}

@WaffleLapkin
Copy link
Member

WaffleLapkin commented Nov 1, 2022

Even more minimized:

#![feature(let_chains)]
#![allow(irrefutable_let_patterns)]

fn it(_: &()) -> Box<dyn Tr<'_>> { todo!() }

trait Tr<'a> {}

fn f() {
    if let n = () && let _ = it(&n) {};
}
error[[E0597]](https://doc.rust-lang.org/nightly/error-index.html#E0597): `n` does not live long enough
 --> src/lib.rs:9:33
  |
9 |     if let n = () && let _ = it(&n) {};
  |                              ---^^-  -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `Box<dyn Tr<'_>>`
  |                              |  |    |
  |                              |  |    `n` dropped here while still borrowed
  |                              |  borrowed value does not live long enough
  |                              a temporary with access to the borrow is created here ...

[play]


Weirdly, I couldn't replace Box<dyn Tr<'a>> with anything else, I thought that it's just significant Drop & invariant lifetime 'a, but something like (String, fn(&'a ()) -> &'a ()) compiles, so I'm not sure what's going on.

@alercah
Copy link
Contributor

alercah commented Nov 3, 2022

Box's drop is actually hard-coded magic, so it might be unique to it.

@est31
Copy link
Member

est31 commented Nov 3, 2022

Good point about the lang item. I've tried it with Arc and Vec, it seems to fail for those too.

This also still reproduces:

#![feature(let_chains)]
#![allow(irrefutable_let_patterns)]

struct B<T: ?Sized> { _t: std::marker::PhantomData<T>, }

impl<T: ?Sized> std::ops::Drop for B<T> {
    fn drop(&mut self) {}
}

fn it(_: &()) -> B<dyn Tr<'_>> { todo!() }

trait Tr<'a> {}

fn f() {
    if let n = () && let _ = it(&n) {};
}

@nathanwhit
Copy link
Member

nathanwhit commented Nov 3, 2022

This reproduces as well:

#![feature(let_chains)]
#![allow(irrefutable_let_patterns)]

struct B<'a>(&'a ());

impl<'a> std::ops::Drop for B<'a> {
    fn drop(&mut self) {}
}

fn f() {
    if let n = () && let _ = B(&n) {};
}

If you remove the Drop impl it compiles without error.

@WaffleLapkin
Copy link
Member

The reason why Box<&'a ()> / Vec<&'a ()> don't work is probably because of #[may_dangle].

@est31
Copy link
Member

est31 commented Nov 4, 2022

There is no #[may_dangle] in the two last snippets, so I think it's not because of it.

@WaffleLapkin
Copy link
Member

@est31 that's the point? @nathanwhit's example can use &'a () because the drop doesn't have #[may_dangle]. Arc has #[may_dangle], so Arc<&'a ()> does not reproduce the issue (you need a dyn Trait because it's possibly drop and has the lifetime).

@est31
Copy link
Member

est31 commented Nov 4, 2022

Good point, you are right about #[may_dangle] I think. If you remove the unsafe from and #[may_dangle] from here it fails again (but works right now):

#![feature(dropck_eyepatch)]
#![feature(let_chains)]
#![allow(irrefutable_let_patterns)]

struct B<T: ?Sized> { _t: *const T, }

unsafe impl<#[may_dangle] T: ?Sized> std::ops::Drop for B<T> {
    fn drop(&mut self) {}
}

fn it(_: &()) -> B<&()> { todo!() }

trait Tr<'a> {}

fn f() {
    if let n = () && let _ = it(&n) {};
}

@WiSaGaN
Copy link
Contributor

WiSaGaN commented Dec 5, 2022

Does this still block the stabilization of let chains?

@est31
Copy link
Member

est31 commented Dec 5, 2022

@WiSaGaN it would probably be very useful for this issue to get input by a borrow checking expert on whether this issue is fixable without doing breaking changes, and whether a fix is easy/feasible, or hard. Also, there are plans to rewrite large parts of let chains.

@est31
Copy link
Member

est31 commented Dec 5, 2022

As for your question: I wouldn't delay let chains for this indefinitely, esp as there is limited progress, but it would be sad that "there was not enough attention from the right people on this" is the reason why some issue can't get fixed indefinitely.

Atm there are also other blockers too like #104843 and #104893. Especially latter would greatly benefit from input from third parties: try out let chains in your own code, and report bugs you find.

@est31
Copy link
Member

est31 commented Nov 16, 2024

This doesn't reproduce on edition 2024 any more, while still reproducing on edition 2021. I would say it's fixed by if let rescoping (#124085).

I've tried both the "a bit further minimized" snippet as well as the snippet right below by @WaffleLapkin.

As to the original snippet provided by @Alexendoo , same behaviour there, although note that latest syn (v1.0.109) does not reproduce the issue while an older version like 1.0.80 reproduces it.

I suppose this can be closed now once we have a test for it in the testsuite.

jhpratt added a commit to jhpratt/rust that referenced this issue Nov 17, 2024
Let chains tests

Filing this as this marks off two of the open issues in rust-lang#132833:

* extending the tests for `move-guard-if-let-chain.rs` and `conflicting_bindings.rs` to have chains with multiple let's (one implementation could for example search for the first `let` and then terminate).
* An instance where a temporary lives shorter than with nested ifs, breaking compilation: rust-lang#103476. This was fixed in the end by the if let rescoping work.

Closes rust-lang#103476
@bors bors closed this as completed in ccc3f86 Nov 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 17, 2024
Rollup merge of rust-lang#133093 - est31:let_chains_tests, r=traviscross

Let chains tests

Filing this as this marks off two of the open issues in rust-lang#132833:

* extending the tests for `move-guard-if-let-chain.rs` and `conflicting_bindings.rs` to have chains with multiple let's (one implementation could for example search for the first `let` and then terminate).
* An instance where a temporary lives shorter than with nested ifs, breaking compilation: rust-lang#103476. This was fixed in the end by the if let rescoping work.

Closes rust-lang#103476
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-let_chains `#![feature(let_chains)]`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants