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 no-std support #6
Conversation
Hey, thanks for looking at this so quickly. I took a quick look at the PR and left some comments around the feature flag definition. |
Cargo.toml
Outdated
@@ -15,6 +15,10 @@ categories = ["rust-patterns", "memory-management"] | |||
|
|||
[features] | |||
unpin = [] | |||
no-std = ['dep:critical-section'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than adding a no-std
feature, you should add an std
feature and make it a default.
From https://doc.rust-lang.org/cargo/reference/features.html#feature-unification:
If you want to optionally support
no_std
environments, do not use ano_std
feature. Instead, use astd
feature that enablesstd
.
An example: https://github.com/matklad/once_cell/blob/master/Cargo.toml
This would allow you to make unpin
dependent on std
. Right now, enabling both unpin
and no-std
wouldn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not work: you cannot enable a dependency based on the lack of a feature. Renaming the feature to critical-section
might be better, however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, enabling both unpin and no-std wouldn't work.
This actually does work, but demonstrates why the name no-std
is misleading: the normal types will continue to use critical-section
Mutexes, and the unpin
module is also available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant that it wouldn't work because unpin
contains std
types, but it looks good to me now that unpin depends on std
127b41e
to
1eda4f5
Compare
This allows running in a no_std environment.
I didn't do any functional testing but I read through the code and checked that I could build for a no-std target for a test project with:
|
Fixes #5.
@kesyog, please test this before I release 0.4.4 with it.