Skip to content
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

[mono] Tracking: covariant return methods support #37509

Open
3 of 5 tasks
lambdageek opened this issue Jun 5, 2020 · 3 comments
Open
3 of 5 tasks

[mono] Tracking: covariant return methods support #37509

lambdageek opened this issue Jun 5, 2020 · 3 comments
Labels
area-VM-meta-mono help wanted [up-for-grabs] Good issue for external contributors runtime-mono specific to the Mono runtime tracking This issue is tracking the completion of other related issues.
Milestone

Comments

@lambdageek
Copy link
Member

lambdageek commented Jun 5, 2020

This is a tracking issue for the implementation of Covariant Return Methods Mono

Future

  • pass test cases for variant interfaces CovariantReturns/Interfaces/UnitTest.il and UnsupportedScenario1.il, UnsupportedScenario2.il and UnsupportedScenario3.il
  • Implement PreserveBaseBaseOverrideAttribute support for SRE images

.NET 5

@lambdageek lambdageek added Epic Groups multiple user stories. Can be grouped under a theme. area-VM-meta-mono runtime-mono specific to the Mono runtime tracking This issue is tracking the completion of other related issues. labels Jun 5, 2020
@lambdageek lambdageek self-assigned this Jun 5, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jun 5, 2020
@lambdageek lambdageek removed the untriaged New issue has not been triaged by the area owner label Jun 8, 2020
@lambdageek lambdageek added this to the 5.0 milestone Jun 8, 2020
@lambdageek lambdageek removed Epic Groups multiple user stories. Can be grouped under a theme. tracking This issue is tracking the completion of other related issues. labels Jun 15, 2020
@lambdageek
Copy link
Member Author

Looking increasingly likely that #37516 is going to have everything in one PR. The changes are pretty coherent, there's no need to land it in stages.

lambdageek added a commit to lambdageek/runtime that referenced this issue Jun 15, 2020
The attribute is expected to be applied to a `newslot virtual` method that
is an explicit .override of some other method.

Becuase the method (MethodImpl) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the MethodDecl that it is overriding.

If a third class subclasses the class with the MethodImpl or one of its
descendents and itself overrides either the MethodDecl or (one of the)
MethodImpl (either implicitly or with another explicit .override)  the
PreserveBaseOverridesAttribute is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "override_map" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

Contributes to dotnet#37509
monojenkins pushed a commit to monojenkins/mono that referenced this issue Jun 23, 2020
…nt returns support

Covariant return implementation for Mono.

---

There are also a couple of general Mono fixes in here, too:

1. Interpreter support for `MONO_TYPE_FNPTR`  in `mint_type()`
2. a "signature assignment" mode for `mono_class_is_assignable_from` that doesn't treat `IntPtr[]` and `long[]` (or `int[]`) as interchangeable
3. support for unmanaged pointer comparisons in `mono_class_is_assignable_from`
4. Factor `mono_byref_type_is_assignable_from` out of `ves_icall_RuntimeTypeHandle_type_is_assignable_from` and make it generally usable.

---

For covariant returns, there are a couple of pieces here:
1. Method overrides come in two flavors: implicit (matching by name and sig) and explicit (using a `.override` directive).
   * for explicit overrides, we have to check that the override has a signature that is subsumed by (its return type is a subtype of the return type of) the previous override, if any, and the declared method.
   * for implicit overrides, we have to check that the override has a signature that is subsumed by the previous most derived override (not just the one that we happened to find with a precisely matching signature).
2. If any override is marked by `[PreserveBaseOverridesAttribute]` then we need to find all the other slots that may have methods from the override chain and update them.  (The attribute is expected to be applied to a `newslot virtual` method that is an explicit `.override` of some other method).

There's an interesting test case where a class has _both_ an implicit and and explicit override for the same slot, arrived via different methods - in this case, the implicit one is supposed to be ignored.  We implement this by doing the implicit slot filling first, then the explicit, and saving the covariant return checking until after both are done.

---

Because the method (`MethodImpl`) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the `MethodDecl` that it is overriding.

