Skip to content

Use stabilized Layout::repeat#24006

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
chescock:Layout-repeat
Apr 28, 2026
Merged

Use stabilized Layout::repeat#24006
alice-i-cecile merged 1 commit intobevyengine:mainfrom
chescock:Layout-repeat

Conversation

@chescock
Copy link
Copy Markdown
Contributor

Objective

BlobArray has copies of Layout::repeat with a note to replace them when it stabilizes. Layout::repeat stabilized in Rust 1.95, so let's use it!

Fix some UB in an edge case. BlobArray assumes that the size() of a type is a multiple of its align(). That's true for all Rust types, but a user may register a component using register_component_with_descriptor passing a Layout where it's not true. In release mode, that causes UB due to unaligned memory access. In debug mode, it triggers a debug_assert!, and then Table::alloc_columns aborts the process with AbortOnPanic.

Solution

Ensure that the Layout used by BlobArray has a size() that is a multiple of its align(), by making it a safety requirement of BlobArray and having ComponentDescriptor enforce it.

Remove the repeat_layout methods. Replace calls to array_layout with Layout::repeat_packed. Since size() is a multiple of align(), that has the same behavior as Layout::repeat, but is simpler. Replace calls to array_layout_unchecked with Layout::repeat_packed followed by debug_checked_unwrap.

Move the check that `size == stride` earlier so that it does not abort the process.
@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy A-Pointers Relating to Bevy pointer abstractions C-Code-Quality A section of code that is hard to understand or change X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples A-ECS Entities, components, systems, and events and removed D-Trivial Nice and easy! A great choice to get started with Bevy A-Pointers Relating to Bevy pointer abstractions labels Apr 27, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Apr 27, 2026
@alice-i-cecile alice-i-cecile added D-Unsafe Touches with unsafe code in some way S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 27, 2026
@alice-i-cecile alice-i-cecile requested a review from kfc35 April 28, 2026 16:53
@alice-i-cecile alice-i-cecile added the P-Unsound A bug that results in undefined compiler behavior label Apr 28, 2026
@alice-i-cecile alice-i-cecile added this to the 0.19 milestone Apr 28, 2026
Copy link
Copy Markdown
Contributor

@Victoronz Victoronz 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!

My understanding of the reason for Layout not enforcing this property on its own is that Rust types have the requirement, but allocations do not.
I think in practice however, most allocators already comply anyway, and we may one day get some updated Allocator trait with a version of Layout that properly enshrines this.

@Victoronz Victoronz 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 Apr 28, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 28, 2026
Merged via the queue into bevyengine:main with commit 480ad98 Apr 28, 2026
63 checks passed
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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples D-Unsafe Touches with unsafe code in some way P-Unsound A bug that results in undefined compiler behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

3 participants