Skip to content

Commit

Permalink
Do not generate reflectable sealed methods as virtual (dotnet#96738)
Browse files Browse the repository at this point in the history
Noticed this while working on dotnet#96672. There are two ways how virtual method can be referred to: as a direct reference to the method body (i.e. non-virtually), or as a reference to a particular slot number (virtually). When we mark a virtual method as visible by reflection, it typically means “virtually visible to reflection” (one can invoke the method with a random derived `this` so the invoke happens through the slot).

But we still try to optimize this into a direct reference if possible so that we don’t waste an extra slot. This is possible for final methods, or methods on sealed classes for example. The marking in dependency graph didn’t take into account this optimization – it was creating an unnecessary virtual slot for reflection-visible final methods that then went unused. With this half of the diff in dotnet#96672 is technically not necessary (we no longer end up with the new virtual method in the vtable), but that fix is still a correct fix.
  • Loading branch information
MichalStrehovsky authored and tmds committed Jan 23, 2024
1 parent c6006c6 commit e5a6a14
Showing 1 changed file with 1 addition and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
}
else
{
if (!factory.VTable(slotDefiningMethod.OwningType).HasFixedSlots)
if (ReflectionVirtualInvokeMapNode.NeedsVirtualInvokeInfo(slotDefiningMethod) && !factory.VTable(slotDefiningMethod.OwningType).HasFixedSlots)
dependencies.Add(factory.VirtualMethodUse(slotDefiningMethod), "Virtually callable reflectable method");
}
}
Expand Down

0 comments on commit e5a6a14

Please sign in to comment.