Covariant returns part 4

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -3371,12 +3371,6 @@ Although C# distinguishes between out and ref, the CLR sees them as the same. Wh
 
 Give the compiler some way to differentiate the methods. For example, you can give them different names or provide an additional parameter on one of them.</value>
   </data>
-  <data name="WRN_MultipleRuntimeOverrideMatches" xml:space="preserve">
-    <value>Member '{1}' overrides '{0}'. There are multiple override candidates at run-time. It is implementation dependent which method will be called.</value>
-  </data>
-  <data name="WRN_MultipleRuntimeOverrideMatches_Title" xml:space="preserve">
-    <value>Member overrides base member with multiple override candidates at run-time</value>
-  </data>
   <data name="ERR_ObjectOrCollectionInitializerWithDelegateCreation" xml:space="preserve">
     <value>Object and collection initializer expressions may not be applied to a delegate creation expression</value>
   </data>
@@ -6253,4 +6247,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="IDS_FeatureCovariantReturnsForOverrides" xml:space="preserve">
     <value>covariant returns</value>
   </data>
+  <data name="ERR_RuntimeDoesNotSupportCovariantReturnsOfClasses" xml:space="preserve">
+    <value>'{0}': Target runtime doesn't support covariant return types in overrides. Return type must be '{2}' to match overridden member '{1}'</value>
+  </data>
+  <data name="ERR_RuntimeDoesNotSupportCovariantPropertiesOfClasses" xml:space="preserve">
+    <value>'{0}': Target runtime doesn't support covariant types in overrides. Type must be '{2}' to match overridden member '{1}'</value>
+  </data>
 </root>

src/Compilers/CSharp/Portable/CodeGen/EmitConversion.cs

@@ -345,7 +345,7 @@ private void EmitDelegateCreation(BoundExpression node, BoundExpression receiver
                 _builder.EmitOpCode(ILOpCode.Ldvirtftn);
 
                 //  substitute the method with original virtual method
-                method = method.GetConstructedLeastOverriddenMethod(_method.ContainingType);
+                method = method.GetConstructedLeastOverriddenMethod(_method.ContainingType, requireSameReturnType: true);
             }
             else
             {

src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs

@@ -1597,7 +1597,7 @@ private void EmitCallExpression(BoundCall call, UseKind useKind)
             MethodSymbol actualMethodTargetedByTheCall = method;
             if (method.IsOverride && callKind != CallKind.Call)
             {
-                actualMethodTargetedByTheCall = method.GetConstructedLeastOverriddenMethod(_method.ContainingType);
+                actualMethodTargetedByTheCall = method.GetConstructedLeastOverriddenMethod(_method.ContainingType, requireSameReturnType: true);
             }
 
             if (callKind == CallKind.ConstrainedCallVirt && actualMethodTargetedByTheCall.ContainingType.IsValueType)

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -984,7 +984,7 @@ internal enum ErrorCode
         ERR_InitializerAddHasParamModifiers = 1954,
         ERR_NonInvocableMemberCalled = 1955,
         WRN_MultipleRuntimeImplementationMatches = 1956,
-        WRN_MultipleRuntimeOverrideMatches = 1957,
+        // WRN_MultipleRuntimeOverrideMatches = 1957, removed per https://github.com/dotnet/roslyn/issues/44067
         ERR_ObjectOrCollectionInitializerWithDelegateCreation = 1958,
         ERR_InvalidConstantDeclarationType = 1959,
         ERR_IllegalVarianceSyntax = 1960,
@@ -1775,6 +1775,8 @@ internal enum ErrorCode
         WRN_MemberNotNullWhen = 8775,
         WRN_MemberNotNullBadMember = 8776,
         WRN_ParameterDisallowsNull = 8777,
+        ERR_RuntimeDoesNotSupportCovariantReturnsOfClasses = 8778,
+        ERR_RuntimeDoesNotSupportCovariantPropertiesOfClasses = 8779,
 
         ERR_DesignatorBeneathPatternCombinator = 8780,
         ERR_UnsupportedTypeForRelationalPattern = 8781,

src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs

@@ -337,7 +337,6 @@ internal static int GetWarningLevel(ErrorCode code)
                 case ErrorCode.WRN_BadXMLRefTypeVar:
                 case ErrorCode.WRN_ReferencedAssemblyReferencesLinkedPIA:
                 case ErrorCode.WRN_MultipleRuntimeImplementationMatches:
-                case ErrorCode.WRN_MultipleRuntimeOverrideMatches:
                 case ErrorCode.WRN_FileAlreadyIncluded:
                 case ErrorCode.WRN_NoConfigNotOnCommandLine:
                 case ErrorCode.WRN_AnalyzerCannotBeCreated:

src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs

@@ -1187,7 +1187,7 @@ public BoundExpression MethodInfo(MethodSymbol method)
             // whether or not to call a method with a value type receiver directly).
             if (!method.ContainingType.IsValueType || !Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.MayUseCallForStructMethod(method))
             {
-                method = method.GetConstructedLeastOverriddenMethod(this.CompilationState.Type);
+                method = method.GetConstructedLeastOverriddenMethod(this.CompilationState.Type, requireSameReturnType: true);
             }
 
             return new BoundMethodInfo(

src/Compilers/CSharp/Portable/Symbols/AssemblySymbol.cs

@@ -427,6 +427,32 @@ internal bool RuntimeSupportsDefaultInterfaceImplementation
             get => !(GetSpecialTypeMember(SpecialMember.System_Runtime_CompilerServices_RuntimeFeature__DefaultImplementationsOfInterfaces) is null);
         }
 
+        /// <summary>
+        /// Figure out if the target runtime supports covariant returns of methods declared in classes.
+        /// </summary>
+        internal bool RuntimeSupportsCovariantReturnsOfClasses
+        {
+            get
+            {
+                // check for the runtime feature indicator.
+                if (!(GetSpecialTypeMember(SpecialMember.System_Runtime_CompilerServices_RuntimeFeature__CovariantReturnsOfClasses) is null))
+                {
+                    return true;
+                }
+
+                // https://github.com/dotnet/roslyn/issues/44206: For testing purposes, we added a feature flag to
+                // the C# compiler that causes the compiler to skip the requirement that the runtime supports covariant
+                // returns. We can remove it once we have a runtime that we can test against that actually supports the feature.
+                if (this is SourceAssemblySymbol { DeclaringCompilation: CSharpCompilation compilation } &&
+                    compilation.Feature("DoNotRequireRuntimeCovariantReturnsSupport") != null)
+                {
+                    return true;
+                }
+
+                return false;
+            }
+        }
+
         /// <summary>
         /// Return an array of assemblies involved in canonical type resolution of
         /// NoPia local types defined within this assembly. In other words, all 

src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs

@@ -544,7 +544,7 @@ internal static Symbol GetLeastOverriddenMember(this Symbol member, NamedTypeSym
             {
                 case SymbolKind.Method:
                     var method = (MethodSymbol)member;
-                    return method.GetConstructedLeastOverriddenMethod(accessingTypeOpt);
+                    return method.GetConstructedLeastOverriddenMethod(accessingTypeOpt, requireSameReturnType: false);
 
                 case SymbolKind.Property:
                     var property = (PropertySymbol)member;

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs

@@ -244,6 +244,7 @@ private sealed class UncommonFields
             public ImmutableArray<string> _lazyNotNullMembers;
             public ImmutableArray<string> _lazyNotNullMembersWhenTrue;
             public ImmutableArray<string> _lazyNotNullMembersWhenFalse;
+            public MethodSymbol _lazyExplicitClassOverride;
         }
 
         private UncommonFields CreateUncommonFields()
@@ -287,6 +288,11 @@ private UncommonFields CreateUncommonFields()
                 retVal._lazyNotNullMembersWhenFalse = ImmutableArray<string>.Empty;
             }
 
+            if (_packedFlags.IsExplicitOverrideIsPopulated)
+            {
+                retVal._lazyExplicitClassOverride = null;
+            }
+
             return retVal;
         }
 
@@ -1177,7 +1183,13 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
                     return explicitInterfaceImplementations;
                 }
 
-                var explicitlyOverriddenMethods = ExplicitlyOverriddenMethods();
+                var moduleSymbol = _containingType.ContainingPEModule;
+
+                // Context: we need the containing type of this method as context so that we can substitute appropriately into
+                // any generic interfaces that we might be explicitly implementing.  There is no reason to pass in the method
+                // context, however, because any method type parameters will belong to the implemented (i.e. interface) method,
+                // which we do not yet know.
+                var explicitlyOverriddenMethods = new MetadataDecoder(moduleSymbol, _containingType).GetExplicitlyOverriddenMethods(_containingType.Handle, _handle, this.ContainingType);
 
                 //avoid allocating a builder in the common case
                 var anyToRemove = false;
@@ -1199,15 +1211,9 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
                     }
                 }
 
-                // CONSIDER: could assert that we're writing the existing value if it's already there
-                // CONSIDER: what we'd really like to do is set this bit only in cases where the explicitly
-                // overridden method matches the method that will be returned by MethodSymbol.OverriddenMethod.
-                // Unfortunately, this MethodSymbol will not be sufficiently constructed (need IsOverride and MethodKind,
-                // which depend on this property) to determine which method OverriddenMethod will return.
-                _packedFlags.InitializeIsExplicitOverride(isExplicitFinalizerOverride: sawObjectFinalize, isExplicitClassOverride: anyToRemove);
-
                 explicitInterfaceImplementations = explicitlyOverriddenMethods;
 
+                bool hasUniqueClassOverride = false;
                 if (anyToRemove)
                 {
                     var explicitInterfaceImplementationsBuilder = ArrayBuilder<MethodSymbol>.GetInstance();
@@ -1220,41 +1226,48 @@ public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations
                     }
 
                     explicitInterfaceImplementations = explicitInterfaceImplementationsBuilder.ToImmutableAndFree();
+
+                    MethodSymbol uniqueClassOverride = null;
+                    foreach (MethodSymbol method in explicitlyOverriddenMethods)
+                    {
+                        if (method.ContainingType.IsClassType())
+                        {
+                            if (uniqueClassOverride is { })
+                            {
+                                // not unique
+                                uniqueClassOverride = null;
+                                break;
+                            }
+
+                            uniqueClassOverride = method;
+                        }
+                    }
+
+                    if (uniqueClassOverride is { })
+                    {
+                        hasUniqueClassOverride = true;
+                        Interlocked.CompareExchange(ref AccessUncommonFields()._lazyExplicitClassOverride, uniqueClassOverride, null);
+                    }
                 }
 
+                // CONSIDER: what we'd really like to do is set this bit only in cases where the explicitly
+                // overridden method matches the method that will be returned by MethodSymbol.OverriddenMethod.
+                // Unfortunately, this MethodSymbol will not be sufficiently constructed (need IsOverride and MethodKind,
+                // which depend on this property) to determine which method OverriddenMethod will return.
+                _packedFlags.InitializeIsExplicitOverride(isExplicitFinalizerOverride: sawObjectFinalize, isExplicitClassOverride: hasUniqueClassOverride);
+
                 return InterlockedOperations.Initialize(ref _lazyExplicitMethodImplementations, explicitInterfaceImplementations);
             }
         }
 
-        private ImmutableArray<MethodSymbol> ExplicitlyOverriddenMethods()
-        {
-            var moduleSymbol = _containingType.ContainingPEModule;
-
-            // Context: we need the containing type of this method as context so that we can substitute appropriately into
-            // any generic interfaces that we might be explicitly implementing.  There is no reason to pass in the method
-            // context, however, because any method type parameters will belong to the implemented (i.e. interface) method,
-            // which we do not yet know.
-            return new MetadataDecoder(moduleSymbol, _containingType).GetExplicitlyOverriddenMethods(_containingType.Handle, _handle, this.ContainingType);
-        }
-
+        /// <summary>
+        /// If a methodimpl record indicates a unique overridden method, that method. Otherwise null.
+        /// </summary>
         internal MethodSymbol ExplicitlyOverriddenClassMethod
         {
             get
             {
-                if (IsExplicitClassOverride)
-                {
-                    foreach (MethodSymbol method in ExplicitlyOverriddenMethods())
-                    {
-                        if (method.ContainingType.IsClassType())
-                        {
-                            return method;
-                        }
-                    }
-
-                    throw ExceptionUtilities.Unreachable;
-                }
-
-                return null;
+                return IsExplicitClassOverride ? AccessUncommonFields()._lazyExplicitClassOverride : null;
             }
         }
 

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs

@@ -535,21 +535,6 @@ public override ImmutableArray<SyntaxReference> DeclaringSyntaxReferences
             }
         }
 
-        internal PropertySymbol ExplicitlyOverriddenClassProperty
-        {
-            get
-            {
-                // A covariant return readonly property override is indicated in metadata by
-                // a get accessor with an explicit override (methodimpl).
-                if (GetMethod is PEMethodSymbol { ExplicitlyOverriddenClassMethod: { AssociatedSymbol: PropertySymbol overriddenProperty } })
-                {
-                    return overriddenProperty;
-                }
-
-                return null;
-            }
-        }
-
         public override ImmutableArray<CSharpAttributeData> GetAttributes()
         {
             if (_lazyCustomAttributes.IsDefault)

src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs

@@ -393,6 +393,17 @@ public virtual ImmutableArray<CSharpAttributeData> GetReturnTypeAttributes()
         /// </summary>
         /// <param name="accessingTypeOpt">The search must respect accessibility from this type.</param>
         internal MethodSymbol GetLeastOverriddenMethod(NamedTypeSymbol accessingTypeOpt)
+        {
+            return GetLeastOverriddenMethodCore(accessingTypeOpt, requireSameReturnType: false);
+        }
+
+        /// <summary>
+        /// Returns the original virtual or abstract method which a given method symbol overrides,
+        /// ignoring any other overriding methods in base classes.
+        /// </summary>
+        /// <param name="accessingTypeOpt">The search must respect accessibility from this type.</param>
+        /// <param name="requireSameReturnType">The returned method must have the same return type.</param>
+        private MethodSymbol GetLeastOverriddenMethodCore(NamedTypeSymbol accessingTypeOpt, bool requireSameReturnType)
         {
             var accessingType = ((object)accessingTypeOpt == null ? this.ContainingType : accessingTypeOpt).OriginalDefinition;
 
@@ -421,7 +432,9 @@ internal MethodSymbol GetLeastOverriddenMethod(NamedTypeSymbol accessingTypeOpt)
                 // See InternalsVisibleToAndStrongNameTests: IvtVirtualCall1, IvtVirtualCall2, IvtVirtual_ParamsAndDynamic.
                 MethodSymbol overridden = m.OverriddenMethod;
                 HashSet<DiagnosticInfo> useSiteDiagnostics = null;
-                if ((object)overridden == null || !AccessCheck.IsSymbolAccessible(overridden, accessingType, ref useSiteDiagnostics))
+                if ((object)overridden == null ||
+                    !AccessCheck.IsSymbolAccessible(overridden, accessingType, ref useSiteDiagnostics) ||
+                    (requireSameReturnType && !this.ReturnType.Equals(overridden.ReturnType, TypeCompareKind.AllIgnoreOptions)))
                 {
                     break;
                 }
@@ -438,9 +451,10 @@ internal MethodSymbol GetLeastOverriddenMethod(NamedTypeSymbol accessingTypeOpt)
         /// Also, if the given method symbol is generic then the resulting virtual or abstract method is constructed with the
         /// same type arguments as the given method.
         /// </summary>
-        internal MethodSymbol GetConstructedLeastOverriddenMethod(NamedTypeSymbol accessingTypeOpt)
+        /// <param name="requireSameReturnType">The returned method must have the same return type.</param>
+        internal MethodSymbol GetConstructedLeastOverriddenMethod(NamedTypeSymbol accessingTypeOpt, bool requireSameReturnType)
         {
-            var m = this.ConstructedFrom.GetLeastOverriddenMethod(accessingTypeOpt);
+            var m = this.ConstructedFrom.GetLeastOverriddenMethodCore(accessingTypeOpt, requireSameReturnType);
             return m.IsGenericMethod ? m.Construct(this.TypeArgumentsWithAnnotations) : m;
         }
 

src/Compilers/CSharp/Portable/Symbols/MethodSymbolExtensions.cs

@@ -72,8 +72,8 @@ public static bool IsRuntimeFinalizer(this MethodSymbol method, bool skipFirstMe
 
                 // At this point, we know method originated with a DestructorDeclarationSyntax in source,
                 // so it can't have the "new" modifier.
-                // First is fine, since there should only be one, since there are no parameters.
-                method = method.GetFirstRuntimeOverriddenMethodIgnoringNewSlot(ignoreInterfaceImplementationChanges: true);
+                // Unique is fine, since there should only be one, since there are no parameters.
+                method = method.GetUniqueRuntimeOverriddenMethodIgnoringNewSlot(ignoreInterfaceImplementationChanges: true);
                 skipFirstMethodKindCheck = false;
             }