Skip to content

Add no_std support for crates using compiler versions >1.36 #46

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ language: rust
rust:
- stable
- nightly
- 1.22.0 # project wide min version
- 1.36.0 # project wide min version
Copy link
Member

Choose a reason for hiding this comment

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

I'm seeing 1.29 on the main rust-bitcoin repository.

Copy link
Author

Choose a reason for hiding this comment

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

The PR description explains why i chose 1.36. I'm afraid this solution isn't supported on earlier versions.

Choose a reason for hiding this comment

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

There are workarounds for conditional compilation against the version of rustc (rustversion, rustc-version-rs) perhaps it is worth considering if backwards compatibility is necessary?

Copy link
Member

@clarkmoody clarkmoody Oct 19, 2020

Choose a reason for hiding this comment

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

Thanks for those links @gregdhill, but I think one of the strengths of this library is having zero dependencies.

It might be better to maintain a fork of the repo with this work and merge once the rust-bitcoin project moves past Rust 1.36.

Copy link
Contributor

@sgeisler sgeisler Dec 12, 2020

Choose a reason for hiding this comment

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

While raising the general msrv to 1.36.0 is a non-starter I don't think this is necessary here. It appears the incompatibility comes from using the alloc crate which only happens in no-std mode. That means we should still test the against rust 1.22.0 in std mode but against 1.36.0 in no_std mode. I think that would be a reasonable compromise. rust-bitcoin can still depend on the std version compatible with 1.22.0 and e.g. embedded devs that have to use more recent versions anyway can use no_std.

That being said, I don't need it currently, so won't put work in beyond reviewing/approving. What do you think @clarkmoody, would this be acceptable? Also: please don't merge in master into your branch, rather rebase.

Copy link
Member

Choose a reason for hiding this comment

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

@sgeisler That proposal sounds good. Would this be reflected in an update to .travis.yml?

Copy link
Contributor

Choose a reason for hiding this comment

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

Depends on if you want to run tests in no-std mode. If there was a feature that controlled std-availability, lime most crates do it, we could just add a dimension to our test matrix afaik to test everything in no-std mode too for stable and nightly (just not 1.22/1.29).

cache: cargo

script:
- cargo build --verbose --no-default-features --features strict
- cargo build --verbose --features strict
- cargo test --verbose --no-default-features --features strict
- cargo test --verbose --features strict

jobs:
Expand Down
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ categories = ["encoding"]
license = "MIT"

[features]
default = ["std"]
std = []
# Only for CI to make all warnings errors, do not activate otherwise (may break forward compatibility)
strict = []

Expand Down
83 changes: 55 additions & 28 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,23 @@
//!
//! The original description in [BIP-0173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki) has more details.
//!
//! # Examples
//!
//! ```
//! use bech32::{self, FromBase32, ToBase32};
//!
//! let encoded = bech32::encode("bech32", vec![0x00, 0x01, 0x02].to_base32()).unwrap();
//! assert_eq!(encoded, "bech321qqqsyrhqy2a".to_string());
//!
//! let (hrp, data) = bech32::decode(&encoded).unwrap();
//! assert_eq!(hrp, "bech32");
//! assert_eq!(Vec::<u8>::from_base32(&data).unwrap(), vec![0x00, 0x01, 0x02]);
//! ```
//!

#![cfg_attr(
feature = "std",
doc = "
# Examples

```
use bech32::{self, FromBase32, ToBase32};

let encoded = bech32::encode(\"bech32\", vec![0x00, 0x01, 0x02].to_base32()).unwrap();
assert_eq!(encoded, \"bech321qqqsyrhqy2a\".to_string());

let (hrp, data) = bech32::decode(&encoded).unwrap();
assert_eq!(hrp, \"bech32\");
assert_eq!(Vec::<u8>::from_base32(&data).unwrap(), vec![0x00, 0x01, 0x02]);
```
"
)]
// Allow trait objects without dyn on nightly and make 1.22 ignore the unknown lint
#![allow(unknown_lints)]
#![allow(bare_trait_objects)]
Expand All @@ -50,13 +53,22 @@
#![deny(non_snake_case)]
#![deny(unused_mut)]
#![cfg_attr(feature = "strict", deny(warnings))]
#![cfg_attr(not(feature = "std"), no_std)]

use std::borrow::Cow;
use std::{error, fmt};
#[cfg(not(feature = "std"))]
#[allow(unused_imports)]
#[macro_use]
extern crate alloc as std;

// Ideally we'd write
// use std::prelude::v1::*;
// but that is an unstable feature at the time of writing
// https://github.com/rust-lang/rust/issues/58935
#[cfg(not(feature = "std"))]
use std::{string::String, vec::Vec};

// AsciiExt is needed for Rust 1.14 but not for newer versions
#[allow(unused_imports, deprecated)]
use std::ascii::AsciiExt;
use std::borrow::Cow;
use std::fmt;

/// Integer in the range `0..32`
#[derive(PartialEq, Eq, Debug, Copy, Clone, Default, PartialOrd, Ord, Hash)]
Expand Down Expand Up @@ -160,7 +172,7 @@ impl<'a> Bech32Writer<'a> {
/// Write out the checksum at the end. If this method isn't called this will happen on drop.
pub fn finalize(mut self) -> fmt::Result {
self.inner_finalize()?;
std::mem::forget(self);
core::mem::forget(self);
Ok(())
}

Expand Down Expand Up @@ -585,7 +597,8 @@ impl fmt::Display for Error {
}
}

impl error::Error for Error {
#[cfg(feature = "std")]
impl std::error::Error for Error {
fn description(&self) -> &str {
match *self {
Error::MissingSeparator => "missing human-readable separator",
Expand All @@ -609,13 +622,24 @@ impl error::Error for Error {
/// Function will panic if attempting to convert `from` or `to` a bit size that
/// is 0 or larger than 8 bits.
///
/// # Examples
///
/// ```rust
/// use bech32::convert_bits;
/// let base5 = convert_bits(&[0xff], 8, 5, true);
/// assert_eq!(base5.unwrap(), vec![0x1f, 0x1c]);
/// ```
#[cfg_attr(
feature = "std",
doc = "
# Examples

```rust
# #![cfg_attr(not(feature = \"std\"), no_std)]
# #[cfg(not(feature = \"std\"))]
# #[macro_use]
# extern crate alloc as std;
# fn main() {
use bech32::convert_bits;
let base5 = convert_bits(&[0xff], 8, 5, true);
assert_eq!(base5.unwrap(), vec![0x1f, 0x1c]);
# }
```
"
)]
pub fn convert_bits<T>(data: &[T], from: u32, to: u32, pad: bool) -> Result<Vec<u8>, Error>
where
T: Into<u8> + Copy,
Expand Down Expand Up @@ -713,6 +737,7 @@ mod tests {
let (s, expected_error) = p;
let dec_result = decode(s);
if dec_result.is_ok() {
#[cfg(feature = "std")]
println!("{:?}", dec_result.unwrap());
panic!("Should be invalid: {:?}", s);
}
Expand Down Expand Up @@ -767,6 +792,8 @@ mod tests {
}
}

// unfortunately this test uses panic features which are unavailable in `core`
#[cfg(feature = "std")]
#[test]
fn convert_bits_invalid_bit_size() {
use std::panic::{catch_unwind, set_hook, take_hook};
Expand Down