Skip to content

Fix #22865 non_snake_case on bundle derive#22866

Open
SOF3 wants to merge 1 commit intobevyengine:mainfrom
SOF3:issues/22865
Open

Fix #22865 non_snake_case on bundle derive#22866
SOF3 wants to merge 1 commit intobevyengine:mainfrom
SOF3:issues/22865

Conversation

@SOF3
Copy link
Copy Markdown
Contributor

@SOF3 SOF3 commented Feb 8, 2026

Objective

Fixes #22865

Solution

Add #[allow(non_snake_case)] to avoid validating the field variable format.

Testing

  • Did you test these changes? If so, how?
  • Are there any parts that need more testing?
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?

Copilot AI review requested due to automatic review settings February 8, 2026 07:04
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes the new non_snake_case warning emitted by code generated via deconstruct_moving_ptr! when field-derived locals end up with names like field__marker (regression noted in #22865, bevy_ecs 0.18).

Changes:

  • Adds #[allow(non_snake_case)] to let bindings generated in the tuple and MaybeUninit::<tuple> arms of deconstruct_moving_ptr!.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1475 to +1478
$(
#[allow(non_snake_case)]
let $pattern = unsafe { ptr.move_field(|f| &raw mut (*f).$field_index) };
)*
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

This only adds #[allow(non_snake_case)] to the tuple (let tuple { ... }) and MaybeUninit::<tuple> arms. The derive(Bundle) expansion that triggered #22865 uses the struct-pattern arms (let StructName { field: field_local, ... } = ptr;), which still expand to let field__marker = ...; without an allow, so the original warning is likely unchanged. Consider applying the same #[allow(non_snake_case)] to the struct and MaybeUninit::<Struct> arms as well (the ones using get_pattern!).

Copilot uses AI. Check for mistakes.
Comment thread crates/bevy_ptr/src/lib.rs Outdated
// - Each field is distinct, since otherwise the block of code above would fail compilation
$(let $pattern = unsafe { ptr.move_field(|f| &raw mut (*f).$field_index) };)*
$(
#[allow(non_snake_case)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a comment explaining why this allow is needed :)

Comment thread crates/bevy_ptr/src/lib.rs Outdated
// - Each field is distinct, since otherwise the block of code above would fail compilation
$(let $pattern = unsafe { ptr.move_maybe_uninit_field(|f| &raw mut (*f).$field_index) };)*
$(
#[allow(non_snake_case)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditto on the comment here.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward D-Macros Code that generates Rust code labels Feb 8, 2026
@SOF3
Copy link
Copy Markdown
Contributor Author

SOF3 commented Feb 8, 2026

CI is failing due to a GitHub network error, could you trigger a rerun?

@chescock
Copy link
Copy Markdown
Contributor

chescock commented Feb 9, 2026

This doesn't seem like the right place to fix this. If a user wants to provide a non-snake-case identifier, can't they write #[allow(non_snake_case)] outside the macro invocation?

Looking at the linked issue, I see this is happening with #[derive(Bundle)]. ... Ah, okay, that generates local variables by doing format_ident!("field_{}", field_member), so it gets field__marker with two underscores when the field is _marker.

let field_local = format_ident!("field_{}", field_member);

I think it would be better to add #[allow(non_snake_case)] to the output of #[derive(Bundle)], since that's what's generating the non-snake-case identifier. That lets us preserve the warning for hand-written deconstruct_moving_ptr calls that do want that lint.

That would mean adding it to the DynamicBundle::get_components method, since that's where the deconstruct_moving_ptr call is:

#[allow(unused_variables)]
#[inline]
unsafe fn get_components(
ptr: #ecs_path::ptr::MovingPtr<'_, Self>,
func: &mut impl FnMut(#ecs_path::component::StorageType, #ecs_path::ptr::OwningPtr<'_>)
) {
use #ecs_path::__macro_exports::DebugCheckedUnwrap;
#ecs_path::ptr::deconstruct_moving_ptr!({
let #struct_name { #(#active_field_members: #active_field_locals,)* #(#inactive_field_members: _,)* } = ptr;
});

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 10, 2026
@cart cart added this to ECS Feb 12, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS Feb 12, 2026
@alice-i-cecile alice-i-cecile moved this from Needs SME Triage to SME Triaged in ECS Feb 12, 2026
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-Usability A targeted quality-of-life change that makes Bevy easier to use D-Macros Code that generates Rust code D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged

Projects

Status: SME Triaged

Development

Successfully merging this pull request may close these issues.

derive(Bundle) results in non_snake_case lint

5 participants