Remove HoistedLocal from runtime-async spec

[agocke]

No description provided.

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

docs/design/specs/runtime-async.md:49

• The specification now states that any read of a by-ref variable after suspension will produce null, which may lead to unexpected behavior if consumers are unaware of this side effect. It would be helpful to provide clarification on how this null value is enforced and under what conditions, to avoid potential runtime issues.

Only local variables which are "hoisted" may be used across suspension points. That is, only "hoisted" local variables will have their state preserved after returning from a suspension. On methods with the `localsinit` flag set, non-"hoisted" local variables will be initialized to their default value when resuming from suspension. By-ref variables may not be hoisted across suspension, and any read of a by-ref variable after suspension will produce null.

docs/design/specs/runtime-async.md:53

• The removal of the bullet regarding pinning locals being marked with HoistedLocal removes information about how these variables should be handled. Consider adding an equivalent clarification to describe the behavior of pinning locals in the updated specification.

* Pinning locals may not be marked `HoistedLocal`

[jakobbotsch]

Should we have a similar rule that locals marked pinned are zeroed at suspension points?

[VSadov]

Yes. Lets break pinned locals in the same way as byrefs.

[VSadov]

Typically a pinned local is an internal pointer (&), so it is already broken by the other rule, if lives across Await.

But object references (i.e. o) can be pinned as well (some forms of fixed(..){..} actually use that), so we need a separate rule for this case.

[VSadov]

Re: "* Applying MethodImplOptions.Async to methods with byref or ref-like parameters is invalid."

I think we should forbid byref or ref-like returns, since we need to transfer the return value to the caller and the caller may end up running on a different thread.

But ref/ref-like parameters seems ok, in principle. As long as they are not used after an Await, it would work.
Allowing byref parameters implicitly allows async method in structs, which we want to allow.

Roslyn can be more strict on the parameters, of course.

@jakobbotsch - any concerns about allowing byref/ref-like parameters from the JIT side?
(we could specialcase allow just struct methods)

[jakobbotsch]

Re: "* Applying MethodImplOptions.Async to methods with byref or ref-like parameters is invalid."

I think we should forbid byref or ref-like returns, since we need to transfer the return value to the caller and the caller may end up running on a different thread.

But ref/ref-like parameters seems ok, in principle. As long as they are not used after an Await, it would work. Allowing byref parameters implicitly allows async method in structs, which we want to allow.

Roslyn can be more strict on the parameters, of course.

@jakobbotsch - any concerns about allowing byref/ref-like parameters from the JIT side? (we could specialcase allow just struct methods)

No, I agree that we should allow these as parameters.

[jakobbotsch]

Should we rather have an item that clarifies that the return type must be Task/ValueTask/Task<T>/ValueTask<T>? The fact that T cannot be a byref type falls out naturally from the constraints on T.

[VSadov]

I think we can be explicit here that the return value cannot be byref. - so no ambiguities whether ref Task foo() is allowed or not.

The part that return type cannot be ref-like kind of follows from requiring Task or ValueTask, but does not seem to hurt to explicitly mention that, so I think it is ok either way.

[jcouv]

@333fred We'll want a corresponding roslyn design doc update to remove HoistedLocal

[agocke]

/ba-g doc only change

[am11]

/ba-g doc only change

It was a real lint error with markdown, failing on other PRs. Fix #115159.