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

Add native lock speclet #7849

Merged
merged 6 commits into from
Jan 26, 2024
Merged

Add native lock speclet #7849

merged 6 commits into from
Jan 26, 2024

Conversation

jjonescz
Copy link
Member

@jjonescz jjonescz commented Jan 16, 2024

@jjonescz jjonescz requested a review from a team as a code owner January 16, 2024 14:15
proposals/native-lock.md Show resolved Hide resolved
proposals/native-lock.md Show resolved Hide resolved
proposals/native-lock.md Show resolved Hide resolved
> {
> class Lock
> {
> public Scope EnterLockScope();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I updated the proposal in the Lock API review to rename this method back to EnterScope, as the Lock in the name is redundant given the type name and it's not being suggested for use as a general pattern anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not being suggested for use as a general pattern anymore

I think that's still considered as future work (when ref structs can implement interfaces). Wouldn't it be better to keep the name EnterLockScope to make the transition to the generic pattern smoother?

Copy link
Member

@kouvel kouvel Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that it may or may not be expanded to a general lock pattern, and figured we might as well use an appropriate name for now. If a general pattern is exposed in the future it may also be more useful for it to be usable by other existing types, which may also involve new syntax or some such. At that time if it's necessary to add EnterLockScope again we could, and perhaps explicitly implement it if it's an interface member to hide it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I assume this can still change in API review. Do you know when the API will be approved?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I don't think it's been scheduled yet, will add you when I find out

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec should reflect our best guess for what the API will be. If API review changes the name we will just update the spec.

Based on previous experiences though we should put the name @kouvel least wants in the spec. Cause inevitably whatever we pick in the initial spec always changes 😉

proposals/native-lock.md Outdated Show resolved Hide resolved
proposals/native-lock.md Outdated Show resolved Hide resolved
Because Scope is a ref struct, awaits are not allowed anyway.
@jjonescz jjonescz merged commit 2a2cdb0 into dotnet:main Jan 26, 2024
1 check passed
@jjonescz jjonescz deleted the NativeLock-01 branch January 26, 2024 07:35
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.

5 participants