search

RustCrypto / utils

Utility crates used in RustCrypto
427 stars 123 forks source link

feat(opaque-debug): support generics #1053

Closed sinui0 closed 5 months ago

sinui0 commented 5 months ago

This PR adds support for opaque debug impls on structs with generics. It does not support structs with trait bounds as working with them using macro_rules! is not possible as far as I can tell.

It also wasn't obvious how to avoid exporting the format_params macro which is required to build the format template without a trailing comma eg Foo<T, U, V,> { ... }.

newpavlov commented 5 months ago

This looks fine on the first glance, but note that opaque-debug is essentially deprecated and not used in modern versions of RustCrypto crates. If this change is not needed for one of your projects, I think we can leave everything as-is.

sinui0 commented 5 months ago

Thanks for taking a look. I wasn't aware that its heading towards deprecation. Would you mind pointing me to any discussion around that, or just dropping the gist of why here? I do use this in some projects but would be happy to close this and use an alternative.

newpavlov commented 5 months ago

We had some complaints about number of dependencies which our crates pull (despite that the pulled crates are part of the same org...) and we decided that pulling a whole crate to save in most cases just 4 lines of code is usually not worth the trouble.

I do use this in some projects

In this case I think we can cut a new release.

sinui0 commented 5 months ago

I see, thanks!

Re-looking at this PR, in particular the use of core::any::type_name I've just realized that in some cases it will write out the entire path of a type parameter. This could introduce a lot of noise. Instead I could trim to just the last element in the path based on the :: separator. But I can also see the case for wanting the full path. Thoughts?

newpavlov commented 5 months ago

I could trim to just the last element in the path based on the :: separator

But generic parameter may contain its own generic parameters, which will not be trimmed with a naive approach. :) I am fine with either option with a slight preference of using output core::any::type_name as-is, because it's a more straightforward and simple solution.

sinui0 commented 5 months ago

Hmm right, good call.

it's a more straightforward and simple solution.

Agreed. Best to avoid adding in String allocations and processing.