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

NLL diagnostic regression on generator + short-lived yield #56508

Closed
pnkfelix opened this issue Dec 4, 2018 · 4 comments · Fixed by #60176
Closed

NLL diagnostic regression on generator + short-lived yield #56508

pnkfelix opened this issue Dec 4, 2018 · 4 comments · Fixed by #60176
Assignees
Labels
A-coroutines Area: Coroutines A-NLL Area: Non Lexical Lifetimes (NLL) E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. NLL-diagnostics Working torwads the "diagnostic parity" goal P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Dec 4, 2018

Spawned off of issue #55850 and PR #56460

Consider code like this (it is issue-55850.rs from PR #56460):

#![allow(unused_mut)]
#![feature(generators, generator_trait)]

use std::ops::Generator;
use std::ops::GeneratorState::Yielded;

pub struct GenIter<G>(G);

impl <G> Iterator for GenIter<G>
where
    G: Generator,
{
    type Item = G::Yield;

    fn next(&mut self) -> Option<Self::Item> {
        unsafe {
            match self.0.resume() {
                Yielded(y) => Some(y),
                _ => None
            }
        }
    }
}

fn bug<'a>() -> impl Iterator<Item = &'a str> {
    GenIter(move || {
        let mut s = String::new();
        yield &s[..]
    })
}

fn main() {
    bug();
}

Right now, AST-borrowck includes this note:

note: borrowed value must be valid for the lifetime 'a as defined on the function body at 35:8...
  --> $DIR/issue-55850.rs:35:8
   |
LL | fn bug<'a>() -> impl Iterator<Item = &'a str> {
   |        ^^

but even after PR #56460 lands (fixing an ICE), NLL does not include such a note.

@pnkfelix pnkfelix added A-NLL Area: Non Lexical Lifetimes (NLL) A-coroutines Area: Coroutines NLL-diagnostics Working torwads the "diagnostic parity" goal labels Dec 4, 2018
@davidtwco
Copy link
Member

davidtwco commented Dec 4, 2018

Results/thoughts from initial investigation is available on Zulip.

@matthewjasper matthewjasper self-assigned this Feb 10, 2019
@matthewjasper matthewjasper added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Feb 24, 2019
@pnkfelix
Copy link
Member Author

triage: Nominating for discussion at tonight's NLL meeting. (I want help figuring out what is the scope of programs that can run into this, with a special focus on ones using async/await, before attempting to assign a formal priority.)

@pnkfelix
Copy link
Member Author

triage: T-compiler. Was briefly discussed with @matthewjasper who thinks that this problem shouldn't arise with async/await itself, because that "yields a fieldless variant of Poll"

@pnkfelix
Copy link
Member Author

triage: P-medium on assumption that async/await itself won't encounter this particular issue.

@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. P-medium Medium priority and removed I-nominated labels Mar 21, 2019
Centril added a commit to Centril/rust that referenced this issue May 13, 2019
…=pnkfelix

Explain error when yielding a reference to a local variable

Closes rust-lang#56508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coroutines Area: Coroutines A-NLL Area: Non Lexical Lifetimes (NLL) E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. NLL-diagnostics Working torwads the "diagnostic parity" goal P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants