Rename AsyncMethodKind enum values: NotAsync→Ordinary, RuntimeAsync→TaskReturningILAsync #121649

Copilot · 2025-11-14T22:28:13Z

The term "TaskReturning" was overloaded, creating confusion between the enum value AsyncMethodKind::TaskReturning and the concept of "task-returning methods" more broadly.

Changes

AsyncMethodKind::NotAsync → AsyncMethodKind::Ordinary
Clearer name for regular methods not involved in async functionality
AsyncMethodKind::RuntimeAsync → AsyncMethodKind::TaskReturningILAsync
Explicitly indicates task-returning methods marked with MethodImpl::Async in metadata
Updated comments to reflect new naming, particularly the relationship between {TaskReturning, AsyncVariantThunk} and {TaskReturningILAsync, AsyncVariantImpl} pairs

Impact

// Before: confusing naming
inline bool IsTaskReturningMethod() const
{
    return asyncKind == AsyncMethodKind::RuntimeAsync ||  // task-returning but not called "TaskReturning"
        asyncKind == AsyncMethodKind::TaskReturning;      // also task-returning
}

// After: consistent naming
inline bool IsTaskReturningMethod() const
{
    return asyncKind == AsyncMethodKind::TaskReturningILAsync ||  // task-returning with IL async
        asyncKind == AsyncMethodKind::TaskReturning;              // task-returning without IL async
}

The new naming convention: enum values prefixed with Async indicate methods using async calling convention (runtime async), while TaskReturning* indicates methods that return Task/ValueTask types.

Modified files: method.hpp, methodtablebuilder.cpp, class.cpp (12 total changes)

Original prompt

This section details on the original issue you should resolve

<issue_title>[Suggestion] Rename AsyncMethodKind.TaskReturning</issue_title>
<issue_description>I got a bit confused by the naming and comments here

Hopefully I understand it correctly now, but if not please correct me. It appears we are using the term "Task Returning" to mean different things in different places:

AsyncMethodKind.TaskReturning - The MethodDesc variant whose entrypoint returns a task and it corresponds to a C# method not marked with the MethodImpl::Async attribute.
// Task-returning methods - Any C# method returning a Task regardless if it has the MethodImpl::Async attribute

I think if you renamed AsyncMethodKind.TaskReturning -> AsyncMethodKind.CompilerAsync it would remove the naming collision and it would also contrast nicely: RuntimeAsync vs CompilerAsync.
</issue_description>

<agent_instructions>Rename AsyncMethodKind::NotAsync to AsyncMethodKind::Ordinary
Rename AsyncMethodKind::RuntimeAsync to AsyncMethodKind::TaskReturningILAsync
Update corresponding comments</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@jakobbotsch > I think if you renamed AsyncMethodKind.TaskReturning -> AsyncMethodKind.CompilerAsync it would remove the naming collision and it would also contrast nicely: RuntimeAsync vs CompilerAsync.

Compiler async is quite confusing to me, since in the context of the runtime/VM the only compiler I see is the JIT/interpreter's compiler. But IIUC, here it would refer to something that originally produced the IL that we are seeing? Such a compiler doesn't even need to exist.

Originally these were the names:
https://github.com/dotnet/runtimelab/blame/d95f6eadeeb696071635d9662fbe33f8a0e0d645/src/coreclr/vm/method.hpp#L58-L64

I quite liked those names, but the enum grew and things changed so it wouldn't really map anymore.</comment_new>
<comment_new>@VSadov

AsyncMethodKind.TaskReturning - The MethodDesc variant whose entrypoint returns a task and it corresponds to a C# method marked with the MethodImpl::Async attribute.

No. AsyncMethodKind.TaskReturning is a method that returns Task, ValueTask or generic version of that and the method's definition in metadata is NOT marked as MethodImpl::Async

Basically, these methoddescs represent ordinary Task-returning methods. The IL may contain a state machine produced by C#/VB/?? or may contain whatever else code.

The IL for these methods is compiled as-is. To participate in runtime async such methods get paired with a thunk that has runtime-async calling convention.

I think TaskReturning is appropriate as the part that they return tasks is what makes them special, while NotAsync are not special at all. </comment_new>
<comment_new>@VSadov
Perhaps it is NotAsync that is confusing and should be called something else. NotAsync is for ordinary methods not interesting to runtime async in any way.
Ordinary could probably be a better name, but I think we have other classifications that use Ordinary so it may be confusing that these are also ordinary but in async sense.</comment_new>
<comment_new>@noahfalk

No. AsyncMethodKind.TaskReturning is a method that returns Task, ValueTask or generic version of that and the method's definition in metadata is NOT marked as MethodImpl::Async

Oops I had the right idea in my head but then still wrote the wrong text. I stink. I edited it in the original post so hopefully its correct now.

I think TaskReturning is appropriate as the part that they return tasks is what makes them special, while NotAsync are not special at all.

The key thing that I am suggesting we fix is that "TaskReturning" should have only one definition. I discovered this function which I think really exemplifies it:

    inline bool IsTaskReturningMethod() const
    {
        ...
        return asyncKind == AsyncMethodKind::RuntimeAsync ||
            asyncKind == AsyncMethodKind::TaskReturning;
    }

The meaning of "TaskReturning" in the method name isn't the same as the meaning of "TaskReturning" in the enum value. Right now AsyncMethodKind::RuntimeAsync is task-returning (IsTaskReturningMethod()==true) and also isn't task-returning (asyncKind != AsyncMethodKind::TaskReturning).

IMO ...

Fixes [Suggestion] Rename AsyncMethodKind.TaskReturning #119565

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

dotnet-policy-service · 2025-11-14T23:42:47Z

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

VSadov · 2025-11-14T23:45:37Z

TODO: this may need to run CI with commits from #119420 - to see if everything works with async enabled.

Those commits would need to be reverted once we ready to merge. (we are not yet enabling async in the main)

VSadov · 2025-11-14T23:48:03Z

CC: @jakobbotsch @agocke - mostly an FYI on renames.

noahfalk · 2025-11-15T00:03:44Z

#119565 (comment)

jkotas · 2025-11-15T00:03:49Z

src/coreclr/vm/method.hpp

 #define METHOD_TOKEN_RANGE_MASK ((1 << METHOD_TOKEN_RANGE_BIT_COUNT) - 1)

 enum class AsyncMethodKind
 {


Would it be better to make this a bit field?

ReturnsTaskOrValueTask = 0x1, // The method returns Task or ValueTask

Async = 0x2, // The method IL in metadata has MethodImplAttributes.Async

AsyncCall = 0x4, // The entrypoint of this method has async calling convention

I took the bit names from the managed type system. I am not attached to these names. If we come up with different names, we should update the managed type system to match.

Would it be better to make this a bit field?

Possibly. While it may be difficult to name combinations of traits, each individual trait seems easier to name.

I'd prefer something like ILAsync for the presence of the MethodImpl, or MethodImplAsync if we go for more verbose names.

Most of the runtime uses Async/ASYNC to mean "async callconv" or "emitted as state machine" and that matters in a lot more places than the places that care about the methodimpl, which is basically just the part that selects who owns the actual IL.
Thus I think the shorter Async should map to the runtime meaning of Async.

Maybe we do not even need the flag for MethodImplAsync once we sort out which variant is a thunk and which is not.

Let me think/try if we can have just:

- ReturnsTaskOrValueTask // returns a promise type, not Async (in runtime sense) - Async // the body is a state machine, assumes async callconv - Variant // method has two variants and this is one of them - Thunk // has synthetic body, which forwards to the other variant

Managed type system may still need something for MethodImplAttributes.Async as a part of object model.

Also in the managed typesystem it is a bit more obvious when things represent metadata/ECMA entities vs. something that models the actual execution. So the managed type system could as well use Async for the methodimpl, maybe..., If not too confusing.

each individual trait seems easier to name.

Yep, individual traits are easier to name and explain.

MethodImplAsync if we go for more verbose names.

More verbose self-describing names are good to avoid confusion. MethodImplAsync flag / IsMethodImplAsync would be fine with me.

MethodImplAsync

The managed type system has similar prior art: The property for MethodImplAttributes.Runtime is called MethodDesc.IsRuntimeImplemented since MethodDesc.IsRuntime would look odd.

noahfalk · 2025-11-15T00:08:13Z

src/coreclr/vm/method.hpp

+    // Note: Generic T-returning methods are Ordinary, even if T could be a Task.
+    Ordinary,

    // Regular methods that return Task/ValueTask


AsyncMethodKind.TaskReturning is the name that I think is important to change to eliminate ambiguity.
#119565 (comment)

AsyncMethodKind.TaskReturning literally means "a method whose definition returns a Task (or ValueTask)". Hard to come up with a better name for the concept.

The MethodImpl.Async only tells how to interpret the IL inside the method. The definition still returns a Task.

Maybe it will become more clear if it is a bit in a bitfield.

"TaskReturning" is an equally accurate description for AsyncMethodKind.RuntimeAsync which are also methods that return a Task. AsyncMethodKind.TaskReturning is not refering to all TaskReturning methods, only to a particular subset of them. I was trying to pick a name which is narrow enough that it doesn't include any methods outside of the intended subset.

I'm just fine with the bitfield Jan suggested as it also avoids the ambiguity around the term Task Returning.

…>TaskReturningILAsync Co-authored-by: VSadov <[email protected]>

VSadov · 2025-11-15T20:34:18Z

src/coreclr/inc/clrconfigvalues.h


 // Runtime-async
-RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_RuntimeAsync, W("RuntimeAsync"), 0, "Enables runtime async method support")
+RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_RuntimeAsync, W("RuntimeAsync"), 1, "Enables runtime async method support")


TODO: undo the commit that enables async feature before merging

jkotas · 2025-11-15T20:41:06Z

src/coreclr/vm/method.hpp

 #define METHOD_TOKEN_RANGE_BIT_COUNT (24 - METHOD_TOKEN_REMAINDER_BIT_COUNT)
 #define METHOD_TOKEN_RANGE_MASK ((1 << METHOD_TOKEN_RANGE_BIT_COUNT) - 1)

 enum class AsyncMethodKind


Should this be called AsyncMethodFlags now that it is a collection of flags and not an enumeration of all possible kinds?

jkotas · 2025-11-15T20:41:44Z

src/coreclr/vm/jitinterface.cpp

            // we cannot resolve to an Async variant in such case.
            // return NULL, so that caller would re-resolve as a regular method call
-            pMD = pMD->IsTaskReturningMethod() ?
+            pMD = pMD->IsTaskOrValueTaskReturningMethod() ?


We should unify on TaskOrValueTaskReturningMethod or ReturnsTaskOrValueTask everywhere.

jkotas · 2025-11-15T20:49:44Z

src/coreclr/vm/method.hpp

-    // which owns the original IL. (AsyncVariantImpl)
-    RuntimeAsync,
+    // Method body is emitted as a state machine and uses Async call convention.
+    Async                  = 1,


We should unify on one of Async or AsyncCall to indentify async calling convention everywhere.

jkotas · 2025-11-15T20:56:37Z

src/coreclr/vm/method.hpp

+    Async                  = 1,
+    // Not Async method that returns a promise type {Task, Task<T>, ValueTask, ValueTask<T>}
+    ReturnsTaskOrValueTask = 2,
+    // Method has two variants and this is one of them.


Suggested change

// Method has two variants and this is one of them.

// MethodDesc represents async calling convention entrypoint of a Task-returning method.

Can the description of AsyncVariant be more precise?

jkotas · 2025-11-15T21:02:48Z

src/coreclr/vm/method.hpp

+
+    // Is this a Variant method?
+    // If yes, the method has another variant.
+    inline bool IsVariantMethod() const


I do not understand how this is different from IsAsyncVariant.

Also, "Variant" without "Async" it makes it look like it has something to do with type variance that is completely different feature.

Variant flag only tells that this is one from a pair. Basically means we can call GetAsyncOtherVariant and get the other variant.

Perhaps HasAsyncOtherVariant is a better name. - it would match GetAsyncOtherVariant.

If we standardize on AsyncCall to mean async calling convention, I think a better names would be IsAsyncCallVariant / GetAsyncCallVariant. They make it clear that it is about the entrypoint with async calling convention.

standardize on AsyncCall to mean async calling convention

If we were to standardize on AsyncCallConv to mean async calling convention, this would be IsAsyncCallConvVariant / GetAsyncCallConvVariant

jkotas · 2025-11-16T02:01:45Z

src/coreclr/vm/methodtablebuilder.h

        {
-            return GetAsyncMethodKind() == AsyncMethodKind::AsyncVariantThunk ||
-                GetAsyncMethodKind() == AsyncMethodKind::AsyncVariantImpl;
+            return hasAsyncKindFlags(GetAsyncMethodKind(), AsyncMethodKind::Async | AsyncMethodKind::HasAsyncOtherVariant);


Suggested change

return hasAsyncKindFlags(GetAsyncMethodKind(), AsyncMethodKind::Async | AsyncMethodKind::HasAsyncOtherVariant);

return hasAsyncKindFlags(GetAsyncMethodKind(), AsyncMethodKind::HasAsyncOtherVariant);

Does this need to test AsyncMethodKind::Async bit?

Copilot AI assigned Copilot and VSadov Nov 14, 2025

Copilot started work on behalf of VSadov November 14, 2025 22:28 View session

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 14, 2025

Copilot AI changed the title ~~[WIP] Rename AsyncMethodKind.TaskReturning for clarity~~ Rename AsyncMethodKind enum values: NotAsync→Ordinary, RuntimeAsync→TaskReturningILAsync Nov 14, 2025

Copilot AI requested a review from VSadov November 14, 2025 23:05

Copilot finished work on behalf of VSadov November 14, 2025 23:05

VSadov added area-VM-coreclr runtime-async and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 14, 2025

VSadov requested review from jakobbotsch and noahfalk November 14, 2025 23:47

jkotas reviewed Nov 15, 2025

View reviewed changes

noahfalk reviewed Nov 15, 2025

View reviewed changes

Copilot AI and others added 5 commits November 14, 2025 20:31

Initial plan

b1ba0d4

Rename AsyncMethodKind enum values: NotAsync->Ordinary, RuntimeAsync-…

eb21d31

…>TaskReturningILAsync Co-authored-by: VSadov <[email protected]>

Enable async feature/tests

17a7f01

introduced flags

596e27b

comments

7f7870f

VSadov force-pushed the copilot/rename-asyncmethodkind-taskreturning branch from 6fd26bf to 7f7870f Compare November 15, 2025 20:33

VSadov reviewed Nov 15, 2025

View reviewed changes

jkotas reviewed Nov 15, 2025

View reviewed changes

cleanup typos and comments

c565b98

HasAsyncOtherVariant

f73651e

jkotas reviewed Nov 16, 2025

View reviewed changes

	// Method has two variants and this is one of them.
	// MethodDesc represents async calling convention entrypoint of a Task-returning method.

	return hasAsyncKindFlags(GetAsyncMethodKind(), AsyncMethodKind::Async \| AsyncMethodKind::HasAsyncOtherVariant);
	return hasAsyncKindFlags(GetAsyncMethodKind(), AsyncMethodKind::HasAsyncOtherVariant);

Rename AsyncMethodKind enum values: NotAsync→Ordinary, RuntimeAsync→TaskReturningILAsync #121649

Are you sure you want to change the base?

Rename AsyncMethodKind enum values: NotAsync→Ordinary, RuntimeAsync→TaskReturningILAsync #121649

Conversation

Copilot AI commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Impact

Comments on the Issue (you are @copilot in this section)

Uh oh!

dotnet-policy-service bot commented Nov 14, 2025

Uh oh!

VSadov commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VSadov commented Nov 14, 2025

Uh oh!

noahfalk commented Nov 15, 2025

Uh oh!

jkotas Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VSadov Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VSadov Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VSadov Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VSadov Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Nov 14, 2025 •

edited

Loading

VSadov commented Nov 14, 2025 •

edited

Loading

jkotas Nov 15, 2025 •

edited

Loading

VSadov Nov 15, 2025 •

edited

Loading

VSadov Nov 15, 2025 •

edited

Loading

VSadov Nov 15, 2025 •

edited

Loading

VSadov Nov 15, 2025 •

edited

Loading