Skip to content

pod!: bytemuk behind feature#181

Open
grod220 wants to merge 1 commit intomainfrom
bytemuck-feature-flag
Open

pod!: bytemuk behind feature#181
grod220 wants to merge 1 commit intomainfrom
bytemuck-feature-flag

Conversation

@grod220
Copy link
Member

@grod220 grod220 commented Feb 25, 2026

  • Gate all bytemuck derives behind a feature flag in spl-pod
  • Set default = [] so consumers opt in to only the features they need

/// This trait is used to indicate that a type can be `None` according to a
/// specific value.
pub trait Nullable: PartialEq + Pod + Sized {
pub trait Nullable: PartialEq + Copy + Sized {
Copy link
Member Author

Choose a reason for hiding this comment

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

Required as PodOption<T: Nullable> derives copy and previously satisfied by the Pod trait.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we remove the Copy bound actually? We can do something like the normal Option and include cloned() and copied() functions for when T: Clone or T:Copy: https://doc.rust-lang.org/std/option/enum.Option.html#method.cloned

bytemuck_derive = { version = "1.10.1", optional = true }
serde = { version = "1.0.228", optional = true, features = ["derive"] }
solana-address = { version = "2.2.0", features = ["bytemuck"] }
solana-address = { version = "2.2.0", features = ["copy"] }
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously provided transitively by bytemuck feature, now needs it explicitly added

@grod220 grod220 mentioned this pull request Feb 25, 2026
11 tasks
Base automatically changed from list-view-migration-pt2 to main February 26, 2026 08:56
@grod220 grod220 force-pushed the bytemuck-feature-flag branch from 21d48c2 to 50838e2 Compare February 26, 2026 08:58
@grod220 grod220 marked this pull request as ready for review February 26, 2026 09:05
@grod220 grod220 requested review from febo and joncinque February 26, 2026 09:06
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great overall! Just the question on the Copy bound

/// This trait is used to indicate that a type can be `None` according to a
/// specific value.
pub trait Nullable: PartialEq + Pod + Sized {
pub trait Nullable: PartialEq + Copy + Sized {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we remove the Copy bound actually? We can do something like the normal Option and include cloned() and copied() functions for when T: Clone or T:Copy: https://doc.rust-lang.org/std/option/enum.Option.html#method.cloned

edition = "2021"

[features]
default = []
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: If the default feature is empty, may as well not have it

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.

2 participants