tkaitchuck / constrandom Goto Github PK

The dependency on once_cell is pulling in std... which makes build fail on no-std.

Probably needs default-features = false.

Hi

The macro crate fails on nightly because it exposes a function (marked as public) which isn't annotated with any one of the #[proc_macro*] annotations.

Running 0.1.13 on 1.53.0-nightly (2021-04-18)

const-random currently emits signed integers in lowercase hex representation:

but this leads to an error if the random number is negative.

error: literal out of range for i32
  --> src/main.rs:14:13
   |
14 |     const RAND: i32 = const_random!(i32);
   |                       ^^^^^^^^^^^^^^^^^^
   |
   = note: `#[deny(overflowing_literals)]` on by default
   = note: the literal `0xda324e7e` (decimal `3660729982`) does not fit into the type `i32` and will become `-634237314i32`
   = help: consider using `u32` instead

Deterministic repro:

pub const RAND: i32 = 0xda324e7e;

fn main() {}

cargo test on the current master branch passes but emits the following warnings. It would be nice to either suppress these with an attribute or just fix the case of these const names to what rustc wants.

warning: constant `value1` should have an upper case name
 --> tests/tests.rs:5:11
  |
5 |     const value1: u32 = const_random!(u32);
  |           ^^^^^^ help: convert the identifier to upper case: `VALUE1`
  |
  = note: `#[warn(non_upper_case_globals)]` on by default

warning: constant `value2` should have an upper case name
 --> tests/tests.rs:6:11
  |
6 |     const value2: u32 = const_random!(u32);
  |           ^^^^^^ help: convert the identifier to upper case: `VALUE2`

warning: constant `value1` should have an upper case name
  --> tests/tests.rs:14:11
   |
14 |     const value1: i64 = const_random!(i64);
   |           ^^^^^^ help: convert the identifier to upper case: `VALUE1`

warning: constant `value2` should have an upper case name
  --> tests/tests.rs:15:11
   |
15 |     const value2: i64 = const_random!(i64);
   |           ^^^^^^ help: convert the identifier to upper case: `VALUE2`

warning: constant `value1` should have an upper case name
  --> tests/tests.rs:24:11
   |
24 |     const value1: u128 = const_random!(u128);
   |           ^^^^^^ help: convert the identifier to upper case: `VALUE1`

warning: constant `value2` should have an upper case name
  --> tests/tests.rs:25:11
   |
25 |     const value2: u128 = const_random!(u128);
   |           ^^^^^^ help: convert the identifier to upper case: `VALUE2``

Compiling bevy which uses your crate, I run into an error with stable Rust (1.46.0) since the release of 0.1.10 on crates.io:

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> /home/akien/.cargo/registry/src/github.com-1ecc6299db9ec823/const-random-macro-0.1.10/src/lib.rs:1:1
  |
1 | #![feature(proc_macro_quote)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Pinning to 0.1.8 fixes it.

Edit: I know that using the nightly toolchain would fix it, but I'm reporting this in case you didn't intend to make const-random-macro a nightly-only crate.

Hi,

I tried using your crate like in the example in the documentation

const MY_RANDOM_BYTES: [u8; 32] = const_random!([u8; 32]);

That will panic with a message "invalid type". The underlying problem is in the parsing code in this line

byte_array: &str if byte_array.starts_with("[u8 ; ") && byte_array.ends_with(']')=> ...

Here you are sensitive to spaces and the formatting is problematic if someone has rustfmt enabled like I do. I could submit a PR to fix this, but rather than just patching that particular line I would want to use the syn crate to parse the tokens correctly. I don't know if you would want this dependency though, so I thought I'd ask. Your code works when I use

#[rustfmt::skip]
const MY_RANDOM_BYTES: [u8; 32] = const_random!([u8 ; 32]);

which seems a bit silly :)

Is it possible to build this crate with glibc < 2.29 (without getrandom())?

Commit 4ef07e25a8b08153d527db84dd7f02d398ea1040 broke no_std support. This in turn broke my no_std usage of the hashbrown crate because I use ahash with the compile-time-rng feature which brings in const-random. (ahash pins const-random to 0.1.12 but because the maintainers yanked that version it appears the dep floats to the latest rev.)

It looks like the integer literals emitted by const_random are plain integers without a type suffix. The below program compiles and prints i32, which is probably not what would be expected.

use const_random::const_random;

fn f<T>(_: T) {
    println!("{}", std::any::type_name::<T>());
}

fn main() {
    f(const_random!(u8));
}

The const_random invocation in this code should expand to 0xNNu8 instead.

Code coverage both with Tarpualin (https://github.com/xd009642/tarpaulin) and intellij's grcov based instrumentation fails with

error: could not compile const-random due to 2 previous errors
Jun 15 14:43:29.604 ERROR cargo_tarpaulin: Failed to compile tests!
error: ../target/debug/deps/libconst_random_macro-d64f87d46f5009b3.so: undefined symbol: __getrandom_custom
--> /../.cargo/registry/src/github.com-1ecc6299db9ec823/const-random-0.1.15/src/lib.rs:16:9
|
16 | pub use const_random_macro::const_random;
| ^^^^^^^^^^^^^^^^^^

Error: "Failed to compile tests!\nerror: /../git-test/di-apps/target/debug/deps/libconst_random_macro-d64f87d46f5009b3.so: undefined symbol: __getrandom_custom\n --> /../.cargo/registry/src/github.com-1ecc6299db9ec823/const-random-0.1.15/src/lib.rs:16:9\n |\n16 | pub use const_random_macro::const_random;\n | ^^^^^^^^^^^^^^^^^^\n\n"

this is less of an issue and more of a question out of curiosity, but is there a reason that there aren't any float types supported?

It looks like the tests in tests.rs are intentionally flaky. Any of the three below assertions could fail nondeterministically. Usually this is frowned upon. In this case the probabilities are so low that it's unlikely to ever be observed, but it would be helpful to acknowledge this with a comment that documents the probability of failure.

It would be very awesome if you could include LICENSE-MIT and LICENSE-APACHE files and make a new release.

Having text of full license is mandatory by the license terms.

Thanks!

It looks like the problems with compiling const-random that I'm seeing in Fedora Linux were fixed by this PR:
#33

But this fix was never released / published to crates.io. There have been new releases of the const-random crate, but no new release of the const-random-macro.

The README says:

Note: As of Rust 1.45 this crate is superseded by native support for #[proc_macro] in expression position. Only consider using this crate if you care about supporting compilers between 1.31 and 1.45.

Since Rust 1.45 came out two years ago, it might be time to consider removing this dependency.

I noticed this because I wanted to use ASan on my code, and for some reason that causes the build of proc-macro-hack to fail.

const_random!(usize) and const_random!(isize) are not currently supported. This is likely because the proc macro runs on the host and may have a different usize size than the target.

One approach that would work for implementing these is to expand to an as cast:

const_random!(usize)

// expands to:
const_random!(u128) as usize

which will truncate high bits and produce a correctly usize-sized random value.