Skip to content

2024 edition regression: cannot write blanket implementation for closures that return the Never type #139610

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

Open
alice-i-cecile opened this issue Apr 10, 2025 · 7 comments
Labels
A-edition-2024 Area: The 2024 edition F-never_type `#![feature(never_type)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-edition Relevant to the edition team. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@alice-i-cecile
Copy link

As laid out in the guide for the Never type fallback, closures which panic are now inferred to return !, rather than ().

In Bevy, we encountered this surprising breaking change in the form of bevyengine/bevy#18778. While individual users can work around this by writing |my closure | -> () and explicitly specifying a return type or by using an ordinary function, we cannot work around this breakage ourselves by writing another blanket implementation because the Never type (!) is not available on stable Rust.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 10, 2025
@alice-i-cecile
Copy link
Author

alice-i-cecile commented Apr 10, 2025

We might be able to abuse the fact that Never is exposed via stable trait impls to work around this, ala https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=77fbd787405712a153404ca23d5d0263. I'm not sure that that's something the Rust team would appreciate a major crate relying on though.

EDIT: It's not clear that a workaround of this form will work for us, but we'd be happy to be proven wrong!

@jieyouxu jieyouxu added T-types Relevant to the types team, which will review and decide on the PR/issue. A-edition-2024 Area: The 2024 edition T-edition Relevant to the edition team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-untriaged Untriaged performance or correctness regression. labels Apr 10, 2025
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 10, 2025
@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Apr 10, 2025

We might be able to abuse the fact that Never is exposed via stable trait impls to work around this, ala https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=77fbd787405712a153404ca23d5d0263.

No, that is not the never type. That is core::convert::Infallible, which is planned to become an alias of the never type in the future, but currently is not. The workaround you are looking for is:

mod fn_ret {
    pub trait FnRet {
        type Output;
    }

    impl<R> FnRet for fn() -> R {
        type Output = R;
    }
}

pub type Never = <fn() -> ! as fn_ret::FnRet>::Output;

(The never-say-never crate, with nearly 1.5 million downloads, uses this trick.)

@alice-i-cecile
Copy link
Author

That workaround should work for us! Would you like me to leave this issue open or close it?

@lcnr
Copy link
Contributor

lcnr commented Apr 10, 2025

To my knowledge the never type is currently only really unstable to avoid people relying on Infallible and ! being different types.

So as long as you don't implement IntoSystem<Infallible> and IntoSystem<!> this should be fine. cc @WaffleLapkin

@lcnr
Copy link
Contributor

lcnr commented Apr 10, 2025

I think keeping this open as fine in case other people encounter this issue as well

@WaffleLapkin WaffleLapkin added the F-never_type `#![feature(never_type)]` label Apr 10, 2025
@WaffleLapkin
Copy link
Member

WaffleLapkin commented Apr 10, 2025

So, on the regression side -- I think this is entirely expected. The reason this worked before is incidental, there is nothing in the code that should force the closure to return () rather than !. On older editions you get a future compatibility warning, so that also works as expected.

For the workaround, I think @lcnr's assesment is correct -- the only thing stopping never type from being stabilized is our desire to make Infallible = ! which is blocked on a breaking change we plan to rollout sometime later (the same change as in the edition). So as long as your code works with Infallible being an alias for !, and you don't expose ! to the users in a way where their code can break once Infallible becomes !, you should be fine.

That being said you are still opting out of stability guarantees, so should is just that -- should.

@WaffleLapkin WaffleLapkin removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 10, 2025
@alice-i-cecile
Copy link
Author

alice-i-cecile commented Apr 11, 2025

You can track our workaround PR here: bevyengine/bevy#18804. I've tried to leave copious warnings and explanations. I will be mildly annoyed if this hack breaks before the real ! type stabilizes, but we can just rip it out in simple backported point releases and we won't be any worse off than if we never did it at all.

From my perspective, this is primarily an "ease user pain in tests and prototypes" problem (todo! ergonomic regressions are by far the worst), which is something we can tolerate sacrificing unexpectedly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2024 Area: The 2024 edition F-never_type `#![feature(never_type)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-edition Relevant to the edition team. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants