Polishing/stabilization roadmap? #60
Comments
Currently we use `Error::InvalidData(u8)` to attempt to cache an invalid value encountered while converting to a `u8`. This is buggy because we cast to the `u8` to stash the value so we loose valuable bits. Implement `TryFrom` by doing: - Add an `Error::Overflow` variant. - Use `Error::Overflow` for conversion _outside_ of `TryFrom` impls - Add a new `TryFromError` and nest it in `Error::TryFrom` - Implement `TryFrom<T> for u5` impls for all standard integer types. Inspired by issue rust-bitcoin#60 and the stdlib macros of the same name.
Currently we use `Error::InvalidData(u8)` to attempt to cache an invalid value encountered while converting to a `u8`. This is buggy because we cast to the `u8` to stash the value so we loose valuable bits. Implement `TryFrom` by doing: - Add an `Error::Overflow` variant. - Use `Error::Overflow` for conversion _outside_ of `TryFrom` impls - Add a new `TryFromError` and nest it in `Error::TryFrom` - Implement `TryFrom<T> for u5` impls for all standard integer types. Inspired by issue rust-bitcoin#60 and the stdlib macros of the same name.
25462c9 Implement TryFrom various integer types (Tobin C. Harding) 7ab12fc Use Error variants locally (Tobin C. Harding) Pull request description: Patch 1 is preparatory clean up. From the commit log of patch 2: Currently we use `Error::InvalidData(u8)` to attempt to cache an invalid value encountered while converting to a `u8`. This is buggy because we cast to the `u8` to stash the value so we loose valuable bits. Implement `TryFrom` by doing: - Add an `Error::Overflow` variant. - Use `Error::Overflow` for conversion _outside_ of `TryFrom` impls - Add a new `TryFromError` and nest it in `Error::TryFrom` - Implement `TryFrom<T> for u5` impls for all standard integer types. Inspired by issue #60 and the stdlib macros of the same name. Please note, there is expected error work to come soon on this crate, can we keep perfect error code out of scope for this PR please (obviously if it has gaping problems then yell at me)? Note that the same derives are used here as for `Error` even though I think they are wrong. Done as part of stabilising (#60). ACKs for top commit: apoelstra: ACK 25462c9 Tree-SHA512: 830c6d0e1a8fa8badc983497b9eb6f0e38f0d81dc4f33dd5cd882d3e34836cfca277b4b1af9d98ea26e8cc0e53df9e708d8bc7d84757104cd05054fcbec169e0
|
Would be great to get your input @Kixunil on how close you think the new code is to being ready for v1.0.0. Current plan is (only from informal discussion between @apoelstra and myself, so still requires input from @clarkmoody):
Seems we are then pretty close to doing a v1.0.0 |
|
Great timing, I'm back! I only took a quick look now and I still have a bunch of things.
|
|
I also want to
Edit also, regarding the |
Indeed, what I meant is the terminology is mostly used in that context. But maybe it's really fine, I just find it a bit more scary than it needs to be.
Screw this name, my mind got instantly triggered... :D
😢 |
|
Kiiiiiiiix! Welcome back!
|
|
As part of the stabalization process would it be useful to hack up code that exercises every error code path and check the output to see if it is correct and useful for debugging? Or is this going too far, is this a v2.0 thing? |
|
@clarkmoody is there anything you would like to see done before we release v1.0.0 of this crate? Said another way, do you have any concerns, what so ever, with what we did over the last few months, things you'd like me to look into? Cheers FTR @apoelstra is going to look into the error detection before we consider pushing out the v1.0 |
|
@tcharding I wouldn't do automated string comparison but writing it as a demo to manually check sounds like a good idea.
If the internals of errors are private we don't have to rush this. But I really want it soon. ;) |
Please see #151 Note, I had in mind while doing this that stabalizing this crate is a test run for doing |
|
Closing in favour of #157 |
It'd be great to polish this crate and make it stable with idiomatic API and good documentation. Since this crate appears stale I'm willing to help maintain it to make the progress a bit faster.
I'm still unsure what all those traits do, they seem to be meant as extension traits?
Thinking about ideal API, I identified these basic building blocks:
u5typeu8to stream ofu5and backu8to ASCII and backI think each deserves its own module, maybe even crate. The commonly used things can still be flattened for consumers.
u5 type
I think this is good as is. Could add
impl TryFrom<*> for u5and arithmetic functions for completeness.u8-u5 conversion
The API should enable to perform this conversion without allocation by having wrappers around iterators.
Convenience functions for slices and
Vecs can be added but not sure how important given.iter()and.collect()are easy to call.This should also provide free-standing functions to calculate encoded/decoded lengths.
checksum calculation
The API should be similar to hashers with possible convenience to compute from iterators.
It should also provide a wrapper for the case when checksum is calculated together with conversions.
u8-ASCII conversion
Should be similar to
u8-u5conversion. Might be useful to provide wrappers returning both ASCIIu8andcharto avoid overhead when writing towriter. Also maybe provide a method to constructStringwithout unnecessary UTF-8 check (usingunsafeinternally).Facade
Convenience functions for converting stream of
u8to strings (with/without checksum) and back should be provided.Also for consideration: exploit the invariant of
u5to skip bounds checks when converting tochar.The text was updated successfully, but these errors were encountered: