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

Added an implementation of lazy_static without std in stable Rust #122

Closed
wants to merge 3 commits into from

Conversation

brunoczim
Copy link
Contributor

@brunoczim brunoczim commented Sep 26, 2018

Currently, lazy static has an implementation for no_std crates which requires nightly. I have added a new implementation of lazy_static which works on stable Rust.

Question: I have added a new feature and a new implementation. Should we keep it as a new implementation or should we overwrite nightly spin implementation?

Revert "removed incompatiblities from tests"

This reverts commit 31cc1f5.

removed incompatiblities from tests
@KodrAus
Copy link
Contributor

KodrAus commented Sep 26, 2018

Hi @brunoczim! 👋

Before we dig into the code itself are you able to share a bit about how you've found yourself needing it, and how this new implementation works compared to the existing nightly one using spin?

@brunoczim
Copy link
Contributor Author

Use

Okay. I confess this is not something I need. I don't want to be redundant, but imagine you are writing a Kernel and you do not want to have to use nightly but need to initialize some... lazy static. I mean, you would have to introduce some instability by using nightly.

Some differences

I think the main important is: I did not wrote some generic API shared between all static variables. I wrote inside the __lazy_static_create a specific type for that static. This removes the need of having a const fn.

Another difference is: spin's implementation does not store a T. It stores an Option<T>. I think there is a little overhead on checking Option<T> every time the static is accessed. My implementation used a trick in order not to have this overhead. I used core::mem::size_of to make a byte array with the size of T. I have also used a zero-sized array of T to force alignment.

However, the spinning over the initialization status works in the same way as spin's implementation of Once. I thought they used a Mutex to write Once, but it is more like a RwLock (just like I did). Not sure why the Mutex was in my head, maybe it was like that one day.

I do recognize there is not too much difference. The real reason I implemented this is so we can have a stable API with no_std. Importing it from another crate does not allow us avoiding const fn.

NOTE: the size_of trick only works on non-generic types. This is the one of the reasons I wrote a specific type for each static.

@brunoczim
Copy link
Contributor Author

Well, there is another option. I was thinking: spin could expose Once::<T>::INIT. This would allow lazy_static to use it on stable.

@brunoczim
Copy link
Contributor Author

brunoczim commented Oct 1, 2018

I have had a merged PR on spin which allows Once to be initialized via constant on stable Rust. This custom implementation of lazy_static is not needed anymore. We may simply rewrite the spin version.

@KodrAus
Copy link
Contributor

KodrAus commented Oct 3, 2018

Thanks for chasing this up @brunoczim! A no-std spin-based implementation that also works on stable sounds great to me 👍

@KodrAus KodrAus closed this Oct 30, 2018
@KodrAus KodrAus reopened this Oct 30, 2018
@KodrAus
Copy link
Contributor

KodrAus commented Oct 30, 2018

@brunoczim now that mvdnes/spin-rs#55 is merged and released on crates.io would you be interested in refactoring our existing no_std impl to work on stable Rust using spin?

@brunoczim
Copy link
Contributor Author

Yes I am. Count me in.

@brunoczim
Copy link
Contributor Author

I am closing this in favor of #130

@brunoczim brunoczim closed this Oct 31, 2018
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