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

Encode optimized MIR of generators when emitting metadata #81003

Merged
merged 1 commit into from
Jan 14, 2021

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Jan 14, 2021

No description provided.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 14, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Jan 14, 2021

@bors r+ p=1 (regression fix)

@bors
Copy link
Contributor

bors commented Jan 14, 2021

📌 Commit ea4cbff has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 14, 2021
@hellow554
Copy link
Contributor

hellow554 commented Jan 14, 2021

hey @tmiasko

does your PR also fix this:

#![feature(type_alias_impl_trait)]

use std::future::Future;

pub struct Task<F: Future>(F);
impl<F: Future> Task<F> {
    fn new() -> Self {
        todo!()
    }
    fn spawn(&self, _: impl FnOnce() -> F) {
        todo!()
    }
}

fn main() {
    async fn cb() {
        let a = Foo;
    }

    type F = impl Future;
    static POOL: Task<F> = Task::new();
    Task::spawn(&POOL, || cb());
}

oh, BTW: the feature flag isn't needed to trigger the ICE itself, worth investigating?

@tmiasko
Copy link
Contributor Author

tmiasko commented Jan 14, 2021

Thanks, it looks like there are multiple issues there (and this PR fixes only one of them).

@hellow554
Copy link
Contributor

Ok, I posted my code in #80998 and I see you altered your initial post. Thanks!

@bors
Copy link
Contributor

bors commented Jan 14, 2021

⌛ Testing commit ea4cbff with merge 7bb1630...

@bors
Copy link
Contributor

bors commented Jan 14, 2021

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 7bb1630 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 14, 2021
@bors bors merged commit 7bb1630 into rust-lang:master Jan 14, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 14, 2021
@tmiasko tmiasko deleted the generator-layout branch January 14, 2021 14:42
@rylev
Copy link
Member

rylev commented Jan 20, 2021

@tmiasko @oli-obk this has some fairly significant performance regressions. Is there any way to mitigate that?

@oli-obk
Copy link
Contributor

oli-obk commented Jan 20, 2021

Not really, these performance regressions were "gained" in #80718 (comment) due to us forgetting to encode something that is definitely necessary... at least for libraries, not sure whether it makes sense to emit most of this stuff for binaries, and not sure whether we do that at all, but it could be an easy win if we aren't doing it already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants