-
-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace type aliases with newtypes #76
Comments
Thank you for your issue! This is definitely planned, as I'm currently working on implementing PER and constraints, and constraints I want to eventually move the constraints impl to being completely
I do want to be clear however that this is acceptable. This crate is still very firmly in
Yes, seperately I'd like to be able to massively revamp documentation, to provide more both informative and normative information on ASN.1, such that rasn's documentation is the primaary if not only source you need to use ASN.1 successfully. |
I think I could start working with this [newtype support] after I finish OER codec, if that is okay. I am still quite new for Rust, so I might need some design help. But the basic idea would be to use Crate level features and with that to change types? For the higher type (Integer) and user facing API, we write our own functions, and then with crate features possibly map to different inner types. |
Right now we're mostly limited by Rust's const capabilities, once we have the ability to use enums it will make the design much more intuitive and straightforward. You can see a simple version of this with the fixed octet string type. |
Decoding functionality for any kind integer type was added in #256 I am trying to implement the encoding part now, but there are some design choices to be made. Currently, We could either remove the whole #[derive(Debug, Clone, Ord, Hash, Eq, PartialEq, PartialOrd)]
#[non_exhaustive]
pub enum Integer {
Primitive(PrimitiveInteger),
Big(BigInt),
} Or maybe something else. @XAMPPRocky do you have any ideas? |
I think the small integer optimisation of using an enum is the way to go forward. |
Instead of directly reexporting types such as
BigInt as Integer
andBytes as OctetString
, I think it makes a lot more sense to newtype these and provide a more stable interface so that the implementations can be changed without affecting the public API. For instance, being able to switch from usingnum-bigint
toibig
for parsing variable-sized integers could provide a significant speed increase, avoiding unnecessary allocations for small integers. However, changing theInteger
alias toIBig
would be a breaking change. Reexporting types also makes the docs quite a bit more confusing in my opinion, as the documentation for the types ends up using a completely, totally different type name, for example when I go to view the docs forOctetString
, I'm met with the following docs:Which is a bit jarring when the type signature is named something completely different IMO
The text was updated successfully, but these errors were encountered: