bit2swaz commented on PR #10324: URL: https://github.com/apache/arrow-rs/pull/10324#issuecomment-4989344683
agreed, id just force validation. this PR already is that so its ready if youre happy with it :) the thing with the other two is that `TypeId` needs `E: 'static` and only fast-paths the one concrete `GeneralPurpose`, every other honest engine (`GeneralPurposeNoPad`, custom alphabets) still hits `try_new`, so its `unsafe` + a footgun to speed up exactly one type. doesnt seem worth it `SafeBase64` is the correct zero-cost shape if we ever want it, the caller proves the invariant via `unsafe impl` and a dishonest engine just wont compile. but its a new public `unsafe trait` for an edge case with no known downstream user (like you said you couldnt find one in datafusion). easy to add later if someone shows up needing it. dont think theres a need to ship it speculatively dropping the generic to `GeneralPurpose`-only works too but thats a bigger breaking change and kills the `Engine` abstraction entirely and feels orthogonal to the soundness fix. happy to do it in a separate PR if youd prefer that direction tho -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
