Skip to content

Use never_say_never hack to work around Rust 2024 regression for fn traits #18804

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

Merged
merged 6 commits into from
Apr 14, 2025

Conversation

alice-i-cecile
Copy link
Member

Objective

After #17967, closures which always panic no longer satisfy various Bevy traits. Principally, this affects observers, systems and commands.

While this may seem pointless (systems which always panic are kind of useless), it is distinctly annoying when using the todo! macro, or when writing tests that should panic.

Fixes #18778.

Solution

  • Add failing tests to demonstrate the problem
  • Add the trick from never_say_never to name the ! type on stable Rust
  • Write looots of docs explaining what the heck is going on and why we've done this terrible thing

To do

Unfortunately I couldn't figure out how to avoid conflicting impls, and I am out of time for today, the week and uh the week after that. Vacation! If you feel like finishing this for me, please submit PRs to my branch and I can review and press the button for it while I'm off.

Unless you're Cart, in which case you have write permissions to my branch!

Testing

I've added a compile test for these failure cases and a few adjacent non-failing cases (with explicit return types).

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! X-Controversial There is active debate or serious implications around merging this PR labels Apr 11, 2025
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Apr 11, 2025
@cart cart marked this pull request as ready for review April 11, 2025 20:40
@cart
Copy link
Member

cart commented Apr 11, 2025

I've added the relevant impls!

@alice-i-cecile alice-i-cecile added X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! X-Controversial There is active debate or serious implications around merging this PR labels Apr 11, 2025
@cart cart added this pull request to the merge queue Apr 14, 2025
Merged via the queue into bevyengine:main with commit 9254297 Apr 14, 2025
36 checks passed
mockersf pushed a commit that referenced this pull request Apr 14, 2025
…raits (#18804)

# Objective

After #17967, closures which always panic no longer satisfy various Bevy
traits. Principally, this affects observers, systems and commands.

While this may seem pointless (systems which always panic are kind of
useless), it is distinctly annoying when using the `todo!` macro, or
when writing tests that should panic.

Fixes #18778.

## Solution

- Add failing tests to demonstrate the problem
- Add the trick from
[`never_say_never`](https://docs.rs/never-say-never/latest/never_say_never/)
to name the `!` type on stable Rust
- Write looots of docs explaining what the heck is going on and why
we've done this terrible thing

## To do

Unfortunately I couldn't figure out how to avoid conflicting impls, and
I am out of time for today, the week and uh the week after that.
Vacation! If you feel like finishing this for me, please submit PRs to
my branch and I can review and press the button for it while I'm off.

Unless you're Cart, in which case you have write permissions to my
branch!

- [ ] fix for commands
- [ ] fix for systems
- [ ] fix for observers
- [ ] revert bevyengine/bevy-website#2092

## Testing

I've added a compile test for these failure cases and a few adjacent
non-failing cases (with explicit return types).

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
None yet
3 participants