[WIP] Test PR

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

@@ -0,0 +1,74 @@
+# 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 more derived reference type than the method it overrides. Covariant return methods can only be described through MethodImpl records, and as an initial implementation, will have the following limitations:
+1. Covariant return methods will only be applicable to methods on reference types: the MethodDecl and MethodImpl records can only be on reference types. Methods on interfaces will not be supported.
+2. Return types in covariant return methods can only be reference types: covariant interface return types are not supported.
+
+Supporting interfaces comes with many complications (ex: interface equivalence, default interface methods, variance on generic interfaces, etc...), which is why the feature will initially only support classes.
+
+MethodImpl checking will allow a return type to vary as long as the override is compatible with the return type of the method overriden (i.e. a derived type).
+
+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 `ValidateMethodImplRemainsInEffectAttribute` 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. In other words, when a MethodImpl on type A overrides some method using a derived return type in the signature, any type deriving from A will be allowed to have a MethodImpl record that overrides the same method as long as the return type used in the signature is the same or more derived than the return type used in the MethodImpl signature on type A. 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
+
+### Signature Checking
+
+Signature checking for MethodImpl is done through the `MetaSig::CompareElementType` method, which is called from various places in the runtime when comparing method signatures. This method compares the signatures of two types, and will now take a boolean flag that would allow for derived type checking behavior. The boolean flag will be set to `TRUE` appropriately during comparison of the return type signatures between a MethodImpl and MethodDecl records.
+
+The type signature checking algorithm will perform the following:
+1. Traverse and compare the signatures for `type1` and `type2` recursively.
+2. If the signatures mismatch at any given point, and the current element type for `type2` is `ELEMENT_TYPE_CLASS` or `ELEMENT_TYPE_GENERICINST`:
+    + Check for covariant return eligibility
+    + Compute the parent type's signature and parent type's generic substitution of `type2`
+    + Perform a recursive call to re-compare `type1` with the new parent type signature of `type2`.
+
+Note: if `ELEMENT_TYPE_INTERNAL` is encountered in either of the type signatures, both types will be fully loaded and compared for compatibility.
+
+### VTable Slot Checking
+
+This will be done during the call to `SetupMethodTable2` where we propagate inheritance (see methodtablebuilder.cpp around line 10642). TODO: add algorithm description here.
+
+### [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/inc/sigbuilder.h

@@ -74,6 +74,8 @@ class SigBuilder
     }
 
     void AppendBlob(const PVOID pBlob, SIZE_T cbBlob);
+
+    void AppendSignature(const PCCOR_SIGNATURE pSig, const PCCOR_SIGNATURE pSigEnd);
 };
 
 #endif // _SIGBUILDER_H_

src/coreclr/src/utilcode/sigbuilder.cpp

@@ -119,6 +119,22 @@ void SigBuilder::AppendBlob(const PVOID pBlob, SIZE_T cbBlob)
     m_dwLength += (DWORD)cbBlob;
 }
 
+void SigBuilder::AppendSignature(const PCCOR_SIGNATURE pSig, const PCCOR_SIGNATURE pSigEnd)
+{
+    STANDARD_VM_CONTRACT;
+
+    // Overflow checks
+    if (pSigEnd < pSig)
+        ThrowOutOfMemory();
+
+    DWORD cbSig = (DWORD)(pSigEnd - pSig);
+
+    Ensure(cbSig);
+    memcpy(&m_pBuffer[m_dwLength], pSig, cbSig);
+
+    m_dwLength += cbSig;
+}
+
 void SigBuilder::Grow(SIZE_T cbMin)
 {
     STANDARD_VM_CONTRACT;

src/coreclr/src/vm/class.cpp

@@ -967,12 +967,90 @@ void ClassLoader::LoadExactParents(MethodTable *pMT)
 
     MethodTableBuilder::CopyExactParentSlots(pMT, pApproxParentMT);
 
+    if (!pMT->IsInterface() && !pMT->IsValueType())
+    {
+        ValidateMethodImplRemainsInEffect(pMT);
+    }
+
     // We can now mark this type as having exact parents
     pMT->SetHasExactParent();
 
     RETURN;
 }
 
