Conversation
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 { |
There was a problem hiding this comment.
Required as PodOption<T: Nullable> derives copy and previously satisfied by the Pod trait.
There was a problem hiding this comment.
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"] } |
There was a problem hiding this comment.
Previously provided transitively by bytemuck feature, now needs it explicitly added
21d48c2 to
50838e2
Compare
joncinque
left a comment
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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 = [] |
There was a problem hiding this comment.
nit: If the default feature is empty, may as well not have it