Skip to content

Comments

[derive] Support #[zerocopy(crate = "...")] attribute#2452

Merged
joshlf merged 1 commit intogoogle:mainfrom
andogq:feat/derive_crate_name
Mar 21, 2025
Merged

[derive] Support #[zerocopy(crate = "...")] attribute#2452
joshlf merged 1 commit intogoogle:mainfrom
andogq:feat/derive_crate_name

Conversation

@andogq
Copy link
Contributor

@andogq andogq commented Mar 19, 2025

Read the #[zerocopy(crate = "...")] attribute (if present), and use the provided path in the derive output when referencing items exported from zerocopy. If the attribute isn't present, then ::zerocopy will be used.

Attention is still needed on the test case. It currently functions on a subset of zerocopy traits using a similar technique to serde.

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.38%. Comparing base (ed32df4) to head (2dea8f2).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2452   +/-   ##
=======================================
  Coverage   90.38%   90.38%           
=======================================
  Files          18       18           
  Lines        7288     7288           
=======================================
  Hits         6587     6587           
  Misses        701      701

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andogq andogq force-pushed the feat/derive_crate_name branch 4 times, most recently from 553f616 to bb297d5 Compare March 19, 2025 03:25
joshlf
joshlf requested changes Mar 19, 2025
View reviewed changes
Copy link
Member

@joshlf joshlf left a comment

Choose a reason for hiding this comment

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

Thanks so much for your work on this! I left a few minor comments, but overall this looks great!

@joshlf
Copy link
Member

joshlf commented Mar 19, 2025

I've put up #2456 to migrate impl_block to a builder pattern, which might make your life easier here (so that passing the crate name around isn't quite as painful).

@andogq andogq force-pushed the feat/derive_crate_name branch from bb297d5 to 523954f Compare March 20, 2025 12:16
@andogq
Copy link
Contributor Author

andogq commented Mar 20, 2025

Thanks for getting that builder in, certainly simplified things!

I found that the padding macros in src/util/macro_util.rs (struct_has_padding, union_has_padding, ...) had the crate hard coded so I fixed this too. The problem is that it's not picked up in the test I've included, and I could not think of a good way to test it given that the trick we're already using can't be applied in the same way. Potentially this is a non-issue given that using $crate is good practice anyway.

@andogq andogq requested a review from joshlf March 20, 2025 12:30
joshlf
joshlf requested changes Mar 20, 2025
View reviewed changes
Copy link
Member

@joshlf joshlf left a comment

Choose a reason for hiding this comment

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

One more small nit, but otherwise this looks great!

@joshlf
Copy link
Member

joshlf commented Mar 20, 2025

Thanks for getting that builder in, certainly simplified things!

np!

I found that the padding macros in src/util/macro_util.rs (struct_has_padding, union_has_padding, ...) had the crate hard coded so I fixed this too. The problem is that it's not picked up in the test I've included, and I could not think of a good way to test it given that the trick we're already using can't be applied in the same way. Potentially this is a non-issue given that using $crate is good practice anyway.

Yeah, agreed that $crate should be fine. Honestly not sure why we didn't just do that to begin with.

@andogq andogq force-pushed the feat/derive_crate_name branch from 523954f to b248cb1 Compare March 21, 2025 11:29
@andogq
[derive] Support #[zerocopy(crate = "...")] attribute
2dea8f2 
Read the `#[zerocopy(crate = "...")]` attribute (if present), and use
the provided path in the derive output when referencing items exported
from zerocopy. If the attribute isn't present, then `::zerocopy` will be
used.
@andogq andogq force-pushed the feat/derive_crate_name branch from b248cb1 to 2dea8f2 Compare March 21, 2025 11:31
@andogq andogq requested a review from joshlf March 21, 2025 11:39
joshlf
joshlf approved these changes Mar 21, 2025
View reviewed changes
Copy link
Member

@joshlf joshlf left a comment

Choose a reason for hiding this comment

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

Fantastic, thanks again!

@joshlf joshlf added this pull request to the merge queue Mar 21, 2025
@joshlf joshlf mentioned this pull request Oct 10, 2024
Open
7 tasks
@andogq
Copy link
Contributor Author

andogq commented Mar 21, 2025

No problem, thank you for all your work!

Merged via the queue into google:main with commit 5056899 Mar 21, 2025
88 checks passed
@andogq andogq deleted the feat/derive_crate_name branch March 21, 2025 12:20
@joshlf
Copy link
Member

joshlf commented Mar 21, 2025

This is now available in 0.8.24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshlf joshlf joshlf approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andogq @codecov-commenter @joshlf