Handle open types to appear in interface maps (#97733)

- Reflection IsAssignableFrom api - As well as constraint checking
dotnet · Feb 8, 2024 · aa4df1f · aa4df1f
1 parent 7f4702a
commit aa4df1f
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 12 deletions.
diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp
@@ -1324,7 +1324,7 @@ BOOL MethodTable::CanCastToInterface(MethodTable *pTargetMT, TypeHandlePairList
         if (CanCastByVarianceToInterfaceOrDelegate(pTargetMT, pVisited))
             return TRUE;
 
-        if (pTargetMT->IsSpecialMarkerTypeForGenericCasting())
+        if (pTargetMT->IsSpecialMarkerTypeForGenericCasting() && !GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap())
             return FALSE; // The special marker types cannot be cast to (at this time, they are the open generic types, so they are however, valid input to this method).
 
         InterfaceMapIterator it = IterateInterfaceMap();
@@ -1360,7 +1360,7 @@ BOOL MethodTable::CanCastByVarianceToInterfaceOrDelegate(MethodTable *pTargetMT,
 
     // Shortcut for generic approx type scenario
     if (pMTInterfaceMapOwner != NULL &&
-        !pMTInterfaceMapOwner->ContainsGenericVariables() &&
+        !pMTInterfaceMapOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap() &&
         IsSpecialMarkerTypeForGenericCasting() &&
         GetTypeDefRid() == pTargetMT->GetTypeDefRid() &&
         GetModule() == pTargetMT->GetModule() &&
@@ -1387,7 +1387,7 @@ BOOL MethodTable::CanCastByVarianceToInterfaceOrDelegate(MethodTable *pTargetMT,
         for (DWORD i = 0; i < inst.GetNumArgs(); i++)
         {
             TypeHandle thArg = inst[i];
-            if (IsSpecialMarkerTypeForGenericCasting() && pMTInterfaceMapOwner && !pMTInterfaceMapOwner->ContainsGenericVariables())
+            if (IsSpecialMarkerTypeForGenericCasting() && pMTInterfaceMapOwner && !pMTInterfaceMapOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap())
             {
                 thArg = pMTInterfaceMapOwner;
             }
@@ -1846,7 +1846,7 @@ NOINLINE BOOL MethodTable::ImplementsInterface(MethodTable *pInterface)
 {
     WRAPPER_NO_CONTRACT;
 
-    if (pInterface->IsSpecialMarkerTypeForGenericCasting())
+    if (pInterface->IsSpecialMarkerTypeForGenericCasting() && !GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap())
         return FALSE; // The special marker types cannot be cast to (at this time, they are the open generic types, so they are however, valid input to this method).
 
     return ImplementsInterfaceInline(pInterface);
@@ -1863,7 +1863,7 @@ BOOL MethodTable::ImplementsEquivalentInterface(MethodTable *pInterface)
     }
     CONTRACTL_END;
 
-    if (pInterface->IsSpecialMarkerTypeForGenericCasting())
+    if (pInterface->IsSpecialMarkerTypeForGenericCasting() && !GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap())
         return FALSE; // The special marker types cannot be cast to (at this time, they are the open generic types, so they are however, valid input to this method).
 
     // look for exact match first (optimize for success)
@@ -9298,12 +9298,13 @@ PTR_MethodTable MethodTable::InterfaceMapIterator::GetInterface(MethodTable* pMT
         GC_TRIGGERS;
         THROWS;
         PRECONDITION(m_i != (DWORD) -1 && m_i < m_count);
+        PRECONDITION(CheckPointer(pMTOwner));
         POSTCONDITION(CheckPointer(RETVAL));
     }
     CONTRACT_END;
 
     MethodTable *pResult = m_pMap->GetMethodTable();
-    if (pResult->IsSpecialMarkerTypeForGenericCasting() && !pMTOwner->ContainsGenericVariables())
+    if (pResult->IsSpecialMarkerTypeForGenericCasting() && !pMTOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap())
     {
         TypeHandle ownerAsInst[MaxGenericParametersForSpecialMarkerType];
         for (DWORD i = 0; i < MaxGenericParametersForSpecialMarkerType; i++)

diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h
@@ -318,7 +318,7 @@ struct MethodTableAuxiliaryData
         enum_flag_IsNotFullyLoaded          = 0x0040,
         enum_flag_DependenciesLoaded        = 0x0080,     // class and all dependencies loaded up to CLASS_LOADED_BUT_NOT_VERIFIED
 
-        // enum_unused                      = 0x0100,
+        enum_flag_MayHaveOpenInterfaceInInterfaceMap = 0x0100,
         // enum_unused                      = 0x0200,
         // enum_unused                      = 0x0400,
         // enum_unused                      = 0x0800,
@@ -431,6 +431,17 @@ struct MethodTableAuxiliaryData
         m_offsetToNonVirtualSlots = offset;
     }
 
+    inline void SetMayHaveOpenInterfacesInInterfaceMap()
+    {
+        LIMITED_METHOD_CONTRACT;
+        InterlockedOr((LONG*)&m_dwFlags, MethodTableAuxiliaryData::enum_flag_MayHaveOpenInterfaceInInterfaceMap);
+    }
+
+    inline bool MayHaveOpenInterfacesInInterfaceMap() const
+    {
+        return !!(m_dwFlags & MethodTableAuxiliaryData::enum_flag_MayHaveOpenInterfaceInInterfaceMap);
+    }
+
     static inline PTR_GenericsStaticsInfo GetGenericStaticsInfo(PTR_Const_MethodTableAuxiliaryData pAuxiliaryData)
     {
         return dac_cast<PTR_GenericsStaticsInfo>(dac_cast<TADDR>(pAuxiliaryData) - sizeof(GenericsStaticsInfo));
@@ -1940,7 +1951,7 @@ class MethodTable
                 if (pCurrentMethodTable->HasSameTypeDefAs(pMT) &&
                     pMT->HasInstantiation() &&
                     pCurrentMethodTable->IsSpecialMarkerTypeForGenericCasting() &&
-                    !pMTOwner->ContainsGenericVariables() &&
+                    !pMTOwner->GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap() &&
                     pMT->GetInstantiation().ContainsAllOneType(pMTOwner))
                 {
                     exactMatch = true;

diff --git a/src/coreclr/vm/methodtable.inl b/src/coreclr/vm/methodtable.inl
@@ -1367,7 +1367,6 @@ FORCEINLINE BOOL MethodTable::ImplementsInterfaceInline(MethodTable *pInterface)
         NOTHROW;
         GC_NOTRIGGER;
         PRECONDITION(pInterface->IsInterface()); // class we are looking up should be an interface
-        PRECONDITION(!pInterface->IsSpecialMarkerTypeForGenericCasting());
     }
     CONTRACTL_END;
 
@@ -1400,7 +1399,7 @@ FORCEINLINE BOOL MethodTable::ImplementsInterfaceInline(MethodTable *pInterface)
     while (--numInterfaces);
 
     // Second scan, looking for the curiously recurring generic scenario
-    if (pInterface->HasInstantiation() && !ContainsGenericVariables() && pInterface->GetInstantiation().ContainsAllOneType(this))
+    if (pInterface->HasInstantiation() && !GetAuxiliaryData()->MayHaveOpenInterfacesInInterfaceMap() && pInterface->GetInstantiation().ContainsAllOneType(this))
     {
         numInterfaces = GetNumInterfaces();
         pInfo = GetInterfaceMap();

diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp
@@ -9365,6 +9365,10 @@ MethodTableBuilder::LoadExactInterfaceMap(MethodTable *pMT)
     // to represent a type instantiated over its own generic variables, and the special marker type is currently the open type
     // and we make this case distinguishable by simply disallowing the optimization in those cases.
     bool retryWithExactInterfaces = !pMT->IsValueType() || pMT->IsSharedByGenericInstantiations() || pMT->ContainsGenericVariables();
+    if (retryWithExactInterfaces)
+    {
+        pMT->GetAuxiliaryDataForWrite()->SetMayHaveOpenInterfacesInInterfaceMap();
+    }
 
     DWORD nAssigned = 0;
     do
@@ -9431,6 +9435,7 @@ MethodTableBuilder::LoadExactInterfaceMap(MethodTable *pMT)
 
         if (retry)
         {
+            pMT->GetAuxiliaryDataForWrite()->SetMayHaveOpenInterfacesInInterfaceMap();
             retryWithExactInterfaces = true;
         }
     } while (retry);

diff --git a/.../Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.cs b/.../Loader/classloader/generics/Instantiation/Positive/CuriouslyRecurringThroughInterface.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.CompilerServices;
 using Xunit;
 
 namespace CuriouslyRecurringPatternThroughInterface
@@ -13,15 +14,69 @@ class CuriouslyRecurringThroughInterface<T_CuriouslyRecurringThroughInterface> :
     {
     }
 
+    class BaseClassWhereAImplementsB<A,B> where A : B {}
+    interface ICuriouslyRecurring2<T_ICuriouslyRecurring> : IGeneric<DerivedCuriouslyRecurringThroughInterface<T_ICuriouslyRecurring>>
+    {
+    }
+    class DerivedCuriouslyRecurringThroughInterface<T_CuriouslyRecurringThroughInterface> : BaseClassWhereAImplementsB<DerivedCuriouslyRecurringThroughInterface<T_CuriouslyRecurringThroughInterface>,ICuriouslyRecurring2<T_CuriouslyRecurringThroughInterface>>, ICuriouslyRecurring2<T_CuriouslyRecurringThroughInterface>
+    {
+    }
+
     public class Program
     {
         static object _o;
         [Fact]
-        public static void TestEntryPoint()
+        public static void TestIfCuriouslyRecurringInterfaceCanBeLoaded()
         {
+            Console.WriteLine("TestIfCuriouslyRecurringInterfaceCanBeLoaded");
+
             // Test that the a generic using a variant of the curiously recurring pattern involving an interface can be loaded.
             _o = typeof(CuriouslyRecurringThroughInterface<int>);
             Console.WriteLine("Found type: {0}", _o);
+            Console.WriteLine("Curiously recurring interface: {0}", typeof(ICuriouslyRecurring<>));
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]: {0}", typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]);
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces().Length: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces().Length);
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0]: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0]);
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments().Length: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments().Length);
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0]: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0]);
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetGenericArguments()[0]: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetGenericArguments()[0]);
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetGenericArguments()[0]==typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetGenericArguments()[0]==typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]);
+            Assert.True(typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetGenericArguments()[0]==typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]);
+
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces().Length: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces().Length);
+
+            // On CoreCLR this gets the Open type, which isn't really correct, but it has been that way for a very long time
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0]: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0]);
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0]==typeof(ICuriouslyRecurring<>): {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0]==typeof(ICuriouslyRecurring<>));
+
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0].GetGenericTypeDefinition()==typeof(ICuriouslyRecurring<>): {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0].GetGenericTypeDefinition()==typeof(ICuriouslyRecurring<>));
+
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0].GetGenericArguments()[0]==typeof(ICuriouslyRecurring<>)GetGenericArguments()[0]: {0}", typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0].GetGenericArguments()[0]==typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]);
+            Assert.True(typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0].GetInterfaces()[0].GetGenericArguments()[0]==typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]);
+        }
+
+        [Fact]
+        public static void TestIfCuriouslyRecurringInterfaceCanCast()
+        {
+            Console.WriteLine("TestIfCuriouslyRecurringInterfaceCanCast");
+            Console.WriteLine("typeof(ICuriouslyRecurring<>).MakeGenericType(typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]).IsAssignableFrom(typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0]): {0}", typeof(ICuriouslyRecurring<>).MakeGenericType(typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]).IsAssignableFrom(typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0]));
+            Assert.True(typeof(ICuriouslyRecurring<>).MakeGenericType(typeof(ICuriouslyRecurring<>).GetGenericArguments()[0]).IsAssignableFrom(typeof(ICuriouslyRecurring<>).GetInterfaces()[0].GetGenericArguments()[0]));
+        }
+
+        [Fact]
+        public static void TestIfCuriouslyRecurringInterfaceCanBeUsedAsConstraint()
+        {
+            Console.WriteLine("TestIfCuriouslyRecurringInterfaceCanBeUsedAsConstraint");
+            TestIfCuriouslyRecurringInterfaceCanBeUsedAsConstraintWorker();
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private static void TestIfCuriouslyRecurringInterfaceCanBeUsedAsConstraintWorker()
+        {
+            // Test that the a generic using a variant of the curiously recurring pattern involving an interface and constraint can be loaded.
+            // This test is just like TestIfCuriouslyRecurringInterfaceCanBeLoaded, except that it is structured so that we perform a cast via a constraint at type load time
+            _o = typeof(DerivedCuriouslyRecurringThroughInterface<int>);
+            Console.WriteLine("Found type: {0}", _o);
         }
     }
-}
+}