Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Normative: Add Mutex and Condition #30
base: main
Are you sure you want to change the base?
Normative: Add Mutex and Condition #30
Changes from 2 commits
5a4f6b2
7c722a6
df7d0c8
5de3d70
070cdc0
db8d64f
9e23b00
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Presumably
lockAsync
to be added later?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.
Yeah, async variants are pretty complex, and TBH I'd prefer to do them as a follow-up proposal entirely.
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.
Also wait why is this static instead of prototype?
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.
Because shared structs can't point to unshared objects, and functions can't be shared, so having a prototype requires something like per-Realm prototypes that I have talked about in the past, together with solving the "correlation problem" (the registry thing). We're still actively discussing how to solve this issue with Mark & co, and have decided to leave it as an open design question to be settled during Stage 2. As such, the extra complexity of per-Realm prototypes is omitted from the spec draft.
If per-Realm prototypes get consensus, then this will definitely be a prototype.
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.
(It's my interpretation that where these methods live isn't as major a semantics point as what they do.)
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 expect we'll eventually make tokens disposable, which makes the common pattern for timeouts look like
Returning a string here means that will throw, which is... probably the thing you want? It'll be kind of a cryptic error message, but that's better than thinking you have the lock when you don't (
using token = null
is legal, so returningnull
here would make that silently succeed).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.
Do we want
tryWithLock
? Not sure what to return in the already-locked case, though. Maybe have a parameter to use for that value?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 don't see that often in other PLs/stdlibs. I'm not against it though.
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.
Swift 6 (new as of 2 days ago) has withLockIfAvailable1, which is this. I also think that's a better name than
tryLock
/tryWithLock
, incidentally.Really it's just filling out the missing quadrant on the (lock()+unlock() vs callback) x (blocking vs only-if-available) matrix, though, so it's pretty natural if we already have the other three quadrants filled out.
Footnotes
You can read through its history in this thread if you want to. ↩
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.
Should this be exposed/constructible? Could be nice for some patterns when re-using tokens, instead of needing to wait until you actually want to lock.
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 don't mind, but where would you put 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.
My first instinct is, if the tokens can only be used with mutexes, then Atomics.Mutex.UnlockToken, otherwise Atomics.UnlockToken.