-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
RequireMethodImplToRemainInEffectAttribute #35315
Comments
Quick clarification as well: we do not expect any developer to directly use this attribute. It will be placed by the Roslyn compiler appropriately on methods. |
cc @dotnet/crossgen-contrib |
@vargaz @lambdageek should be looped in on this issue. |
cc @janvorli |
We will need to teach the IL linker to keep this attribute /cc @vitek-karas |
IL Linker will not strip attributes it knows nothing about because it would be a bug farm. dotnet/linker#952 (comment) |
A few questions:
|
|
If we choose to encode this in metadata, it should be a bit in the MethodAttributes column where the existing bits that control virtual method resolution live ( |
We don't have bits in the MethodAttribute column anymore, unless we want to do creative combinations (
Doesn't the same concern apply to the custom attribute? Roslyn already enfoces that user can't set bits in the MethodImpl flag (e.g. you can't set bit 1 and say the method body is native code).
We'll need to touch ILASM/ILDASM for covariant returns anyway: #37045. |
Let's go with namespace System.Runtime.CompilerServices
{
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = false)]
public sealed class PreserveBaseOverridesAttribute : Attribute
{
}
} |
from the previous name. Following latest comments in dotnet#35315
Our normal custom attribute lookup method may trigger loading. This is undesirable during class initialization, for example. For this reason, we have `mono_class_metadata_foreach_custom_attr` that scans a class for custom attributes without triggering loading. This PR adds `mono_method_metadata_foreach_custom_attr` that does the same thing - looks for custom attributes on methods without causing any loader activity. This PR also reorganizes the code in `class-init.c` that uses these metadata custom attribute lookups to be a bit easier to use. Finally we add a convenience method (unused so far) to look for `PreserveBaseOverridesAttribute` which is defined in dotnet/runtime#35315 and which will be necessary to implement covariant returns.
) Our normal custom attribute lookup method may trigger loading. This is undesirable during class initialization, for example. For this reason, we have `mono_class_metadata_foreach_custom_attr` that scans a class for custom attributes without triggering loading. This PR adds `mono_method_metadata_foreach_custom_attr` that does the same thing - looks for custom attributes on methods without causing any loader activity. This PR also reorganizes the code in `class-init.c` that uses these metadata custom attribute lookups to be a bit easier to use. Finally we add a convenience method (unused so far) to look for `PreserveBaseOverridesAttribute` which is defined in dotnet/runtime#35315 and which will be necessary to implement covariant returns. Co-authored-by: lambdageek <[email protected]>
) * [metadata] Add mono_method_metadata_foreach_custom_attr Iterate over the custom attributes on a method in a way that's safe to use during class initialization. * [class-init] Generalize search for well-known attributes Just on classes or methods for now. Mark `method_has_require_methodimpl_to_remain_in_effect_attribute` with `G_GNUC_UNUSED` for now - it will be needed for covariant returns * [class-init] Fix debug message - it's not an iface override We're going over the virtual methods of the base class looking for ones that match the subclass. These are plain old virtual methods, not interface methods * rename to PreserveBaseOverridesAttribute from the previous name. Following latest comments in #35315
Fixed by #35308 |
This new attribute will be used by the covariant return types and records C# language features posed for C# 9.0.
Today when we have an explicit override to a method (using MethodImpls), this override gets a new vtable slot on the type, and the slot for the overriden method gets updated to point to the new overriding method. Example:
Now when a new derived type explicitly overrides
A.VirtualFunction
, it would also get a new vtable slot, and only the vtable slot of the method being overriden gets updated with the new override. Example:With the new records and covariant return feature, a change of behavior is required to vtable slot overrides when a method gets explictly overriden. In the example above, the second slot on type C would also need to be updated to point to the latest override of the method:
The reason for this change is to ensure that any virtual call to the method, whether it uses the base signature or derived signature of the method, we always execute the most derived override:
To achieve this, a new attribute will be introduced and applied to the MethodImpls that need to propagate to all applicable vtable slots:
RequireMethodImplToRemainInEffect
attribute. Example:This attribute will only be applicable to methods with explicit overrides (MethodImpls), and takes no arguments. The proposed namespace for this attribute is the
System.Runtime.CompilerServices
namespace. Here is the proposed implementation of the attribute:The text was updated successfully, but these errors were encountered: