Skip to content

Remove interner_name argument from define_label! and make the interner private.#24445

Merged
alice-i-cecile merged 1 commit into
bevyengine:mainfrom
kpreid:label-interner
Jun 2, 2026
Merged

Remove interner_name argument from define_label! and make the interner private.#24445
alice-i-cecile merged 1 commit into
bevyengine:mainfrom
kpreid:label-interner

Conversation

@kpreid
Copy link
Copy Markdown
Contributor

@kpreid kpreid commented May 25, 2026

Objective

Simplify the use of define_label!().

Solution

  • Move the static interner inside the intern() method, so that it is not visible anywhere else and does not need to have a non-conflicting name. The only operation the interner has is interning, so the intern() trait method is all the access that is ever needed.
  • Also added support for trailing commas.

This PR will conflict with #24444. I will rebase whichever one doesn’t get merged first.

Testing

  • Ran tests with cargo run -p ci -- --keep-going test. There were some failures that I believe are existing/spurious.

@kfc35 kfc35 added A-App Bevy apps and plugins S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Macros Code that generates Rust code labels May 25, 2026
Copy link
Copy Markdown
Contributor

@ChristopherBiscardi ChristopherBiscardi left a comment

Choose a reason for hiding this comment

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

This moves the static from the top level to the interior of the intern function, which I believe is fine since statics are only initialized at compile time.

The old static is technically visible inside the module, so someone could be accessing it and it might warrant a migration guide?

@kpreid
Copy link
Copy Markdown
Contributor Author

kpreid commented May 26, 2026

This moves the static from the top level to the interior of the intern function, which I believe is fine since statics are only initialized at compile time.

That’s right. This is a common Rust pattern, though more often for statics that need lazy initialization.

The old static is technically visible inside the module, so someone could be accessing it and it might warrant a migration guide?

More importantly, users of the macro need to change the way they call it! I have added a migration guide.

@alice-i-cecile alice-i-cecile 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 Jun 2, 2026
@alice-i-cecile
Copy link
Copy Markdown
Member

Merging #24444; ping me once the conflicts from that are resolved.

MyCodingSpace5 pushed a commit to MyCodingSpace5/bevy that referenced this pull request Jun 2, 2026
…e#24444)

# Objective

Fixes <bevyengine#22834>.

## Solution

- Add more explanation of what `define_label!` does
- Add example implementations
- Change the existing `extra_methods_impl` example to forward the call

This PR will conflict with
bevyengine#24445. I will rebase whichever
one doesn’t get merged first.

## Testing

- Ran doctests
- Reviewed rendered documentation
…erner private.

The only operation the interner has is interning, so the `intern()`
trait method is all the access that is ever needed.

Also added support for trailing commas.
@kpreid
Copy link
Copy Markdown
Contributor Author

kpreid commented Jun 2, 2026

@alice-i-cecile Rebased.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 2, 2026
Merged via the queue into bevyengine:main with commit 163bec9 Jun 2, 2026
38 checks passed
@kpreid kpreid deleted the label-interner branch June 3, 2026 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-App Bevy apps and plugins D-Macros Code that generates Rust code 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.

4 participants