Covariant Returns Feature

docs/design/features/covariant-return-methods.md

@@ -0,0 +1,96 @@
+# Covariant Return Methods
+
+Covariant return methods is a runtime feature designed to support the [covariant return types](https://github.com/dotnet/csharplang/blob/master/proposals/covariant-returns.md) and [records](https://github.com/dotnet/csharplang/blob/master/proposals/records.md) C# language features posed for C# 9.0.
+
+This feature allows an overriding method to have a return type that is different than the one on the method it overrides, but compatible with it. The type compability rules are defined in ECMA I.8.7.1. Example: using a more derived return type. The only exception applicable to this rule for this feature is the compatibility between a value type and an interface that it implements, and this is due to the difference in ABI between reference types and value types (unboxed value types can be returned through a hidden return buffer parameter).
+
+Covariant return methods can only be described through MethodImpl records, and as an initial implementation will only be applicable to methods on reference types. Methods on interfaces and value types will not be supported (may be supported later in the future).
+
+MethodImpl checking will allow a return type to vary as long as the override is compatible with the return type of the method overriden (ECMA I.8.7.1).
+
+If a language wishes for the override to be semantically visible such that users of the more derived type may rely on the covariant return type it shall make the override a newslot method with appropriate visibility AND name to be used outside of the class.
+
+For virtual method slot MethodImpl overrides, each slot shall be checked for compatible signature on type load. (Implementation note: This behavior can be triggered only if the type has a covariant return type override in its hierarchy, so as to make this pay for play.)
+
+A new `RequireMethodImplToRemainInEffectAttribute` shall be added. The presence of this attribute is to require the type loader to ensure that the MethodImpl records specified on the method have not lost their slot unifying behavior due to other actions. This is used to allow the C# language to require that overrides have the consistent behaviors expected. The expectation is that C# would place this attribute on covariant override methods in classes.
+
+## Implementation Notes
+
+### Return Type Checking
+
+During enumeration of MethodImpls on a type (`MethodTableBuilder::EnumerateMethodImpls()`), if the signatures of the MethodImpl and the MethodDecl do not match:
+1. We repeat the signature comparison a second time, but skip the comparison of the return type signatures. If the signatures for the rest of the method arguments match, we will conditionally treat that MethodImpl as a valid one, but flag it for a closer examination of the return type compatibility at a later stage of type loading (end of `CLASS_LOAD_EXACTPARENTS` stage).
+2. At the end of the `CLASS_LOAD_EXACTPARENTS` type loading stage, examing each virtual method on the type, and if it has been flagged for further return type checking:
+    + Load the `TypeHandle` of the return type of the method on base type.
+    + Load the `TypeHandle` of the return type of the method on the current type being validated.
+    + Verify that the second `TypeHandle` is compatible with the first `TypeHandle` using the `MethodTable::CanCastTo()` API. If they are not compatible, a TypeLoadException is thrown.
+
+Once a method is flagged for return type checking, every time the vtable slot containing that method gets overridden on a derived type, the new override will also be checked for compatiblity. This is to ensure that no derived type can implicitly override some virtual method that has already been overridden by some MethodImpl with a covariant return type.
+
+### VTable Slot Unification
+
+If a MethodImpl has the `RequireMethodImplToRemainInEffectAttribute` attribute, it needs to propagate all applicable vtable slots on the type. This is to ensure that if we use the signature of one of the base type methods to call the overriding method, we still execute the overriding method.
+
+Consider this case:
+``` C#
+     class A {
+         RetType VirtualFunction() { }
+     }
+     class B : A {
+         [RequireMethodImplToRemainInEffect]
+         DerivedRetType VirtualFunction() { .override A.VirtualFuncion }
+     }
+     class C : B {
+         [RequireMethodImplToRemainInEffect]
+         MoreDerivedRetType VirtualFunction() { .override A.VirtualFunction }
+     }
+```
+
+Given an object of type `C`, the attribute will ensure that:
+``` C#
+     callvirt RetType A::VirtualFunc()               -> executes the MethodImpl on C
+     callvirt DerivedRetType B::VirtualFunc()        -> executes the MethodImpl on C
+     callvirt MoreDerivedRetType C::VirtualFunc()    -> executes the MethodImpl on C
+```
+
+Without the attribute, the second callvirt would normally execute the MethodImpl on `B` (the MethodImpl on `C` does not override the vtable slot of `B`'s MethodImpl, but only overrides the declaring method's vtable slot.
+
+This slot unification step will also take place during the last step of type loading (end of `CLASS_LOAD_EXACTPARENTS` stage).
+
+### [Future] Interface Support
+
+An interface method may be both non-final and have a MethodImpl that declares that it overrides another interface method. If it does, NO other interface method may .override it. Instead further overrides must override the method that it overrode. Also the overriding method may only override 1 method.
+
+The default interface method resolution algorithm shall change from:
+
+``` console
+Given interface method M and type T.
+Let MSearch = M
+Let MFound = Most specific implementation within the interfaces for MSearch within type T. If multiple implementations are found, throw Ambiguous match exception.
+Return MFound
+```
+
+To:
+
+``` console
+Given interface method M and type T.
+Let MSearch = M
+
+If (MSearch overrides another method MBase)
+    Let MSearch = MBase
+
+Let MFound = Most specific implementation within the interfaces for MSearch within type T. If multiple implementations are found, throw Ambiguous match exception.
+Let M Code = NULL
+
+If ((MFound != Msearch) and (MFound is not final))
+    Let M ClassVirtual = ResolveInterfaceMethod for MFound to virtual override on class T without using Default interface method implementation or return NULL if not found.
+    If (M ClassVirtual != NULL)
+        Let M Code= ResolveVirtualMethod for MFound on class T to implementation method
+
+If (M Code != NULL)
+    Let M Code = MFound
+
+Check M Code For signature <compatible-with> interface method M.
+
+Return M Code
+```

src/coreclr/src/dlls/mscorrc/mscorrc.rc

@@ -421,6 +421,7 @@ BEGIN
     IDS_CLASSLOAD_MI_BODY_DECL_MISMATCH     "Signature of the body and declaration in a method implementation do not match.  Type: '%1'.  Assembly: '%2'."
     IDS_CLASSLOAD_MI_MISSING_SIG_BODY       "Signature for the body in a method implementation cannot be found.  Type: '%1'.  Assembly: '%2'."
     IDS_CLASSLOAD_MI_MISSING_SIG_DECL       "Signature for the declaration in a method implementation cannot be found.  Type: '%1'.  Assembly: '%2'."
+    IDS_CLASSLOAD_MI_BADRETURNTYPE          "Return type in method '%1' on type '%2' from assembly '%3' is not compatible with base type method '%4'."
 
     IDS_CLASSLOAD_EQUIVALENTSTRUCTMETHODS   "Could not load the structure '%1' from assembly '%2'. The structure is marked as eligible for type equivalence, but it has a method."
     IDS_CLASSLOAD_EQUIVALENTSTRUCTFIELDS    "Could not load the structure '%1' from assembly '%2'. The structure is marked as eligible for type equivalence, but it has a static or non-public field."

src/coreclr/src/dlls/mscorrc/resource.h

@@ -167,6 +167,7 @@
 #define IDS_CLASSLOAD_MI_BODY_DECL_MISMATCH     0x17a5
 #define IDS_CLASSLOAD_MI_MISSING_SIG_BODY       0x17a6
 #define IDS_CLASSLOAD_MI_MISSING_SIG_DECL       0x17a7
+#define IDS_CLASSLOAD_MI_BADRETURNTYPE          0x17a8
 
 #define IDS_CLASSLOAD_TOOMANYGENERICARGS        0x17ab
 #define IDS_ERROR                               0x17b0

src/coreclr/src/vm/class.cpp

@@ -967,12 +967,186 @@ void ClassLoader::LoadExactParents(MethodTable *pMT)
 
     MethodTableBuilder::CopyExactParentSlots(pMT, pApproxParentMT);
 
+    ValidateMethodsWithCovariantReturnTypes(pMT);
+
     // We can now mark this type as having exact parents
     pMT->SetHasExactParent();
 
     RETURN;
 }
 
+/*static*/
+void ClassLoader::ValidateMethodsWithCovariantReturnTypes(MethodTable* pMT)
+{
+    CONTRACTL
+    {
+        STANDARD_VM_CHECK;
+        PRECONDITION(CheckPointer(pMT));
+    }
+    CONTRACTL_END;
+
+    //
+    // Validation for methods with covariant return types is a two step process.
+    //
+    // The first step is to validate that the return types on overriding methods with covariant return types are
+    // compatible with the return type of the method being overridden. Compatibility rules are defined by
+    // ECMA I.8.7.1, which is what the CanCastTo() API checks.
+    //
+    // The second step is to propagate an overriding MethodImpl to all applicable vtable slots if the MethodImpl
+    // has the RequireMethodImplToRemainInEffect attribute. This is to ensure that if we use the signature of one of
+    // the base type methods to call the overriding method, we still execute the overriding method.
+    //
+    // Consider this case:
+    //
+    //      class A {
+    //          RetType VirtualFunction() { }
+    //      }
+    //      class B : A {
+    //          [RequireMethodImplToRemainInEffect]
+    //          DerivedRetType VirtualFunction() { .override A.VirtualFuncion }
+    //      }
+    //      class C : B {
+    //          MoreDerivedRetType VirtualFunction() { .override A.VirtualFunction }
+    //      }
+    //
+    // NOTE: Typically the attribute would be added to the MethodImpl on C, but was omitted in this example to
+    //       illustrate how its presence on a MethodImpl on the base type can propagate as well. In other words,
+    //       think of it as applying to the vtable slot itself, so any MethodImpl that overrides this slot on a
+    //       derived type will propagate to all other applicable vtable slots.
+    //
+    // Given an object of type C, the attribute will ensure that:
+    //      callvirt RetType A::VirtualFunc()               -> executes the MethodImpl on C
+    //      callvirt DerivedRetType B::VirtualFunc()        -> executes the MethodImpl on C
+    //      callvirt MoreDerivedRetType C::VirtualFunc()    -> executes the MethodImpl on C
+    //
+    // Without the attribute, the second callvirt would normally execute the MethodImpl on B (the MethodImpl on
+    // C does not override the vtable slot of B's MethodImpl, but only overrides the declaring method's vtable slot.
+    //
+
+    // Validation not applicable to interface types and value types, since these are not currently
+    // supported with the covariant return feature
+    if (pMT->IsInterface() || pMT->IsValueType())
+        return;
+
+    MethodTable* pParentMT = pMT->GetParentMethodTable();
+    if (pParentMT == NULL)
+        return;
+
+    // Step 1: Validate compatibility of return types on overriding methods
+
+    for (WORD i = 0; i < pParentMT->GetNumVirtuals(); i++)
+    {
+        MethodDesc* pMD = pMT->GetMethodDescForSlot(i);
+        MethodDesc* pParentMD = pParentMT->GetMethodDescForSlot(i);
+
+        if (pMD == pParentMD)
+            continue;
+
+        if (!pMD->RequiresCovariantReturnTypeChecking() && !pParentMD->RequiresCovariantReturnTypeChecking())
+            continue;
+
+        // If the bit is not set on this method, but we reach here because it's been set on the method at the same slot on
+        // the base type, set the bit for the current method to ensure any future overriding method down the chain gets checked.
+        if (!pMD->RequiresCovariantReturnTypeChecking())
+            pMD->SetRequiresCovariantReturnTypeChecking();
+
+        // The context used to load the return type of the parent method has to use the generic method arguments
+        // of the overriding method, otherwise the type comparison below will not work correctly
+        SigTypeContext context1(pParentMD->GetClassInstantiation(), pMD->GetMethodInstantiation());
+        MetaSig methodSig1(pParentMD);
+        TypeHandle hType1 = methodSig1.GetReturnProps().GetTypeHandleThrowing(pParentMD->GetModule(), &context1, ClassLoader::LoadTypesFlag::LoadTypes, CLASS_LOAD_EXACTPARENTS);
+
+        SigTypeContext context2(pMD);
+        MetaSig methodSig2(pMD);
+        TypeHandle hType2 = methodSig2.GetReturnProps().GetTypeHandleThrowing(pMD->GetModule(), &context2, ClassLoader::LoadTypesFlag::LoadTypes, CLASS_LOAD_EXACTPARENTS);
+
+        _ASSERTE(hType1.GetMethodTable() != NULL);
+        _ASSERTE(hType2.GetMethodTable() != NULL);
+
+        {
+            GCX_COOP();
+
+            TypeHandlePairList visited(hType1, hType2, NULL);
+            if (!hType2.GetMethodTable()->CanCastTo(hType1.GetMethodTable(), &visited))
+            {
+                SString strAssemblyName;
+                pMD->GetAssembly()->GetDisplayName(strAssemblyName);
+
+                SString strInvalidTypeName;
+                TypeString::AppendType(strInvalidTypeName, TypeHandle(pMD->GetMethodTable()));
+
+                SString strInvalidMethodName;
+                TypeString::AppendMethod(strInvalidMethodName, pMD, pMD->GetMethodInstantiation());
+
+                SString strParentMethodName;
+                TypeString::AppendMethod(strParentMethodName, pParentMD, pParentMD->GetMethodInstantiation());
+
+                COMPlusThrow(
+                    kTypeLoadException,
+                    IDS_CLASSLOAD_MI_BADRETURNTYPE,
+                    strInvalidMethodName,
+                    strInvalidTypeName,
+                    strAssemblyName,
+                    strParentMethodName);
+            }
+        }
+    }
+
+    // Step 2: propate overriding MethodImpls to applicable vtable slots if the declaring method has the attribute
+
+    MethodTable::MethodDataWrapper hMTData(MethodTable::GetMethodData(pMT, FALSE));
+
+    for (WORD i = 0; i < pParentMT->GetNumVirtuals(); i++)
+    {
+        MethodDesc* pMD = pMT->GetMethodDescForSlot(i);
+        MethodDesc* pParentMD = pParentMT->GetMethodDescForSlot(i);
+        if (pMD == pParentMD)
+            continue;
+
+        // The attribute is only applicable to MethodImpls. For anything else, it will be treated as a no-op
+        if (!pMD->IsMethodImpl())
+            continue;
+
+        // Search if the attribute has been applied on this vtable slot, either by the current MethodImpl, or by a previous
+        // MethodImpl somewhere in the base type hierarchy.
+        bool foundAttribute = false;
+        MethodTable* pCurrentMT = pMT;
+        while (!foundAttribute && pCurrentMT != NULL && i < pCurrentMT->GetNumVirtuals())
+        {
+            MethodDesc* pCurrentMD = pCurrentMT->GetMethodDescForSlot(i);
+
+            // The attribute is only applicable to MethodImpls. For anything else, it will be treated as a no-op
+            if (pCurrentMD->IsMethodImpl())
+            {
+                BYTE* pVal = NULL;
+                ULONG cbVal = 0;
+                if (pCurrentMD->GetCustomAttribute(WellKnownAttribute::RequireMethodImplToRemainInEffectAttribute, (const void**)&pVal, &cbVal) == S_OK)
+                    foundAttribute = true;
+            }
+
+            pCurrentMT = pCurrentMT->GetParentMethodTable();
+        }
+
+        if (!foundAttribute)
+            continue;
+
+        // Search for any vtable slot still pointing at the parent method, and update it with the current overriding method
+        for (WORD j = i; j < pParentMT->GetNumVirtuals(); j++)
+        {
+            MethodDesc* pCurrentMD = pMT->GetMethodDescForSlot(j);
+            if (pCurrentMD == pParentMD)
+            {
+                // This is a vtable slot that needs to be updated to the new overriding method because of the
+                // presence of the attribute.
+                pMT->SetSlot(j, pMT->GetSlot(i));
+                _ASSERT(pMT->GetMethodDescForSlot(j) == pMD);
+
+                hMTData->UpdateImplMethodDesc(pMD, j);
+            }
+        }
+    }
+}
+
 //*******************************************************************************
 // This is the routine that computes the internal type of a given type.  It normalizes
 // structs that have only one field (of int/ptr sized values), to be that underlying type.

src/coreclr/src/vm/classcompat.cpp

@@ -1405,7 +1405,7 @@ VOID MethodTableBuilder::BuildInteropVTable_PlaceVtableMethods(
                                     bmtType->pModule, NULL,
                                     pInterfaceMethodSig,
                                     cInterfaceMethodSig,
-                                    pInterfaceMD->GetModule(), NULL))
+                                    pInterfaceMD->GetModule(), NULL, FALSE))
                                 {   // Found match, break from loop
                                     break;
                                 }
@@ -2263,7 +2263,8 @@ VOID    MethodTableBuilder::EnumerateMethodImpls()
                                                 pSigBody,
                                                 cbSigBody,
                                                 bmtType->pModule,
-                                                NULL))
+                                                NULL,
+                                                FALSE))
                 {
                     BuildMethodTableThrowException(IDS_CLASSLOAD_MI_BODY_DECL_MISMATCH);
                 }

src/coreclr/src/vm/clsload.hpp

@@ -969,10 +969,12 @@ class ClassLoader
 
     // Phase CLASS_LOAD_EXACTPARENTS of class loading
     // Load exact parents and interfaces and dependent structures (generics dictionary, vtable fixes)
-    static void LoadExactParents(MethodTable *pMT);
+    static void LoadExactParents(MethodTable* pMT);
 
     static void LoadExactParentAndInterfacesTransitively(MethodTable *pMT);
 
+    static void ValidateMethodsWithCovariantReturnTypes(MethodTable* pMT);
+
     // Create a non-canonical instantiation of a generic type based off the canonical instantiation
     // (For example, MethodTable for List<string> is based on the MethodTable for List<__Canon>)
     static TypeHandle CreateTypeHandleForNonCanonicalGenericInstantiation(TypeKey *pTypeKey,

src/coreclr/src/vm/ecall.cpp

@@ -349,7 +349,7 @@ static INT FindECIndexForMethod(MethodDesc *pMD, const LPVOID* impls)
 
             //@GENERICS: none of these methods belong to generic classes so there is no instantiation info to pass in
             if (!MetaSig::CompareMethodSigs(pMethodSig, cbMethodSigLen, pModule, NULL,
-                                            sig.GetRawSig(), sig.GetRawSigLen(), MscorlibBinder::GetModule(), NULL))
+                                            sig.GetRawSig(), sig.GetRawSigLen(), MscorlibBinder::GetModule(), NULL, FALSE))
             {
                 continue;
             }