Skip to content
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

Fix no-std support. #844

Merged
merged 4 commits into from Jul 26, 2019
Merged

Fix no-std support. #844

merged 4 commits into from Jul 26, 2019

Conversation

rubdos
Copy link
Contributor

@rubdos rubdos commented Jul 20, 2019

rand_chacha used c2-chacha without trickling through features.
This disables default options in rand_chacha, enabling simd and std
when the option is selected at the root.

@rubdos
Copy link
Contributor Author

rubdos commented Jul 20, 2019

Maybe, as a side note: this issue was not caught by CI. no_std is tested by compiling with --no-default-features, which disables std only in the root crate. Maybe an extra test in CI would be useful?

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

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

True, we should not force std here.

What additional test do you propose? Probably we should have a bare-metal target included here, e.g. thumbv6m-none-eabi.

Cargo.toml Outdated
alloc = ["rand_core/alloc"] # enables Vec and Box support (without std)
# re-export optional WASM dependencies to avoid breakage:
wasm-bindgen = ["getrandom_package/wasm-bindgen"]
stdweb = ["getrandom_package/stdweb"]
getrandom = ["getrandom_package", "rand_core/getrandom"]

# Configuration:
simd_support = ["packed_simd"] # enables SIMD support
simd_support = ["packed_simd", "rand_chacha/simd"] # enables SIMD support
Copy link
Member

Choose a reason for hiding this comment

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

These two have very little to do with one another — simd_support is about generating SIMD types (using unstable machinery from nightly), while rand_chacha/simd is about enabling SIMD optimisations (without requirements on nightly or packed_simd).

I think rand_chacha should not expose this feature and always require simd on c2-chacha (and keep a dummy simd feature for compatibility). (Thus we should bump the version of rand_chacha first and then depend on it here.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to bump rand_chacha in this PR then, or should I rebase on something you folks do?

@rubdos
Copy link
Contributor Author

rubdos commented Jul 20, 2019

What additional test do you propose? Probably we should have a bare-metal target included here, e.g. thumbv6m-none-eabi.

Exactly what I'd propose: build for some thumb target. Would you want that in this PR?

EDIT: included in 4a13441.

`rand_chacha` used `c2-chacha` without trickling through features.
This disables default options in `rand_chacha`, enabling simd and std
when the option is selected at the root.
@dhardy
Copy link
Member

dhardy commented Jul 20, 2019

Thanks. Looks good but depends on #845.

@dhardy
Copy link
Member

dhardy commented Jul 20, 2019

BTW please add a note to the changelog, either under [Unreleased] or if you'd like to see this released soon, bump the version (also in Cargo.toml).

@rubdos
Copy link
Contributor Author

rubdos commented Jul 23, 2019

BTW please add a note to the changelog, either under [Unreleased] or if you'd like to see this released soon, bump the version (also in Cargo.toml).

Done! Let me know if you want something more here.

@dhardy
Copy link
Member

dhardy commented Jul 23, 2019

No, just waiting for review on #845.

@rubdos
Copy link
Contributor Author

rubdos commented Jul 23, 2019

No, just waiting for review on #845.

Right, sorry. Keeping track of many issues and merge requests lately, while we're breaking heat records without A/C. Thanks again!

@dhardy
Copy link
Member

dhardy commented Jul 23, 2019

Yes, certainly is hot!

@dhardy dhardy merged commit 40dbb54 into rust-random:master Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants