-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix sequence generation to be always increasing #6
Conversation
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.
Thanks for the PR, please run cargo fmt
on the thing once. Looks almost fine besides that.
Thanks for reviewing 😊 ! Your feedback has been applied. I'm running the loom tests locally now (I did not realize that they didn't run as part of the normal test suite before), using |
The trick was that since the sequence stores the 'next' value, at the very start we have to initialize it at In real programs this won't ever come up of course, since time will always have passed since the epoch. |
a988bf4
to
9502781
Compare
This is very strange, I have no idea why |
Hang on; I think I might be running a slightly older version of |
9502781
to
f61231b
Compare
That should've done it 👍 |
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.
Just a few leftovers from debugging it seems.
Co-authored-by: MrGunflame <[email protected]>
Co-authored-by: MrGunflame <[email protected]>
Apologies, that was sloppy. I should stop trying to make these kinds of PRs late at night. Thanks for your keen eye! |
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.
No need to apologize, thanks.
Released in version |
test_generate
tests withtest_generate_ordered
testsMaybeUninit
inside the sync generation. I've looked at the created assembly (using cargo asm) and the compiler is more than clever enough to optimize the wrapping and unwrapping into anOption
away here, so there's no need for unsafe. (Extra fun as it was the only unsafe in this crate, so now it's fullyno-unsafe
!)Fixes #5