Skip to content

Conversation

@fjarri
Copy link
Contributor

@fjarri fjarri commented Aug 3, 2023

This helps with deserialization - one doesn't need to create a mutable buffer and use a special purpose deserialize mutating its argument (like serdect does currently), but instead one can just use a standard deserialize with the API compatible with that of #[serde(with)] that returns the Repr object.

+ Copy
+ Clone
+ Sized
+ for<'a> TryFrom<&'a [u8], Error = core::array::TryFromSliceError>;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the error type is particularly relevant here? Do we really need to mandate it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It helps a lot with writing down trait bounds in the user code. See rust-lang/rust#113517 for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To add to that, even with the workaround in the issue above, it does not work when another level is added. That is, if the foo() in the issue is a serde-standard deserialize() function, and I want to make a custom deserializer using serde_with as

impl<'de, const N: usize> serde_with::DeserializeAs<'de, Uint<N>> for UintWrapper<N>
where
    Uint<N>: Encoding,

there is no way to write trait bounds that would tell the compiler that the returned error is Display (so that it could be wrapped in the serde error).

Copy link
Member

Choose a reason for hiding this comment

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

Okay, if there's a legitimate use case I guess we can include it.

It's just a completely useless error that includes no relevant information, so it's easily handled without bounding on it by using .ok() or .map_err(...).

But if it helps with serde, I guess that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is pretty useless, I agree. But if you have a function that is generic on something that's TryFrom<&[u8]> and you want the error to be Display (because it may be useful for some types, so you don't want to discard it), fixing the error in this specific case helps with the bounds. I don't particularly like it either, but unfortunately Rust compiler is currently not smart enough to handle bounds without it. I think it should be pretty harmless, since it's essentially an internal trait that's not supposed to be used outside of the crate.

@tarcieri
Copy link
Member

tarcieri commented Aug 3, 2023

Also note: this is a breaking change, so we can't merge it until the next breaking release.

@tarcieri
Copy link
Member

Bumped version to v0.6.0-pre in #295. Merging

@tarcieri tarcieri merged commit b0a566c into RustCrypto:master Nov 21, 2023
@fjarri fjarri deleted the repr-trait branch December 2, 2023 19:00
@tarcieri tarcieri mentioned this pull request Jan 22, 2025
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