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

Add support for no_std compilation (updated) #177

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pitdicker
Copy link

@pitdicker pitdicker commented Feb 26, 2024

See discussion in #74.

This is useful to fuzz crates in their no_std configuration, which may utilize different code paths in such a crate.

I started by rebasing #74 but made a number of changes to split it in an std and alloc feature.

Fixes #38.

@fitzgen
Copy link
Member

fitzgen commented Feb 26, 2024

This was the conclusion of #74: #74 (comment)

@pitdicker
Copy link
Author

What do you think of the use case mentioned above?

This is useful to fuzz crates in their no_std configuration, which may utilize different code paths in such a crate.

@pitdicker
Copy link
Author

I am not sure how to replicate the warnings in the "Run int_in_range fuzz target" CI job. Hopefully this fixes it.

@nagisa
Copy link
Member

nagisa commented Jul 4, 2024

@fitzgen counter question to the implementation constraints concern: is it really as bad as your comment makes it seem to restrict ourselves to libcore in the few circumstances where doing so would be necessary? I'm asking because arbitrary is largely a collection of impl Arbitrary for .... Since most of the implementing types hail from libstd, those will be able to use std in their implementation. Outside of the Arbitrary trait and the few impls for the basic libcore types, everything else (including Unstructured) can be made libstd-only and considered for no_std compatibility on a case-by-case basis. Wouldn't that be good enough?

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

I guess I won't be the one holding things back if everyone else wants this.

@@ -14,6 +14,7 @@
//! [`Arbitrary`](./trait.Arbitrary.html) trait's documentation for details on
//! automatically deriving, implementing, and/or using the trait.

#![cfg_attr(not(any(feature = "std", test)), no_std)]
Copy link
Member

Choose a reason for hiding this comment

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

To avoid dealing with different preludes that are vs aren't implicitly in scope depending on the features enabled, we should make this crate always be no_std. And then because the std feature implies the alloc feature, we can layer in imports like

#[cfg(feature = "alloc")]
use alloc::boxed::Box;

and we don't have to use any complex logic like all(feature = "alloc", not(feature = "std")) kinds of things.

My experience has also been that tools like rust-analyzer handle this pattern for no_std crates better as well.

@refcell
Copy link

refcell commented Sep 4, 2024

Landing this would be highly appreciated.

Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

in favor of landing once it's fixed

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.

#![no_std] Support
5 participants