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

Remove std feature #459

Merged
merged 1 commit into from
Dec 8, 2022
Merged

Remove std feature #459

merged 1 commit into from
Dec 8, 2022

Conversation

tarcieri
Copy link
Contributor

@tarcieri tarcieri commented Dec 8, 2022

All of the existing usages of std can be replaced with alloc.

They are legacy usages from before when liballoc was stabilized.

All of the existing usages of `std` can be replaced with `alloc`.

They are legacy usages from before when liballoc was stabilized.
@rozbb
Copy link
Contributor

rozbb commented Dec 8, 2022

This is odd. The benches now build, but cargo bench doesn't perform any benchmarks anymore 🤔

Cargo.toml Show resolved Hide resolved

[[bench]]
name = "dalek_benchmarks"
harness = false
required-features = ["rand_core"]
Copy link
Contributor Author

@tarcieri tarcieri Dec 8, 2022

Choose a reason for hiding this comment

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

@rozbb this is why it doesn't run benchmarks by default. You have to run the with:

$ cargo bench --features rand_core

(sidebar: it might be nice to make the benchmarks deterministic so it doesn't need rand_core)

It's unrelated to this PR but I was getting a build failure without these changes.

Seems related to #447?

Copy link
Contributor

Choose a reason for hiding this comment

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

#447 doesn't cause this in itself. cargo bench runs benches currently on release/4.0. Hmm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously std was activated by default, and std activated rand_core/std.

The benchmarks clearly do require rand_core, it's just it used to be "accidentally" enabled by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok. Then I'll just make a note in the README for now

@rozbb rozbb merged commit 1013560 into release/4.0 Dec 8, 2022
@tarcieri tarcieri deleted the remove-std-feature branch December 8, 2022 20:12
@pinkforest pinkforest mentioned this pull request Dec 9, 2022
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.

2 participants