If a third class subclasses the class with the `MethodImpl` or one of its
descendents and itself overrides either the `MethodDecl` or (one of the)
`MethodImpl` (either implicitly or with another explicit .override)  the
`PreserveBaseOverridesAttribute` is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "`override_map`" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

Contributes to dotnet/runtime#37509
lambdageek added a commit that referenced this issue Jun 26, 2020
…nt returns support (#37516)

Covariant return implementation for Mono.

---

There are also a couple of general Mono fixes in here, too:

1. Interpreter support for `MONO_TYPE_FNPTR`  in `mint_type()`
2. a "signature assignment" mode for `mono_class_is_assignable_from` that doesn't treat `IntPtr[]` and `long[]` (or `int[]`) as interchangeable
3. support for unmanaged pointer comparisons in `mono_class_is_assignable_from`
4. Factor `mono_byref_type_is_assignable_from` out of `ves_icall_RuntimeTypeHandle_type_is_assignable_from` and make it generally usable.

---

For covariant returns, there are a couple of pieces here:
1. Method overrides come in two flavors: implicit (matching by name and sig) and explicit (using a `.override` directive).
   * for explicit overrides, we have to check that the override has a signature that is subsumed by (its return type is a subtype of the return type of) the previous override, if any, and the declared method.
   * for implicit overrides, we have to check that the override has a signature that is subsumed by the previous most derived override (not just the one that we happened to find with a precisely matching signature).
2. If any override is marked by `[PreserveBaseOverridesAttribute]` then we need to find all the other slots that may have methods from the override chain and update them.  (The attribute is expected to be applied to a `newslot virtual` method that is an explicit `.override` of some other method).

There's an interesting test case where a class has _both_ an implicit and and explicit override for the same slot, arrived via different methods - in this case, the implicit one is supposed to be ignored.  We implement this by doing the implicit slot filling first, then the explicit, and saving the covariant return checking until after both are done.

---

Because the method (`MethodImpl`) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the `MethodDecl` that it is overriding.

If a third class subclasses the class with the `MethodImpl` or one of its
descendents and itself overrides either the `MethodDecl` or (one of the)
`MethodImpl` (either implicitly or with another explicit .override)  the
`PreserveBaseOverridesAttribute` is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "`override_map`" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

Contributes to #37509


* [metadata] Implement support for PreserveBaseOverridesAttribute

The attribute is expected to be applied to a `newslot virtual` method that
is an explicit .override of some other method.

Becuase the method (MethodImpl) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the MethodDecl that it is overriding.

If a third class subclasses the class with the MethodImpl or one of its
descendents and itself overrides either the MethodDecl or (one of the)
MethodImpl (either implicitly or with another explicit .override)  the
PreserveBaseOverridesAttribute is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "override_map" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

* add FIXME for dynamic images

* scan entire chain of inheritance, don't stop at the class with the attribute

   Makes OverrideMoreDerivedReturn.il pass

* UnitTestDelegates is also passing

* [metadata] Add mono_metadata_signature_equal_no_ret

* inflate signature->ret when doing covariant checking

* [class] Add mono_class_signature_is_assignable_from_checked

   In ECMA I.8.7.1 "assignment compatability for signature types" we need to
   distinguish IntPtr[] from int32[] and int64[].  The ordinary cast_class for an
   array type in Mono implements the "intermediate type" relation from I.8.7.3
   "general assignment compatability" - IntPtr[] gets treated as inter-castable
   with int32[] or int64[], depending on platform pointer size.

   The new mono_class_signature_is_assignable_from_checked function does the finer
   relation for signatures, while the old mono_class_signature_is_assignable_from_checked
   preserves the existing behavior used elsewhere in the runtime.

* Check every method previously in the slot against the new impl

* UnitTest_GVM passes

* [metadata] Don't infinite loop in mono_class_get_flags for a FNPTR

   When a MonoClass is a MONO_TYPE_FNPTR, the element_class is itself.  We should
   probably not do that, but meanwhile, make `mono_class_get_flags` return
   something else for function pointer types.

   Makes mono_class_is_interface not overflow the stack

* [metadata] Set MonoClass:cast_class for pointer types to the intermediate type

   Ignore signedness differences and set cast_class to the storage type, same as
   for arrays.  This is used by mono_class_is_assignable_from when comparing
   IntPtr* vs ulong*, for example.

* [metadata] Make mono_class_is_assignable_from work for unmanaged pointer types

   The issue is that we need to compare intermediate type or the reduced type of
   the element type, depending on whether we're doing compatability or general
   assignment compatability (upshot: ulong* and long* are always inter-assignable,
   but IntPtr* is not in signatures, but is for general storage)

* [metadata] Pull mono_type_byref_is_assignable_from out of ves_icall_RuntimeTypeHandle_type_is_assignable_from

   Make it a separate internal function

* [metadata] mono_type_byref_is_assignable_from for valuetypes and class types should check for identity

   X& and Y& where both are either a reference types or valuetypes are only
   assignable if X and Y are identical.  The old code was checking for identity
   for valuetypes, but for reference types it would allow any two reference types
   at all, which is incorrect.

* [class-init] Check for byref types in covariant return signatures

Lets us pass Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.il from the CoreCLR testsuite

* [interp] handle MONO_TYPE_FNPTR in mint_type

   It's just a pointer-sized integer, as far as the interpreter's type discipline is concerned

* [class-init] Use a bit for covariant return method impls

   Allows us to check implicit overrides against previous impl's sig when the previous impl potentially had a covariant return type.

   Also use the bit to decide if we need to check signatures of the entire impl chain for explicit instantiations

* ReturnTypeValidation unit tests work on Mono now

* [class-init] check covariant vtable slots after overloads are applied

   The check has to be done after both implicit and explicit overrides have been
   applied because some implicit overrides must be ignored.  If the check is done
   as soon as we detect that a covariant return method override is needed, we will
   incorrectly check implicit overrides that are to be ignored because a later
   explicit override applied to the same slot.

* [metadata] Fix byref IsAssignableFrom

   The reflection API has an interesting behavior, whereas the ECMA definition is
   stricter.

   ```
   public interface I {}
   
   public class B {}
   public class D : B, I {}
   
   public struct S : I { }
   
   class Program {
           static void Main(string[] args)
           {
   	    var tb = typeof (B).MakeByRefType ();
   	    var td = typeof (D).MakeByRefType ();
   	    var ti = typeof (I).MakeByRefType ();
   	    var ts = typeof (S).MakeByRefType ();
   
   	    Console.WriteLine ($"{tb.FullName} is assignable from {td.FullName} ? {tb.IsAssignableFrom (td) }");
   	    Console.WriteLine ($"{td.FullName} is assignable from {tb.FullName} ? {td.IsAssignableFrom (tb) }");
   
   	    Console.WriteLine ($"{ti.FullName} is assignable from {td.FullName} ? {ti.IsAssignableFrom (td) }");
   	    Console.WriteLine ($"{td.FullName} is assignable from {ti.FullName} ? {td.IsAssignableFrom (ti) }");
   
   	    Console.WriteLine ($"{ti.FullName} is assignable from {ts.FullName} ? {ti.IsAssignableFrom (ts) }");
   	    Console.WriteLine ($"{ts.FullName} is assignable from {ti.FullName} ? {ts.IsAssignableFrom (ti) }");
   
           }
   }
   
   // Expected Output:
   // B& is assignable from D& ? True
   // D& is assignable from B& ? False
   // I& is assignable from D& ? True
   // D& is assignable from I& ? False
   // I& is assignable from S& ? False
   // S& is assignable from I& ? False
   ```

* rename mono_class_signature_is_assignable

   No need for _checked suffix

* rename mono_byref_type_is_assignable_from

* [class-init] Move vtable setup to a separate file

   The code is getting quite long, move it to a separate file
lambdageek pushed a commit to mono/mono that referenced this issue Jun 26, 2020
…urns support (#19927)

Covariant return implementation for Mono.

---

There are also a couple of general Mono fixes in here, too:

1. Interpreter support for `MONO_TYPE_FNPTR`  in `mint_type()`
2. a "signature assignment" mode for `mono_class_is_assignable_from` that doesn't treat `IntPtr[]` and `long[]` (or `int[]`) as interchangeable
3. support for unmanaged pointer comparisons in `mono_class_is_assignable_from`
4. Factor `mono_byref_type_is_assignable_from` out of `ves_icall_RuntimeTypeHandle_type_is_assignable_from` and make it generally usable.

---

For covariant returns, there are a couple of pieces here:
1. Method overrides come in two flavors: implicit (matching by name and sig) and explicit (using a `.override` directive).
   * for explicit overrides, we have to check that the override has a signature that is subsumed by (its return type is a subtype of the return type of) the previous override, if any, and the declared method.
   * for implicit overrides, we have to check that the override has a signature that is subsumed by the previous most derived override (not just the one that we happened to find with a precisely matching signature).
2. If any override is marked by `[PreserveBaseOverridesAttribute]` then we need to find all the other slots that may have methods from the override chain and update them.  (The attribute is expected to be applied to a `newslot virtual` method that is an explicit `.override` of some other method).

There's an interesting test case where a class has _both_ an implicit and and explicit override for the same slot, arrived via different methods - in this case, the implicit one is supposed to be ignored.  We implement this by doing the implicit slot filling first, then the explicit, and saving the covariant return checking until after both are done.

---

Because the method (`MethodImpl`) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the `MethodDecl` that it is overriding.

If a third class subclasses the class with the `MethodImpl` or one of its
descendents and itself overrides either the `MethodDecl` or (one of the)
`MethodImpl` (either implicitly or with another explicit .override)  the
`PreserveBaseOverridesAttribute` is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "`override_map`" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

Contributes to dotnet/runtime#37509


* [metadata] Implement support for PreserveBaseOverridesAttribute

The attribute is expected to be applied to a `newslot virtual` method that
is an explicit .override of some other method.

Becuase the method (MethodImpl) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the MethodDecl that it is overriding.

If a third class subclasses the class with the MethodImpl or one of its
descendents and itself overrides either the MethodDecl or (one of the)
MethodImpl (either implicitly or with another explicit .override)  the
PreserveBaseOverridesAttribute is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "override_map" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

* add FIXME for dynamic images

* scan entire chain of inheritance, don't stop at the class with the attribute

   Makes OverrideMoreDerivedReturn.il pass

* UnitTestDelegates is also passing

* [metadata] Add mono_metadata_signature_equal_no_ret

* inflate signature->ret when doing covariant checking

* [class] Add mono_class_signature_is_assignable_from_checked

   In ECMA I.8.7.1 "assignment compatability for signature types" we need to
   distinguish IntPtr[] from int32[] and int64[].  The ordinary cast_class for an
   array type in Mono implements the "intermediate type" relation from I.8.7.3
   "general assignment compatability" - IntPtr[] gets treated as inter-castable
   with int32[] or int64[], depending on platform pointer size.

   The new mono_class_signature_is_assignable_from_checked function does the finer
   relation for signatures, while the old mono_class_signature_is_assignable_from_checked
   preserves the existing behavior used elsewhere in the runtime.

* Check every method previously in the slot against the new impl

* UnitTest_GVM passes

* [metadata] Don't infinite loop in mono_class_get_flags for a FNPTR

   When a MonoClass is a MONO_TYPE_FNPTR, the element_class is itself.  We should
   probably not do that, but meanwhile, make `mono_class_get_flags` return
   something else for function pointer types.

   Makes mono_class_is_interface not overflow the stack

* [metadata] Set MonoClass:cast_class for pointer types to the intermediate type

   Ignore signedness differences and set cast_class to the storage type, same as
   for arrays.  This is used by mono_class_is_assignable_from when comparing
   IntPtr* vs ulong*, for example.

* [metadata] Make mono_class_is_assignable_from work for unmanaged pointer types

   The issue is that we need to compare intermediate type or the reduced type of
   the element type, depending on whether we're doing compatability or general
   assignment compatability (upshot: ulong* and long* are always inter-assignable,
   but IntPtr* is not in signatures, but is for general storage)

* [metadata] Pull mono_type_byref_is_assignable_from out of ves_icall_RuntimeTypeHandle_type_is_assignable_from

   Make it a separate internal function

* [metadata] mono_type_byref_is_assignable_from for valuetypes and class types should check for identity

   X& and Y& where both are either a reference types or valuetypes are only
   assignable if X and Y are identical.  The old code was checking for identity
   for valuetypes, but for reference types it would allow any two reference types
   at all, which is incorrect.

* [class-init] Check for byref types in covariant return signatures

Lets us pass Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.il from the CoreCLR testsuite

* [interp] handle MONO_TYPE_FNPTR in mint_type

   It's just a pointer-sized integer, as far as the interpreter's type discipline is concerned

* [class-init] Use a bit for covariant return method impls

   Allows us to check implicit overrides against previous impl's sig when the previous impl potentially had a covariant return type.

   Also use the bit to decide if we need to check signatures of the entire impl chain for explicit instantiations

* ReturnTypeValidation unit tests work on Mono now

* [class-init] check covariant vtable slots after overloads are applied

   The check has to be done after both implicit and explicit overrides have been
   applied because some implicit overrides must be ignored.  If the check is done
   as soon as we detect that a covariant return method override is needed, we will
   incorrectly check implicit overrides that are to be ignored because a later
   explicit override applied to the same slot.

* [metadata] Fix byref IsAssignableFrom

   The reflection API has an interesting behavior, whereas the ECMA definition is
   stricter.

   ```
   public interface I {}
   
   public class B {}
   public class D : B, I {}
   
   public struct S : I { }
   
   class Program {
           static void Main(string[] args)
           {
   	    var tb = typeof (B).MakeByRefType ();
   	    var td = typeof (D).MakeByRefType ();
   	    var ti = typeof (I).MakeByRefType ();
   	    var ts = typeof (S).MakeByRefType ();
   
   	    Console.WriteLine ($"{tb.FullName} is assignable from {td.FullName} ? {tb.IsAssignableFrom (td) }");
   	    Console.WriteLine ($"{td.FullName} is assignable from {tb.FullName} ? {td.IsAssignableFrom (tb) }");
   
   	    Console.WriteLine ($"{ti.FullName} is assignable from {td.FullName} ? {ti.IsAssignableFrom (td) }");
   	    Console.WriteLine ($"{td.FullName} is assignable from {ti.FullName} ? {td.IsAssignableFrom (ti) }");
   
   	    Console.WriteLine ($"{ti.FullName} is assignable from {ts.FullName} ? {ti.IsAssignableFrom (ts) }");
   	    Console.WriteLine ($"{ts.FullName} is assignable from {ti.FullName} ? {ts.IsAssignableFrom (ti) }");
   
           }
   }
   
   // Expected Output:
   // B& is assignable from D& ? True
   // D& is assignable from B& ? False
   // I& is assignable from D& ? True
   // D& is assignable from I& ? False
   // I& is assignable from S& ? False
   // S& is assignable from I& ? False
   ```

* rename mono_class_signature_is_assignable

   No need for _checked suffix

* rename mono_byref_type_is_assignable_from

* [class-init] Move vtable setup to a separate file

   The code is getting quite long, move it to a separate file
kevinwkt pushed a commit to kevinwkt/runtimelab that referenced this issue Jul 15, 2020
…nt returns support (#37516)

Covariant return implementation for Mono.

---

There are also a couple of general Mono fixes in here, too:

1. Interpreter support for `MONO_TYPE_FNPTR`  in `mint_type()`
2. a "signature assignment" mode for `mono_class_is_assignable_from` that doesn't treat `IntPtr[]` and `long[]` (or `int[]`) as interchangeable
3. support for unmanaged pointer comparisons in `mono_class_is_assignable_from`
4. Factor `mono_byref_type_is_assignable_from` out of `ves_icall_RuntimeTypeHandle_type_is_assignable_from` and make it generally usable.

---

For covariant returns, there are a couple of pieces here:
1. Method overrides come in two flavors: implicit (matching by name and sig) and explicit (using a `.override` directive).
   * for explicit overrides, we have to check that the override has a signature that is subsumed by (its return type is a subtype of the return type of) the previous override, if any, and the declared method.
   * for implicit overrides, we have to check that the override has a signature that is subsumed by the previous most derived override (not just the one that we happened to find with a precisely matching signature).
2. If any override is marked by `[PreserveBaseOverridesAttribute]` then we need to find all the other slots that may have methods from the override chain and update them.  (The attribute is expected to be applied to a `newslot virtual` method that is an explicit `.override` of some other method).

There's an interesting test case where a class has _both_ an implicit and and explicit override for the same slot, arrived via different methods - in this case, the implicit one is supposed to be ignored.  We implement this by doing the implicit slot filling first, then the explicit, and saving the covariant return checking until after both are done.

---

Because the method (`MethodImpl`) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the `MethodDecl` that it is overriding.

If a third class subclasses the class with the `MethodImpl` or one of its
descendents and itself overrides either the `MethodDecl` or (one of the)
`MethodImpl` (either implicitly or with another explicit .override)  the
`PreserveBaseOverridesAttribute` is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "`override_map`" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

Contributes to dotnet/runtime#37509


* [metadata] Implement support for PreserveBaseOverridesAttribute

The attribute is expected to be applied to a `newslot virtual` method that
is an explicit .override of some other method.

Becuase the method (MethodImpl) is both an override of another virtual method,
and is itself a `newslot`, it will occupy (at least) 2 slots in the vtable -
its own newslot and the MethodDecl that it is overriding.

If a third class subclasses the class with the MethodImpl or one of its
descendents and itself overrides either the MethodDecl or (one of the)
MethodImpl (either implicitly or with another explicit .override)  the
PreserveBaseOverridesAttribute is a signal to apply the new override that it
should apply to all the slots.

The way this works is when processing the overrides, we walk up the class
hierarchy looking at the contents of the impl slot and if any of them have the
attribute, we make sure to add a mapping to the "override_map" to replace any
occurrences of a previous impl with the newest (most derived) impl.

An (existing) later pass goes through the vtable and applies the override_map.

* add FIXME for dynamic images

* scan entire chain of inheritance, don't stop at the class with the attribute

   Makes OverrideMoreDerivedReturn.il pass

* UnitTestDelegates is also passing

* [metadata] Add mono_metadata_signature_equal_no_ret

* inflate signature->ret when doing covariant checking

* [class] Add mono_class_signature_is_assignable_from_checked

   In ECMA I.8.7.1 "assignment compatability for signature types" we need to
   distinguish IntPtr[] from int32[] and int64[].  The ordinary cast_class for an
   array type in Mono implements the "intermediate type" relation from I.8.7.3
   "general assignment compatability" - IntPtr[] gets treated as inter-castable
   with int32[] or int64[], depending on platform pointer size.

   The new mono_class_signature_is_assignable_from_checked function does the finer
   relation for signatures, while the old mono_class_signature_is_assignable_from_checked
   preserves the existing behavior used elsewhere in the runtime.

* Check every method previously in the slot against the new impl

* UnitTest_GVM passes

* [metadata] Don't infinite loop in mono_class_get_flags for a FNPTR

   When a MonoClass is a MONO_TYPE_FNPTR, the element_class is itself.  We should
   probably not do that, but meanwhile, make `mono_class_get_flags` return
   something else for function pointer types.

   Makes mono_class_is_interface not overflow the stack

* [metadata] Set MonoClass:cast_class for pointer types to the intermediate type

   Ignore signedness differences and set cast_class to the storage type, same as
   for arrays.  This is used by mono_class_is_assignable_from when comparing
   IntPtr* vs ulong*, for example.

* [metadata] Make mono_class_is_assignable_from work for unmanaged pointer types

   The issue is that we need to compare intermediate type or the reduced type of
   the element type, depending on whether we're doing compatability or general
   assignment compatability (upshot: ulong* and long* are always inter-assignable,
   but IntPtr* is not in signatures, but is for general storage)

* [metadata] Pull mono_type_byref_is_assignable_from out of ves_icall_RuntimeTypeHandle_type_is_assignable_from

   Make it a separate internal function

* [metadata] mono_type_byref_is_assignable_from for valuetypes and class types should check for identity

   X& and Y& where both are either a reference types or valuetypes are only
   assignable if X and Y are identical.  The old code was checking for identity
   for valuetypes, but for reference types it would allow any two reference types
   at all, which is incorrect.

* [class-init] Check for byref types in covariant return signatures

Lets us pass Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest.il from the CoreCLR testsuite

* [interp] handle MONO_TYPE_FNPTR in mint_type

   It's just a pointer-sized integer, as far as the interpreter's type discipline is concerned

* [class-init] Use a bit for covariant return method impls

   Allows us to check implicit overrides against previous impl's sig when the previous impl potentially had a covariant return type.

   Also use the bit to decide if we need to check signatures of the entire impl chain for explicit instantiations

* ReturnTypeValidation unit tests work on Mono now

* [class-init] check covariant vtable slots after overloads are applied

   The check has to be done after both implicit and explicit overrides have been
   applied because some implicit overrides must be ignored.  If the check is done
   as soon as we detect that a covariant return method override is needed, we will
   incorrectly check implicit overrides that are to be ignored because a later
   explicit override applied to the same slot.

* [metadata] Fix byref IsAssignableFrom

   The reflection API has an interesting behavior, whereas the ECMA definition is
   stricter.

   ```
   public interface I {}
   
   public class B {}
   public class D : B, I {}
   
   public struct S : I { }
   
   class Program {
           static void Main(string[] args)
           {
   	    var tb = typeof (B).MakeByRefType ();
   	    var td = typeof (D).MakeByRefType ();
   	    var ti = typeof (I).MakeByRefType ();
   	    var ts = typeof (S).MakeByRefType ();
   
   	    Console.WriteLine ($"{tb.FullName} is assignable from {td.FullName} ? {tb.IsAssignableFrom (td) }");
   	    Console.WriteLine ($"{td.FullName} is assignable from {tb.FullName} ? {td.IsAssignableFrom (tb) }");
   
   	    Console.WriteLine ($"{ti.FullName} is assignable from {td.FullName} ? {ti.IsAssignableFrom (td) }");
   	    Console.WriteLine ($"{td.FullName} is assignable from {ti.FullName} ? {td.IsAssignableFrom (ti) }");
   
   	    Console.WriteLine ($"{ti.FullName} is assignable from {ts.FullName} ? {ti.IsAssignableFrom (ts) }");
   	    Console.WriteLine ($"{ts.FullName} is assignable from {ti.FullName} ? {ts.IsAssignableFrom (ti) }");
   
           }
   }
   
   // Expected Output:
   // B& is assignable from D& ? True
   // D& is assignable from B& ? False
   // I& is assignable from D& ? True
   // D& is assignable from I& ? False
   // I& is assignable from S& ? False
   // S& is assignable from I& ? False
   ```

* rename mono_class_signature_is_assignable

   No need for _checked suffix

* rename mono_byref_type_is_assignable_from

* [class-init] Move vtable setup to a separate file

   The code is getting quite long, move it to a separate file
@marek-safar marek-safar added the tracking This issue is tracking the completion of other related issues. label Aug 3, 2020
@marek-safar marek-safar modified the milestones: 5.0.0, 6.0.0 Aug 3, 2020
@SamMonoRT
Copy link
Member

Moving to 7.0.0

@SamMonoRT SamMonoRT modified the milestones: 6.0.0, 7.0.0 Jul 22, 2021
@SamMonoRT
Copy link
Member

moving tracking issues to 8.0.0

@SamMonoRT SamMonoRT modified the milestones: 7.0.0, 8.0.0 Aug 3, 2022
@lambdageek lambdageek modified the milestones: 8.0.0, 9.0.0 Aug 4, 2023
@steveisok steveisok modified the milestones: 9.0.0, Future Aug 8, 2024
@steveisok steveisok added the help wanted [up-for-grabs] Good issue for external contributors label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-VM-meta-mono help wanted [up-for-grabs] Good issue for external contributors runtime-mono specific to the Mono runtime tracking This issue is tracking the completion of other related issues.
Projects
None yet
Development

No branches or pull requests

5 participants