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

bevy_reflect: Fix trailing comma breaking derives #8014

Merged
merged 2 commits into from
Mar 27, 2023

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Mar 10, 2023

Objective

Fixes #7989

Based on #7991 by @CoffeeVampir3

Solution

There were three parts to this issue:

  1. extend_where_clause did not account for the optionality of a where clause's trailing comma
    // OKAY
    struct Foo<T> where T: Asset, {/* ... */}
    // ERROR
    struct Foo<T> where T: Asset {/* ... */}
  2. FromReflect derive logic was not actively using extend_where_clause which led to some inconsistencies (enums weren't adding any additional bounds even)
  3. Using extend_where_clause in the FromReflect derive logic meant we had to optionally add Default bounds to ignored fields iff the entire item itself was not already Default (otherwise the definition for Handle<T> wouldn't compile since HandleType doesn't impl Default but Handle<T> itself does)

Changelog

  • Fixed issue where a missing trailing comma could break the reflection derives

@MrGVSV MrGVSV added C-Bug An unexpected or incorrect behavior A-Reflection Runtime information about types labels Mar 10, 2023
@MrGVSV MrGVSV added this to the 0.10.1 milestone Mar 21, 2023
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

Makes sense. Nice fix!

@cart cart added this pull request to the merge queue Mar 27, 2023
Merged via the queue into bevyengine:main with commit 5e5a305 Mar 27, 2023
mockersf pushed a commit that referenced this pull request Mar 27, 2023
# Objective

Fixes #7989

Based on #7991 by @CoffeeVampir3

## Solution

There were three parts to this issue:
1. `extend_where_clause` did not account for the optionality of a where
clause's trailing comma
    ```rust
    // OKAY
    struct Foo<T> where T: Asset, {/* ... */}
    // ERROR
    struct Foo<T> where T: Asset {/* ... */}
    ```
2. `FromReflect` derive logic was not actively using
`extend_where_clause` which led to some inconsistencies (enums weren't
adding _any_ additional bounds even)
3. Using `extend_where_clause` in the `FromReflect` derive logic meant
we had to optionally add `Default` bounds to ignored fields iff the
entire item itself was not already `Default` (otherwise the definition
for `Handle<T>` wouldn't compile since `HandleType` doesn't impl
`Default` but `Handle<T>` itself does)

---

## Changelog

- Fixed issue where a missing trailing comma could break the reflection
derives
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
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Issue with specific reflect breaking after upgrading from 0.9-0.10
2 participants