Skip to content

Conversation

@bushrat011899
Copy link
Contributor

@bushrat011899 bushrat011899 commented Feb 27, 2025

Objective

  • Add support for no_std and no_alloc
  • Allow using internment on targets such as x86_64-unknown-none and thumbv6m-none-eabi

Solution

  • Added #[no_std] unconditionally 1
  • Re-introduced std and alloc when features of the same names are enabled.
  • Added std and alloc as required features of all features which fail to compile without them.
  • Changed imports to pull from core and/or alloc as required.
  • Added spin feature to provide a spin based alternative to std::sync::Mutex 2
  • Added portable-atomic and critical-section features to allow portable-atomic to provide compatibility to atomically challenged platforms such as thumbv6m-none-eabi 3
  • Switched to resolver = "2" to ensure dev-dependencies don't enable std on serde 4

Testing

  • cargo check --no-default-features --target x86_64-unknown-none --features serde,arena,spin,intern,alloc,deepsize
  • cargo check --no-default-features --target thumbv6m-none-eabi --features serde,arena,spin,intern,portable-atomic,critical-section

Release Notes

internment can now be used on no_std platforms. Simply disable default features, and enable any optional extras which themselves don't enable the new std feature:

internment = { version = "X.Y.Z", default-features = false, features = [
    "spin", # Required for `no_std` targets
    "serde",
    "intern",
    "deepsize",
    "arena",
# Incompatible with `no_std`
#  "arc",
#  "tinyset",
]}

If you are compiling for a target without full support for Rust atomics, you may need to enable the portable-atomic and critical-section features which aid in platform compatibility.

internment = { version = "X.Y.Z", default-features = false, features = [
    "spin", # Required for `no_std` targets
    "portable-atomic", # Required for atomically challenged platforms
    "critical-section", # Often required for atomically challenged platforms
    "serde",
    "intern",
    "arena",
# Incompatible with `no_std` on atomically challenged platforms
#  "deepsize",
#  "arc",
#  "tinyset",
]}

Notes

  • Based on cargo msrv, I believe these changes do not increase the MSRV of this crate.
  • This PR is motivated by the possibility of including this crate in Bevy. Our current implementation has broad support for no_std platforms, so this is a prerequisite for our usage of internment. I also believe adding this support is generally beneficial for the broader Rust community without substantially increasing maintenance burden on crate maintainers.
  • This is my first contribution to this project. Please let me know if there's anything I can do to assist with reviewing this PR.
  • It may be prudent to add x86_64-unknown-none and thumbv6m-none-eabi targets to your CI tests with the relevant features enabled/disabled to ensure compatibility does not regress. I am not familiar with this particular CI system so I'd rather leave that out of this PR, but I am happy to investigate adding it if that's desirable!

Footnotes

  1. Adding #[no_std] using something like #![cfg_attr(not(feature = "std"), no_std)] will cause the implicit prelude to change based on the features used, which can make development harder than it needs to be.

  2. If the library is compiled without std or spin a compile_error! is thrown indicating the user must enable one of those features. This allows the spin dependency to only come into the Cargo.lock if it is explicitly enabled.

  3. Keeping this as an optional feature ensures it is only included in Cargo.lock when explicitly enabled.

  4. Without resolver = "2", feature unification causes the features enabled by dev-dependencies to be included in normal dependencies, which makes no_std support impossible without removing examples, benches, and/or tests.

@bushrat011899
Copy link
Contributor Author

I believe the CI failure is due to the removal of the -Zprofile unstable rustc option. I haven't personally used it, but a quick search shows that apparently it was replaced with -C instrument-coverage. I could look into fixing the CI task if you'd like?

@droundy
Copy link
Owner

droundy commented Mar 2, 2025

I think I've fixed the CI issue, and merging with main should fix the failure.

Could you try to add your checks to rust.yaml? You should be able to add them to the existing Check by adding more - run: cargo check --whatever lines.

I have a hokey copy of your branch that started doing this before I figured out I needed a larger overhaul of the coverage check (and then ran out of time this morning before the family woke up). You could glance at that for inspiration.

@bushrat011899
Copy link
Contributor Author

Thanks for working on this! I've merged in those changes you mentioned, and I believe I have a pair of no_std checks added based on the no_std branch that should suffice.

Comment on lines +14 to +15
#[cfg(test)]
use std::println;
Copy link
Owner

Choose a reason for hiding this comment

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

question: Does this mean that std is required for testing?

Would it be possible to make at least some of the tests work for spin? Or am I wrong and you can run the tests without the std feature (but presumably with the std library used for testing)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately the way this library is setup right now it'd be a big change to run the tests on these platforms. The issue is once you compile tests, you bring in all of the dev-dependencies too, most of which aren't no_std friendly. And there is currently no way to mark a dev-dependency as optional. This also applies to examples.

A workaround could be to add a separate crate for no_std tests, or do the inverse and move the current tests into a std-tests crate. The important thing is no_std should be a subset of std, so as long as no_std compiles, and std passes tests, no_std should work as expected.

@droundy droundy merged commit 6deb87d into droundy:master Mar 2, 2025
7 of 8 checks passed
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