Conversation
| /// The implementer must ensure that the enum discriminants have no gaps. Failing to do so will | ||
| /// allow out of bounds access to the stepper array as well as cause unknown discriminants. | ||
| #[proc_macro_derive(StepperStates)] | ||
| pub fn derive_stepper_states(input: TokenStream) -> TokenStream { |
There was a problem hiding this comment.
This is a cool idea and I like the direction, but I think some improvements can be made in terms of safety:
- Require concrete stepper types to implement an
unsafetrait that states their step array size. There's no unsafe derive macros, so currently there's a soundness hole (even if the comment states the invariants). - Check for discriminator gaps.
There was a problem hiding this comment.
Isn't the expectation that implementing an unsafe trait even through a derive macro is sound as long as the safety requirements are followed? Not to say I'm not looking forward to derive(unsafe(...)) but I don't think the current state is UB (otherwise I think defining a derive macro for an unsafe trait at all would be an error).
There was a problem hiding this comment.
@Dasaav-dsv I think this is a soundness issue the same way implementing FromStatic using a macro would be. I'd rather just add the discriminator gap validation and laugh at people fucking up their enum in deranged ways
vswarte
changed the title
vswarte
changed the title
vswarte
force-pushed
the
feat/er-steppers
branch
from
227c199 to
24e5a6d
Compare
4 participants
No description provided.