[derive] Stop emitting non-ASCII identifiers from TryFromBytes derive - #3565
[derive] Stop emitting non-ASCII identifiers from TryFromBytes derive#3565rootkiller6788 wants to merge 1 commit into
Conversation
Fixes google#2880. The TryFromBytes derive generates per-field marker enum types using a non-ASCII identifier prefix (`ẕ`, U+1E95) so that they can't collide with user identifiers. These markers are emitted into the user's crate, and so trigger the `non_ascii_idents` lint when the user has `#![deny(non_ascii_idents)]` enabled. Because rustc currently ignores `#[allow(non_ascii_idents)]` at non-crate levels (rust-lang/rust#151025), the only viable fix is to stop using non-ASCII identifiers internally. Rename the generated marker types from `ẕ<field>` to `___ZerocopyHasField_<field>`, following the existing `___ZerocopyRawEnum` / `___ZcAlignment` naming convention used for other derive-generated items. Enable `FromBytes` derivation in the google#2880 regression test, and update the generated-output and UI test expectations.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3565 +/- ##
=======================================
Coverage 91.85% 91.85%
=======================================
Files 20 20
Lines 6093 6093
=======================================
Hits 5597 5597
Misses 496 496 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28d5315c98
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .build(); | ||
| let has_fields = fields.iter().map(move |(_, ident, ty)| { | ||
| let field_token = ident!(("ẕ{}", ident), ident.span()); | ||
| let field_token = ident!(("___ZerocopyHasField_{}", ident), ident.span()); |
There was a problem hiding this comment.
When a struct or union has a type parameter named after this new predictable marker (for example, struct S<___ZerocopyHasField_a> { private a: ___ZerocopyHasField_a }), this reference inside the generated impl resolves to that type parameter rather than the private marker enum. The resulting HasField<___ZerocopyHasField_a, ...> impl is externally usable, so code outside the defining module can infer the marker and project the private field, violating HasField's requirement that its Field parameter have the field's visibility. Generate a marker that cannot shadow any input type parameter, or explicitly disambiguate/rename collisions.
Useful? React with 👍 / 👎.
|
See #2880 (comment) |
3 participants
Fixes #2880.
Motivation
When a user applies
#![deny(non_ascii_idents)]at their crate root and derivesFromBytes/TryFromBytes, compilation fails:The cause is that the
TryFromBytesderive generates per-field marker enum types whose names are prefixed withẕ(U+1E95) - a non-ASCII character chosen to avoid colliding with user identifiers. These markers are emitted into the user's crate, and since their span points back at the user's field, thenon_ascii_identslint fires at the field even though the offending identifier lives in generated code.Emitting
#[allow(non_ascii_idents)]around the generated items doesn't work: rustc currently ignoresallow(non_ascii_idents)at non-crate levels (see rust-lang/rust#151025), so the lint still fires.Change
Rename the generated marker types from
ẕ<field>to___ZerocopyHasField_<field>, following the existing___ZerocopyRawEnum/___ZcAlignmentnaming convention used for other derive-generated items. All generated identifiers are now ASCII, so the lint can no longer fire regardless of lint configuration.Additionally:
FromBytesderivation in theissue_2880.rsregression test (removing the FIXME) so the fix is covered by CI.output_testsexpected files.privacyUI-test annotations and.stderrfiles for the renamed marker types.Testing
cargo test -p zerocopy-derive(output tests + derive integration tests) passes.issue_2880test compiles under#![deny(non_ascii_idents)]withFromBytesderived on structs, unions, and enums.cargo test -p zerocopy --libpasses.