Our `Cursor` is a replacement of sorts for the `std::io::Cursor`. Users
of the `Cursor` are likely to expect the same API so to get a reference
and/or mutable reference users will likely reach for `get_ref` and
`get_mut`. We should provide these.
Deprecate `inner` since it is the same as `get_ref` but less
idiomatically named (should have been `as_inner`).
Add `get_ref` and `get_mut` methods to the `Cursor` type.
There is a range of different wordings used in the docs of constructor
type functions.
Change all to start with `Constructs a new` or `Constructs an empty`.
In functions that act like constructors there is a mixture of the usage
of `creates` and `constructs`.
Replace all occurrences of `creates` with `constructs` in the first line
of docs of constructor like functions.
This has been fixed and we use nightly to lint so we have access to the
merged fix.
Removing the attribute uncovers a bunch of real lint warnings, fix
them while we are at it.
Examples in documentation are not linted in the same way as other code,
but should still contain correctly written code.
Throughout all of the crates except internals (another commit) unused
variables have been prefixed with `_`, unused imports have been removed,
and a warn attribute added to all of the `lib.rs` files.
4ead0adcb5 Add blanket impl of io traits for `&mut T` (Martin Habovstiak)
Pull request description:
The impl wasn't previously available because we thought we can do a blanket impl for `std` traits. We've removed the `std` blanket impl when we realized it's broken but forgot to add the `&mut T` impls. This adds them.
Note: this is in part an experiment to see if this is API breaking. I suspect it might be.
ACKs for top commit:
apoelstra:
ACK 4ead0adcb5 successfully ran local tests
tcharding:
ACK 4ead0adcb5
Tree-SHA512: 1e4411fdf019f3793e6366eda7e8b46246e3263bd7e41802877c20cc5e8ea451a79d89b4c59204ea9c1eb590054975de52a791a9d17a1d180a5cfac316efa959
ad34a98c61 Refactor Rust version checking (Martin Habovstiak)
7d5ce89dad Fix type ambiguity in IO tests (Martin Habovstiak)
Pull request description:
Conditional compilation depending on Rust version using `cfg` had the disadvantage that we had to write the same code multiple times, compile it multiple times, execute it multiple times, update it multiple times... Apart from obvious maintenance issues the build script wasn't generating the list of allowed `cfg`s so those had to be maintained manually in `Cargo.toml`. This was fixable by printing an appropriate line but it's best to do it together with the other changes.
Because we cannot export `cfg` flags from a crate to different crates we take a completely different approach: we define a macro called `rust_version` that takes a very naturally looking condition such as `if >= 1.70 {}`. This macro is auto-generated so that it produces different results based on the compiler version - it either expands to first block or the second block (after `else`).
This way, the other crates can simply call the macro when needed.
Unfortunately some minimal maintenance is still needed: to update the max version number when a newer version is used. (Note that code will still work with higher versions, it only limits which conditions can be used in downstream code.) This can be automated with the pin update script or we could just put the pin file into the `internals` directory and read the value from there. Not automating isn't terrible either since anyone adding a cfg with higher version will see a nice error about unknown version of Rust and can update it manually.
Because this changes syntax to a more naturally looking version number, as a side effect the `cond_const` macro could be also updated to use the new macro under the hood, providing much nicer experience - it is no longer needed to provide human-readable version of the version string to put in the note about `const`ness requiring a newer version. As such the note is now always there using a single source of truth.
It's also a great moment to introduce this change right now since there's currently no conditional compilation used in `bitcoin` crate making the changes minimal. However it is not yet added to `bitcoin-io` since `bitcoin-io` is not depending on `internals`. It might be a reason to start depending on it but that's for later discussion.
ACKs for top commit:
apoelstra:
ACK ad34a98c61 successfully ran local tests
tcharding:
ACK ad34a98c61
Tree-SHA512: 7a82017fc51ba1b473ca638c7bdbf5c893da0a78c70ea8f1a0241049e7874592fad328abd60b3e09a00439b771e7cfc5ebad5e874314d15a48271ec6cb2d7bb7
The impl wasn't previously available because we thought we can do a
blanket impl for `std` traits. We've removed the `std` blanket impl when
we realized it's broken but forgot to add the `&mut T` impls. This adds
them.
Without the ability to seek, the `Cursor` type is only useful for
wrapping owned buffers. In `std` the equivalent type also allows
seeking. We currently do not have the `Seek` trait so this simply adds a
method to set the position. Further, this adds a method to access inner
type so seeking from the end can be implemented (to compute the position
relative to the end).
Closes#3174
This adds a reverse of the previous commit however it does not add
convenience methods since user should generally prefer the macro
instead. I had a clever idea to add a convenience conversion trait but
that is blocked by Rust stabilizing overlapping *marker* trait impls.
When we've removed the blanket impl we forgot to replace it with a
bridge that would make the usage with `std` types easy. This change
implements the missing bridge forwarding all the methods and traits.
051c358bcb Remove deprecated legacy numeric methods (Divyansh Gupta)
Pull request description:
As `rustc 1.79.0-nightly (9d79cd5f7 2024-04-05)` is released which solves the issue mentioned , but the release has deperacted legacy numeric methods.
Thus replaced `u16::max_value()` etc with `u32::MAX` & `core::u16` to directly `u16`.
fix#2639
ACKs for top commit:
tcharding:
ACK 051c358bcb
apoelstra:
ACK 051c358bcb thanks! I will remove an equivalent commit from my #2669
Tree-SHA512: c08c856f7f3b281417c29283351eac5e0f75cc1c8d23d9aae58d969219a327b2337fe57932053e53773ebb9dbec04254f90149266b6639a66c5c09f2ad1675ef
As `rustc 1.79.0-nightly (9d79cd5f7 2024-04-05)` is released which solves the issue mentioned , but the release has deperacted legacy numeric methods.
Thus replace `u16::max_value()` etc with `u32::MAX` & `core::u16` to directly `u16`.
fix#2639
This lint triggers when parsing a reference to a large struct as a
generic argument, which is wrong.
Allow it crate wide because [subjectively] this lint never warns for
anything useful.
Remove the blanket impls of `Read`, `BufRead`, and `Write`. Replace them
with a set of sane impls.
Note, we add code to the `impl_write` macro to implement both
`crate::Write` and `std::io::Write` when "std" feature is
enabled.
Fix: #2432
Currently `Take::read_to_end` is private forcing users to use our
"custom" `read_to_limit`, for seasoned Rust hackers
`foo.take(16).read_to_end(buf)` make be more unsurprising.
Make `read_to_end` public.
Make the trait level attributes uniform across all released crates in
the repo. Excludes things that are obviously not needed, eg, bench stuff
if there is not bench code.
- Remove `uninhabited_references` - this is allow by default now.
- Remove `unconditional_recursion` and mark the single false positive we
have with an `allow`.
Note, this does not add `missing_docs` to the `io` crate. There is an
open PR at the moment to add that along with the required docs.
The `std::io::Read` trait includes `read_to_end` but that method
provides a denial of service attack vector since an unbounded reader
will exhaust all system memory.
Add a method to our `Read` trait called `read_to_limit` that does the
same as `std::io::Read::read_to_end` but with memory exhaustion
protection.
Add a `read_to_end` method on our `Take` trait and call through to it
from the new method on our `Read` trait called `read_to_limit`.
Add a `BufRead` trait for types that perform buffered reading.
Implement it for:
- `Take`
- `Cursor`
- `std::io::BufRead` readers
- (in no-std builds) for slice of u8s
Move error code to a private `error` submodule and re-export it at the
crate root.
(Puts private `sealed` module at the bottom of the file i.e., this patch
is strictly a code move but we re-order the code while moving it.)
Its not immediately obvious why we nest the whole `io` code in an `io`
submodule within `lib.rs`. As far as I can tell we can inline it and
re-export from `rust-bitcoin` same as we do for our other dependencies.
This change would effect other users of the crate but since the `io`
crate is unreleased this effects no-one except us.
In preparation for inlining the `io` molule, move the public macros to a
private `macros` module.
Includes removal of the public re-export of `std` as `_std` - flaggin
this because I do not understand why it is here in the first place, we
can use `std::io::Write` in code that is feature gated on "std".
In order to move towards our own I/O traits in the `rust-bitcoin`
ecosystem, we have to slowly replace our use of the `std` and
`core2` traits.
This is the final step in removing the explicit `core2` dependency
for I/O in no-std - replacing the `io::Error` type with our own.
Sadly the `std::io::Error` type requires `std::error::Error` as a
bound on the inner error, which is rather difficult to duplicate in
a way that allows for mapping to `std` and back.
To take a more general approach, we use bound on any `Debug`
instead.
In order to move towards our own I/O traits in the `rust-bitcoin`
ecosystem, we have to slowly replace our use of the `std` and
`core2` traits.
Here we take the second big step, replacing
`{std,core2}::io::Read` with our own `bitcoin_io::io::Read`. We
provide a blanket impl for our trait for all `std::io::Read`, if
the `std` feature is enabled, allowing users who use their own
streams or `std` streams to call `rust-bitcoin` methods directly.
Tobin C. Harding
Jamil Lambert, PhD
Fmt Bot
merge-script
Martin Habovstiak
Divyansh Gupta
apoelstra
Matt Corallo