[Merged by Bors] - Allow returning a value from EntityMut::world_scope
#7385
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoJoJet
added
C-Feature
james7132
approved these changes
Jan 27, 2023
|
Definitely better. Can we remove |
|
alice-i-cecile
added
C-Usability
alice-i-cecile
added
the
S-Ready-For-Final-Review
bors bot
pushed a commit
that referenced
this pull request
Jan 27, 2023
# Objective The function `EntityMut::world_scope` is a safe abstraction that allows you to temporarily get mutable access to the underlying `World` of an `EntityMut`. This function is purely stateful, meaning it is not easily possible to return a value from it. ## Solution Allow returning a computed value from the closure. This is similar to how `World::resource_scope` works. --- ## Changelog - The function `EntityMut::world_scope` now allows returning a value from the immediately-computed closure.
bors
bot
changed the title
EntityMut::world_scopeEntityMut::world_scope
bors bot
pushed a commit
that referenced
this pull request
Jan 29, 2023
# Objective Found while working on #7385. The struct `EntityMut` has the safety invariant that it's cached `EntityLocation` must always accurately specify where the entity is stored. Thus, any time its location might be invalidated (such as by calling `EntityMut::world_mut` and moving archetypes), the cached location *must* be updated by calling `EntityMut::update_location`. The method `world_scope` encapsulates this pattern in safe API by requiring world mutations to be done in a closure, after which `update_location` will automatically be called. However, this method has a soundness hole: if a panic occurs within the closure, then `update_location` will never get called. If the panic is caught in an outer scope, then the `EntityMut` will be left with an outdated location, which is undefined behavior. An example of this can be seen in the unit test `entity_mut_world_scope_panic`, which has been added to this PR as a regression test. Without the other changes in this PR, that test will invoke undefined behavior in safe code. ## Solution Call `EntityMut::update_location()` from within a `Drop` impl, which ensures that it will get executed even if `EntityMut::world_scope` unwinds.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective The function `EntityMut::world_scope` is a safe abstraction that allows you to temporarily get mutable access to the underlying `World` of an `EntityMut`. This function is purely stateful, meaning it is not easily possible to return a value from it. ## Solution Allow returning a computed value from the closure. This is similar to how `World::resource_scope` works. --- ## Changelog - The function `EntityMut::world_scope` now allows returning a value from the immediately-computed closure.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective Found while working on bevyengine#7385. The struct `EntityMut` has the safety invariant that it's cached `EntityLocation` must always accurately specify where the entity is stored. Thus, any time its location might be invalidated (such as by calling `EntityMut::world_mut` and moving archetypes), the cached location *must* be updated by calling `EntityMut::update_location`. The method `world_scope` encapsulates this pattern in safe API by requiring world mutations to be done in a closure, after which `update_location` will automatically be called. However, this method has a soundness hole: if a panic occurs within the closure, then `update_location` will never get called. If the panic is caught in an outer scope, then the `EntityMut` will be left with an outdated location, which is undefined behavior. An example of this can be seen in the unit test `entity_mut_world_scope_panic`, which has been added to this PR as a regression test. Without the other changes in this PR, that test will invoke undefined behavior in safe code. ## Solution Call `EntityMut::update_location()` from within a `Drop` impl, which ensures that it will get executed even if `EntityMut::world_scope` unwinds.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
The function
EntityMut::world_scopeis a safe abstraction that allows you to temporarily get mutable access to the underlyingWorldof anEntityMut. This function is purely stateful, meaning it is not easily possible to return a value from it.Solution
Allow returning a computed value from the closure. This is similar to how
World::resource_scopeworks.Changelog
EntityMut::world_scopenow allows returning a value from the immediately-computed closure.