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

Allow initialization using a const fn, and initialization to values other than None #3

Closed
wants to merge 2 commits into from

Conversation

thomcc
Copy link

@thomcc thomcc commented Jun 12, 2020

Hi. This seems to work, and somewhat invalidates the need for the macro too, although I left it and just used static_none to initialize it instead. I don't mind reverting the macro changes if you want -- They are not fully backwards compatible, if someone did &*THE_STATIC_FROM_THE_MACRO it would work before (invoke the Deref of the dummy type explicitly) but not after.

I wasn't sure what to name the functions, and don't really what I chose a lot, but maybe they're fine. If you'd prefer different names let me know.

The move to AtomicPtr for AtomicRef::static_some is needed because pointer to int in const fn isn't allowed. I kind of doubt it ever will be allowed, given that a relocation will need to be emitted for constructs like this in dynamic libraries on some platforms, and usize completely obscures that (I was a little surprised it worked with AtomicPtr, to be honest.

I think if you wanted you could clean this up a bit now, since the macro and the ATOMIC_U8_REF_INIT are both a little redundant. The crate-level docs probably need updating too since this maybe is a more ergonomic API than the one they show, but I can do that if you want, or I can leave it to you.

@thomcc
Copy link
Author

thomcc commented Jun 12, 2020

I also wonder if it would be worth providing a different type for an AtomicRef that's not allowed to be None, now that you can provide a static initializer.

@thomcc
Copy link
Author

thomcc commented Jun 12, 2020

Oh, hm, I missed the other PR, which does this too. I guess it's a bit redundant if conditionals in const fn are that close to stabilizing.

@mystor
Copy link
Owner

mystor commented Oct 31, 2020

I think I'm going to close this in favour of #2. Thanks for the PR though!

@mystor mystor closed this Oct 31, 2020
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