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
Add draft of runtime async ECMA-335 changes #104063
base: main
Are you sure you want to change the base?
Add draft of runtime async ECMA-335 changes #104063
Changes from 1 commit
e582a11
d830f9b
3463e21
e3f25cd
db9697c
52769b8
21a66e8
745ebe9
43fea46
b161b78
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.
async2? will this temporary keyword for the prototype be part of our ecma spec?
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.
Temporary 😄 Long-term I think we can use “runtime async” and “compiler async”
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.
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.
We should split these restrictions into fundamental (hard/impossible to ever remove them) and non-fundamental ones that just exist to make the initial implementation easier.
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.
Are there any truly fundamental restrictions? I think, with enough effort, we could make almost anything work.
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 think escaping ref and byref locals are fundamental restriction category. I agree that we can make them work in principle, but it would come with tough performance trade-offs.
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.
Note that
ldloca
andldarga
already return managed pointers in the spec. This should rather be a modification around the note we have on transient pointers here: https://github.com/dotnet/runtime/blob/main/docs/design/specs/Ecma-335-Augments.md#transient-pointersThere is a question of what level of behavior we want to specify. We can either specify
or
The latter precludes code like
unsafe { <unsafe code taking addresses of locals without any suspension points> }
in C#, but Roslyn already warns on that today.I think in either case we also need to document that values of managed pointers and structs containing managed pointers are not preserved across suspension points (at least for an initial version).
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 picked the last one because it's the most restrictive. We can relax it if necessary.
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.
This sounds like it would impact aspect of Ref.Emit and invoke. Has Ref.Emit been considered with respect to async2 methods?
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.
Not sure if it's obvious but this means that if you want to use the generated signature you have to emit a
call
,callvirt
,ldftn
orldvirtfn
with aMethodRef
orMethodSpec
token, notMethodDef
, right?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.
That's a good question. I think that's correct, but I have to review the full ECMA-335 spec to make sure.
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.
OK, reviewed, you're correct. I don't have time right now to find every spot in the spec that would need to be adjusted here, but the intent is that basically the calling convention for implicit definitions of both sync and async methods would be to use
MethodRef
just like aVARARG
call. I think every spot in the spec that mentions special casing for VARARG should be adjusted to also mention implicit definitions.