Skip to content

Make TypeId::hash more robust in case of upstream rustc changes#11334

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
SpecificProtagonist:typeid-no-rehash-followup
Jan 14, 2024
Merged

Make TypeId::hash more robust in case of upstream rustc changes#11334
alice-i-cecile merged 1 commit intobevyengine:mainfrom
SpecificProtagonist:typeid-no-rehash-followup

Conversation

@SpecificProtagonist
Copy link
Contributor

Based on discussion after #11268 was merged:
Instead of panicking should the impl of TypeId::hash change significantly, have a fallback and detect this in a test.

@alice-i-cecile alice-i-cecile changed the title Followup to #11268 Make TypeId::hash more robust Jan 13, 2024
@alice-i-cecile alice-i-cecile changed the title Make TypeId::hash more robust Make TypeId::hash more robust in case of upstream rustc changes Jan 13, 2024
@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-ECS Entities, components, systems, and events labels Jan 13, 2024
@alice-i-cecile
Copy link
Member

@NthTensor @orzogc I'd appreciate a quick review from one or both of you here :)

Copy link
Contributor

@NthTensor NthTensor left a comment

Choose a reason for hiding this comment

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

Yeah, this looks like the best approach. Good work!

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 14, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 14, 2024
Merged via the queue into bevyengine:main with commit cd12e7c Jan 14, 2024
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-Code-Quality A section of code that is hard to understand or change S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants