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

core-foundation-sys: Enable no_std environment #609

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michaelwright235
Copy link
Contributor

Since the purpose of this crate is the bindings to an external library and they're pretty much complete, we can see that the only thing we use in libstd are the pointer types and std::cmp. In fact they're defined in libcore and then exported through libstd. I doubt that we will ever need any libstd only stuff such as heap allocation, vectors, hashmaps, etc. I propose making core-foundation-sys a no_std crate. It maybe useful in some rare situations. For instance, windows-sys crate (Microsoft's official Rust bindings) is no_std and it can be used for writing kernel drivers.

Since the purpose of this crate is the bindings to an external
library and they're pretty much complete, we can see that the only
thing we use in libstd are the pointer types and `std::cmp`.
In fact they're defined in libcore and then exported through libstd.
I doubt that we will ever need any libstd only stuff such as heap
allocation, vectors, hashmaps, etc. I propose making
core-foundation-sys a no_std crate. It maybe useful in some rare
situations. For instance, windows-sys crate (Microsoft's official
Rust bindings) is no_std and it can be used for writing kernel
drivers.
Copy link
Contributor

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

👍

Beware that this bumps MSRV to 1.64.

@jrmuizel
Copy link
Collaborator

jrmuizel commented Jul 4, 2023

I think we should have an actual use case otherwise it doesn't seem worth increasing the MSRV for.

@michaelwright235
Copy link
Contributor Author

Maybe it's acceptable because core-foundation that depends on this crate already has minimum Rust version of 1.64.0 (using of std::ffi::Cstr).

@michaelwright235
Copy link
Contributor Author

Hmm, it looks like this import has been added a couple of years ago, but rust docs says it was added in 1.64.0

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably b009c87) made this pull request unmergeable. Please resolve the merge conflicts.

@mrobinson
Copy link
Member

This was done in #692 in the end.

@waywardmonkeys
Copy link
Collaborator

If we treat this like an issue rather than a PR, there's still 3 things outstanding:

  • std:::cmp::Ordering is used in core-foundation-sys/src/base.rs
  • std::ptr::null() is used in core-foundation-sys/src/bundle.rs
  • no_std isn't enabled specifically.

If @michaelwright235 would like to rebase this to do that, that's great ... or we can open a new PR that does so?

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.

6 participants