Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lang: add anchor-zero-copy-repr-packed feature flag #1199

Conversation

crispheaney
Copy link
Contributor

Addresses #1198.

Not sure the best way to write a test for this! If you have a suggestion I can add that as well

I tested locally by running the following in tests/chat/programs:

  • cargo expand
  • cargo expand --features anchor-zero-copy-repr-packed

@@ -289,7 +289,8 @@ pub fn zero_copy(

proc_macro::TokenStream::from(quote! {
#[derive(anchor_lang::__private::ZeroCopyAccessor, Copy, Clone)]
#[repr(C)]
#[cfg_attr(not(feature = "anchor-zero-copy-repr-packed"), repr(C))]
Copy link
Member

Choose a reason for hiding this comment

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

Can we pass this in as an argument instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

discussed elsewhere: instead of argument, would be even better to give the user the choice to add a repr
if they add one, do nothing.
If they don't. Add repr(C)

@armaniferrante
Copy link
Member

What do you think about an argument instead of a feature flag. E.g. #[account(zero_copy, packed)]

@crispheaney
Copy link
Contributor Author

Yeah I think that's probably nicer! Haven't worked in this part of the anchor code before, if you could link me to an example commit that does something similar, it might help me out

@armaniferrante
Copy link
Member

armaniferrante commented Jan 7, 2022

@crispheaney you can check if the repr attribute is on the syn::ItemStruct here https://github.com/project-serum/anchor/blob/master/lang/attribute/account/src/lib.rs#L292

See https://docs.rs/syn/1.0.85/syn/. Specifically the attrs field here https://docs.rs/syn/1.0.85/syn/struct.ItemStruct.html.

@crispheaney
Copy link
Contributor Author

This is supporting #[account(zero_copy, packed)] now.

I see @paul-schaaf suggested we support #[account(zero_copy, repr = packed)]. Do yall have an example on the cleanest way to parse the repr = packed? (Sorry I'm a marco noob)

@@ -79,6 +80,8 @@ pub fn account(
.collect();
if ns == "zero_copy" {
is_zero_copy = true;
} else if ns == "packed" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The account attribute only takes two args but it can't take a namespace so I think this is ok?

@paul-schaaf
Copy link
Contributor

@crispheaney
armani and I (we shouldve had the discussion here in the open mb) thought it would be better to have

[account(zero_copy)]
[repr(X)]

X being your desired alignment config
if the user marks it with a [repr(X)], use that. if there is none. add [repr(C)] to the struct.

you can still do that in the function of the account macro attribute

@armaniferrante
Copy link
Member

Implemented the repr override here #1273

@paul-schaaf
Copy link
Contributor

we have the override now so Im closing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants