Consider method accessibility in interface resolution (#81409)

Fixes dotnet/corert#1986 (yep, all the way to CoreRT repo).

We finally found an instance where this matters - in MAUI. Non-public methods never implement an interface by name+sig combo. We got the `ProtectedDerived` case in the newly added test wrong.

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedMethod.cs b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedMethod.cs
index 0c4294d..e215752 100644 (file)
--- a/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedMethod.cs
+++ b/src/coreclr/tools/Common/TypeSystem/Common/InstantiatedMethod.cs
@@ -117,6 +117,14 @@ namespace Internal.TypeSystem
             }
         }
 
+        public override bool IsPublic
+        {
+            get
+            {
+                return _methodDef.IsPublic;
+            }
+        }
+
         public override bool HasCustomAttribute(string attributeNamespace, string attributeName)
         {
             return _methodDef.HasCustomAttribute(attributeNamespace, attributeName);

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs
index 8b01121..ae2c4cb 100644 (file)
--- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs
+++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs
@@ -415,7 +415,7 @@ namespace Internal.TypeSystem
             return FindMatchingVirtualMethodOnTypeByNameAndSig(method, currentType, reverseMethodSearch, nameSigMatchMethodIsValidCandidate: s_VerifyMethodsHaveTheSameVirtualSlot);
         }
 
-        private static Func<MethodDesc, MethodDesc, bool> s_VerifyMethodsHaveTheSameVirtualSlot = VerifyMethodsHaveTheSameVirtualSlot;
+        private static readonly Func<MethodDesc, MethodDesc, bool> s_VerifyMethodsHaveTheSameVirtualSlot = VerifyMethodsHaveTheSameVirtualSlot;
 
         // Return true if the slot that defines methodToVerify matches slotDefiningMethod
         private static bool VerifyMethodsHaveTheSameVirtualSlot(MethodDesc slotDefiningMethod, MethodDesc methodToVerify)
@@ -424,6 +424,14 @@ namespace Internal.TypeSystem
             return slotDefiningMethodOfMethodToVerify == slotDefiningMethod;
         }
 
+        private static readonly Func<MethodDesc, MethodDesc, bool> s_VerifyMethodIsPublic = VerifyMethodIsPublic;
+
+        // Return true if the method to verify is public
+        private static bool VerifyMethodIsPublic(MethodDesc slotDefiningMethod, MethodDesc methodToVerify)
+        {
+            return methodToVerify.IsPublic;
+        }
+
         private static void FindBaseUnificationGroup(MetadataType currentType, UnificationGroup unificationGroup)
         {
             MethodDesc originalDefiningMethod = unificationGroup.DefiningMethod;
@@ -615,7 +623,7 @@ namespace Internal.TypeSystem
             {
                 MethodDesc foundOnCurrentType = FindMatchingVirtualMethodOnTypeByNameAndSig(interfaceMethod, currentType,
                     reverseMethodSearch: false, /* When searching for name/sig overrides on a type that explicitly defines an interface, search through the type in the forward direction*/
-                    nameSigMatchMethodIsValidCandidate :null);
+                    nameSigMatchMethodIsValidCandidate: s_VerifyMethodIsPublic);
                 foundOnCurrentType = FindSlotDefiningMethodForVirtualMethod(foundOnCurrentType);
 
                 if (baseType == null)
@@ -653,7 +661,7 @@ namespace Internal.TypeSystem
                 {
                     MethodDesc foundOnCurrentType = FindMatchingVirtualMethodOnTypeByNameAndSig(interfaceMethod, currentType,
                                             reverseMethodSearch: false, /* When searching for name/sig overrides on a type that is the first type in the hierarchy to require the interface, search through the type in the forward direction*/
-                                            nameSigMatchMethodIsValidCandidate: null);
+                                            nameSigMatchMethodIsValidCandidate: s_VerifyMethodIsPublic);
 
                     foundOnCurrentType = FindSlotDefiningMethodForVirtualMethod(foundOnCurrentType);
 
@@ -729,7 +737,7 @@ namespace Internal.TypeSystem
 
                 MethodDesc nameSigOverride = FindMatchingVirtualMethodOnTypeByNameAndSig(interfaceMethod, currentType,
                     reverseMethodSearch: true, /* When searching for a name sig match for an interface on parent types search in reverse order of declaration */
-                    nameSigMatchMethodIsValidCandidate:null);
+                    nameSigMatchMethodIsValidCandidate: s_VerifyMethodIsPublic);
 
                 if (nameSigOverride != null)
                 {

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs b/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs
index 95218b9..7802c85 100644 (file)
--- a/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs
+++ b/src/coreclr/tools/Common/TypeSystem/Common/MethodDesc.cs
@@ -584,6 +584,14 @@ namespace Internal.TypeSystem
             }
         }
 
+        public virtual bool IsPublic
+        {
+            get
+            {
+                return false;
+            }
+        }
+
         public abstract bool HasCustomAttribute(string attributeNamespace, string attributeName);
 
         /// <summary>

diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MethodForInstantiatedType.cs b/src/coreclr/tools/Common/TypeSystem/Common/MethodForInstantiatedType.cs
index ff9df46..db3df98 100644 (file)
--- a/src/coreclr/tools/Common/TypeSystem/Common/MethodForInstantiatedType.cs
+++ b/src/coreclr/tools/Common/TypeSystem/Common/MethodForInstantiatedType.cs
@@ -109,6 +109,14 @@ namespace Internal.TypeSystem
             }
         }
 
+        public override bool IsPublic
+        {
+            get
+            {
+                return _typicalMethodDef.IsPublic;
+            }
+        }
+
         public override bool HasCustomAttribute(string attributeNamespace, string attributeName)
         {
             return _typicalMethodDef.HasCustomAttribute(attributeNamespace, attributeName);

diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaMethod.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaMethod.cs
index 8cd76e1..9a7edd1 100644 (file)
--- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaMethod.cs
+++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaMethod.cs
@@ -353,6 +353,14 @@ namespace Internal.TypeSystem.Ecma
             }
         }
 
+        public override bool IsPublic
+        {
+            get
+            {
+                return Attributes.IsPublic();
+            }
+        }
+
         public override bool IsStaticConstructor
         {
             get

diff --git a/src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs b/src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs
index a6d345c..f163dbb 100644 (file)
--- a/src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs
+++ b/src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs
@@ -35,6 +35,7 @@ public class Interfaces
         if (TestIterfaceCallOptimization() == Fail)
             return Fail;
 
+        TestPublicAndNonpublicDifference.Run();
         TestDefaultInterfaceMethods.Run();
         TestDefaultInterfaceVariance.Run();
         TestVariantInterfaceOptimizations.Run();
@@ -477,6 +478,46 @@ public class Interfaces
 
     #endregion
 
+    class TestPublicAndNonpublicDifference
+    {
+        interface IFrobber
+        {
+            string Frob();
+        }
+        class ProtectedBase : IFrobber
+        {
+            string IFrobber.Frob() => "IFrobber.Frob";
+            protected virtual string Frob() => "Base.Frob";
+        }
+
+        class ProtectedDerived : ProtectedBase, IFrobber
+        {
+            protected override string Frob() => "Derived.Frob";
+        }
+
+        class PublicBase : IFrobber
+        {
+            string IFrobber.Frob() => "IFrobber.Frob";
+            public virtual string Frob() => "Base.Frob";
+        }
+
+        class PublicDerived : PublicBase, IFrobber
+        {
+            public override string Frob() => "Derived.Frob";
+        }
+
+        public static void Run()
+        {
+            IFrobber f1 = new PublicDerived();
+            if (f1.Frob() != "Derived.Frob")
+                throw new Exception();
+
+            IFrobber f2 = new ProtectedDerived();
+            if (f2.Frob() != "IFrobber.Frob")
+                throw new Exception();
+        }
+    }
+
     class TestDefaultInterfaceMethods
     {
         interface IFoo