Skip to content

Remove panic from code gen of an enum's FromReflect derive.#24068

Merged
alice-i-cecile merged 2 commits into
bevyengine:mainfrom
andriyDev:enum-reflect-panic
May 4, 2026
Merged

Remove panic from code gen of an enum's FromReflect derive.#24068
alice-i-cecile merged 2 commits into
bevyengine:mainfrom
andriyDev:enum-reflect-panic

Conversation

@andriyDev
Copy link
Copy Markdown
Contributor

@andriyDev andriyDev commented May 2, 2026

Objective

  • I noticed this while reviewing fix: Handle<T>::from_reflect incorrectly instantiating regardless of T type #24048. Our current FromReflect derive macro adds a panic for cases when a PartialReflect-ed enum includes a variant we weren't expecting. This can happen for a few reasons, e.g., a variant of the enum has been removed. In any case, crashing the program is not the correct answer, since we already have an appropriate response to a failed conversion: just return None!

Solution

  • Return None instead of panicking.

I also checked if there are any other panics, and it looks like none in the codegen. The only remaining panics have to do with some auto registration file stuff, which seems appropriate to panic on.

Testing

  • Added a unit test.

@andriyDev andriyDev added C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy A-Reflection Runtime information about types S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 2, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Reflection May 2, 2026
Copy link
Copy Markdown
Contributor

@makspll makspll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds valid to me!

@andriyDev andriyDev added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 2, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 3, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 3, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 3, 2026
Merged via the queue into bevyengine:main with commit 2730c6e May 4, 2026
49 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in Reflection May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Reflection Runtime information about types C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants