Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

(Review with whitespace changes off, this is not a big diff.)

Calling EnumAllVirtualSlots on classes returns all newslot methods that exist in the class hierarchy. Calling this on interfaces produced an empty enumeration.

For runtime-async, it would be convenient if this returned the logical slots on interfaces. While interfaces don't have physical slots, they still have logical slots, so it's not a big stretch.

Doing this now simplifies things in some places, and requires an extra if check in other places. But it will be really convenient for runtime-async.

Cc @dotnet/ilc-contrib

Calling `EnumAllVirtualSlots` on classes returns all `newslot` methods that exist in the class hierarchy. Calling this on interfaces produced an empty enumeration.

For runtime-async, it would be convenient if this returned the logical slots on interfaces. While interfaces don't have physical slots, they still have logical slots, so it's not a big stretch.

Doing this now simplifies things in some places, and requires an extra `if` check in other places. But it will be really convenient for runtime-async.
Copilot AI review requested due to automatic review settings November 7, 2025 07:34
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies EnumAllVirtualSlots to return virtual method slots for interfaces, whereas previously it returned an empty enumeration. For interfaces, it now delegates to GetAllVirtualMethods(), while for classes it continues to enumerate slots through the class hierarchy.

Key changes:

  • EnumAllVirtualSlots now returns interface methods when called on interface types
  • Code previously branching between interfaces and classes now uniformly calls EnumAllVirtualSlots
  • New guard conditions added where interface-specific behavior should be excluded

Reviewed Changes

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

Show a summary per file
File Description
MetadataVirtualMethodAlgorithm.cs Modified EnumAllVirtualSlots to handle interfaces by delegating to GetAllVirtualMethods()
VTableSliceNode.cs Removed interface check and conditional logic, now uniformly uses EnumAllVirtualSlots
EETypeNode.cs Added interface guard and replaced GetAllVirtualMethods() calls with EnumAllVirtualSlots()
TypeValidationChecker.cs Added interface guard to skip validation logic that only applies to classes
TypeGVMEntriesNode.cs Added assertion to document that interfaces are not supported
NativeLayoutVertexNode.cs Removed unused helper method EnumVirtualSlotsDeclaredOnType

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this pull request Nov 7, 2025
This makes virtual methods work with runtime async. Depends on dotnet#121438.

Runtime async methods can be virtually called two ways: as Task-returning, or as runtime-async. We can even end up in situation where Task-returning non-runtime-async virtual methods get called as runtime-async.

The easiest way to solve this is to give each Task-returning method two separate virtual slots. We still track the slot use so unused slots will not get generated.

The `VirtualMethodAlgorithm` abstraction that we added for universal shared generics comes in handy because it lets us centralize where all of this work happens. As a result, we don't need to teach pretty much any node dealing with virtuals about runtime async, it just falls out.

This doesn't make generic virtual methods yet, those need more fixes.
@MichalStrehovsky
Copy link
Member Author

/ba-g android timeouts

@MichalStrehovsky MichalStrehovsky merged commit e3f6dd9 into dotnet:main Nov 7, 2025
92 of 105 checks passed
@MichalStrehovsky MichalStrehovsky deleted the virtualslots branch November 7, 2025 22:22
MichalStrehovsky added a commit that referenced this pull request Nov 8, 2025
This makes virtual methods work with runtime async. Depends on #121438.

Runtime async methods can be virtually called two ways: as
Task-returning, or as runtime-async. We can even end up in situation
where Task-returning non-runtime-async virtual methods get called as
runtime-async.

The easiest way to solve this is to give each Task-returning method two
separate virtual slots. We still track the slot use so unused slots will
not get generated.

The `VirtualMethodAlgorithm` abstraction that we added for universal
shared generics comes in handy because it lets us centralize where all
of this work happens. As a result, we don't need to teach pretty much
any node dealing with virtuals about runtime async, it just falls out.

This doesn't make generic virtual methods yet, those need more fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants