multihash const constructors unusable

[aatifsyed]

Multihash's constructors are const fn, but they can't actually be used as such:

use multihash::Multihash;

const _: Multihash<64> = match Multihash::wrap(
    0x16,
    &[
        0x16, 0x20, 0x64, 0x4b, 0xcc, 0x7e, 0x56, 0x43, 0x73, 0x04, 0x09, 0x99, 0xaa, 0xc8, 0x9e,
        0x76, 0x22, 0xf3, 0xca, 0x71, 0xfb, 0xa1, 0xd9, 0x72, 0xfd, 0x94, 0xa3, 0x1c, 0x3b, 0xfb,
        0xf2, 0x4e, 0x39, 0x38,
    ],
) {
    Ok(o) => o,
    Err(_) => panic!(),
};

Fails to compile:

error[E0493]: destructor of `Result<Multihash<64>, multihash::Error>` cannot be evaluated at compile-time
  --> src/lib.rs:3:32
   |
3  |   const _: Multihash<64> = match Multihash::wrap(
   |  ________________________________^
4  | |     0x16,
5  | |     &[
6  | |         0x16, 0x20, 0x64, 0x4b, 0xcc, 0x7e, 0x56, 0x43, 0x73, 0x04, 0x09, 0x99, 0xaa, 0xc8, 0x9e,
...  |
9  | |     ],
10 | | ) {
   | |_^ the destructor for this type cannot be evaluated in constants
...
13 |   };
   |   - value is dropped here

For more information about this error, try `rustc --explain E0493`.

This is presumably because the top level error type is non-copy/contains an io::Error which cannot be const

rust-multihash/src/error.rs

Lines 42 to 50 in 16e68d8

	#[derive(Debug)]
	enum Kind {
	/// Io error.
	Io(io::Error),
	/// Invalid multihash size.
	InvalidSize(u64),
	/// Invalid varint.
	Varint(decode::Error),
	}

Here is the source for the constructor

rust-multihash/src/multihash.rs

Lines 59 to 75 in 16e68d8

	pub const fn wrap(code: u64, input_digest: &[u8]) -> Result<Self, Error> {
	if input_digest.len() > S {
	return Err(Error::invalid_size(input_digest.len() as _));
	}
	let size = input_digest.len();
	let mut digest = [0; S];
	let mut i = 0;
	while i < size {
	digest[i] = input_digest[i];
	i += 1;
	}
	Ok(Self {
	code,
	size: size as u8,
	digest,
	})
	}

Fix is either:

• drop the illusion of const from the wrap constructor. This is a breaking change
• have wrap return a new, const-compatible InvalidSize error in its constructor. This is also a breaking change. This is my preference, and I'm happy to create a PR for such a change.

[aatifsyed]

forest have interest in const-construction of CIDs.
See also multiformats/rust-cid#138

[thomaseizinger]

I'd really like to avoid making a breaking change. Can we instead introduce a new function and deprecate wrap? Such a new function can then have a different signature and introduce a new const compatible error type.

I'd also like to see a test for this :)

[vmx]

• drop the illusion of const from the wrap constructor. This is a breaking change

Why is that a breaking change. Given that it's not really const, wouldn't removing the const just be a bugfix? It shouldn't break any code as code using it as const wouldn't compile. What am I missing?

[aatifsyed]

Why is that a breaking change. Given that it's not really const, wouldn't removing the const just be a bugfix? It shouldn't break any code as code using it as const wouldn't compile. What am I missing?

const calling the function would compile, but not accessing the result, so it's a breaking change

[aatifsyed]

Closing - the constructors aren't unusable. Maybe there should be some user-facing documentation for the const case so this issue isn't created again?

See #331 (comment) for closing comment.