+/*static*/
+void ClassLoader::ValidateMethodImplRemainsInEffect(MethodTable* pMT)
+{
+    CONTRACT_VOID
+    {
+        STANDARD_VM_CHECK;
+        PRECONDITION(CheckPointer(pMT));
+        PRECONDITION(!pMT->IsInterface() && !pMT->IsValueType());
+    }
+    CONTRACT_END;
+
+    MethodTable* pParentMT = pMT->GetParentMethodTable();
+    if (pParentMT == NULL)
+        RETURN;
+
+    BYTE* pVal = NULL;
+    ULONG cbVal = 0;
+    HRESULT hr = pMT->GetCustomAttribute(WellKnownAttribute::ValidateMethodImplRemainsInEffectAttribute, (const void**)&pVal, &cbVal);
+    if (hr != S_OK)
+        RETURN;
+
+    for (WORD i = 0; i < pParentMT->GetNumVirtuals(); i++)
+    {
+        MethodDesc* pMD = pMT->GetMethodDescForSlot(i);
+        MethodDesc* pParentMD = pParentMT->GetMethodDescForSlot(i);
+
+        DWORD originalIndex = pMD->GetSlot();
+        if (originalIndex == i)
+            continue;
+
+        DWORD originalIndexParent = pParentMD->GetSlot();
+        if (originalIndex == originalIndexParent)
+            continue;
+
+        // 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);
+
+        // Type1 has to be equal to Type2, or a base type of Type2 (covariant returns)
+
+        if (!MetaSig::CompareTypeHandles(hType1, hType2, TRUE /* allowDerivedClass */ ))
+        {
+            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);
+        }
+    }
+
+    RETURN;
+}
+
 //*******************************************************************************
 // 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 ValidateMethodImplRemainsInEffect(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;
             }

src/coreclr/src/vm/memberload.cpp

@@ -243,7 +243,7 @@ void MemberLoader::GetDescFromMemberRef(Module * pModule,
 
             pMethodDef->GetSig(&pMethodSig, &cMethodSig);
             if (!MetaSig::CompareMethodSigs(pSig, cSig, pModule, NULL, pMethodSig,
-                                            cMethodSig, pModule, NULL))
+                                            cMethodSig, pModule, NULL, FALSE))
             {
                 // If the signatures do not match, then the correct MethodDesc has not been found.
                 fMissingMethod = TRUE;
@@ -1043,7 +1043,7 @@ BOOL CompareMethodSigWithCorrectSubstitution(
 
         pCurDeclMD->GetSig(&pCurMethodSig, &cCurMethodSig);
         return MetaSig::CompareMethodSigs(pSignature, cSignature, pModule, NULL, pCurMethodSig,
-                                       cCurMethodSig, pCurDeclMD->GetModule(), pDefSubst);
+                                       cCurMethodSig, pCurDeclMD->GetModule(), pDefSubst, FALSE);
     }
     else
     {
@@ -1443,7 +1443,7 @@ MemberLoader::FindConstructor(MethodTable * pMT, PCCOR_SIGNATURE pSignature,DWOR
         DWORD cCurMethodSig;
         pCurMethod->GetSig(&pCurMethodSig, &cCurMethodSig);
 
-        if (MetaSig::CompareMethodSigs(pSignature, cSignature, pModule, NULL, pCurMethodSig, cCurMethodSig, pCurMethod->GetModule(), NULL))
+        if (MetaSig::CompareMethodSigs(pSignature, cSignature, pModule, NULL, pCurMethodSig, cCurMethodSig, pCurMethod->GetModule(), NULL, FALSE))
         {
             return pCurMethod;
         }