Address PR feedback and revert a downlevel change

Author: stephentoub

src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs

@@ -156,56 +156,59 @@ public override int GetHashCode()
 
         protected virtual MethodInfo GetMethodImpl()
         {
-            if (_methodBase is null or not MethodInfo)
+            if (_methodBase is MethodInfo methodInfo)
             {
-                IRuntimeMethodInfo method = FindMethodHandle();
-                RuntimeType? declaringType = RuntimeMethodHandle.GetDeclaringType(method);
-                // need a proper declaring type instance method on a generic type
-                if (declaringType.IsGenericType)
+                return methodInfo;
+            }
+
+            IRuntimeMethodInfo method = FindMethodHandle();
+            RuntimeType? declaringType = RuntimeMethodHandle.GetDeclaringType(method);
+            // need a proper declaring type instance method on a generic type
+            if (declaringType.IsGenericType)
+            {
+                bool isStatic = (RuntimeMethodHandle.GetAttributes(method) & MethodAttributes.Static) != (MethodAttributes)0;
+                if (!isStatic)
                 {
-                    bool isStatic = (RuntimeMethodHandle.GetAttributes(method) & MethodAttributes.Static) != (MethodAttributes)0;
-                    if (!isStatic)
+                    if (_methodPtrAux == IntPtr.Zero)
                     {
-                        if (_methodPtrAux == IntPtr.Zero)
+                        // The target may be of a derived type that doesn't have visibility onto the
+                        // target method. We don't want to call RuntimeType.GetMethodBase below with that
+                        // or reflection can end up generating a MethodInfo where the ReflectedType cannot
+                        // see the MethodInfo itself and that breaks an important invariant. But the
+                        // target type could include important generic type information we need in order
+                        // to work out what the exact instantiation of the method's declaring type is. So
+                        // we'll walk up the inheritance chain (which will yield exactly instantiated
+                        // types at each step) until we find the declaring type. Since the declaring type
+                        // we get from the method is probably shared and those in the hierarchy we're
+                        // walking won't be we compare using the generic type definition forms instead.
+                        Type? currentType = _target!.GetType();
+                        Type targetType = declaringType.GetGenericTypeDefinition();
+                        while (currentType != null)
                         {
-                            // The target may be of a derived type that doesn't have visibility onto the
-                            // target method. We don't want to call RuntimeType.GetMethodBase below with that
-                            // or reflection can end up generating a MethodInfo where the ReflectedType cannot
-                            // see the MethodInfo itself and that breaks an important invariant. But the
-                            // target type could include important generic type information we need in order
-                            // to work out what the exact instantiation of the method's declaring type is. So
-                            // we'll walk up the inheritance chain (which will yield exactly instantiated
-                            // types at each step) until we find the declaring type. Since the declaring type
-                            // we get from the method is probably shared and those in the hierarchy we're
-                            // walking won't be we compare using the generic type definition forms instead.
-                            Type? currentType = _target!.GetType();
-                            Type targetType = declaringType.GetGenericTypeDefinition();
-                            while (currentType != null)
+                            if (currentType.IsGenericType &&
+                                currentType.GetGenericTypeDefinition() == targetType)
                             {
-                                if (currentType.IsGenericType &&
-                                    currentType.GetGenericTypeDefinition() == targetType)
-                                {
-                                    declaringType = currentType as RuntimeType;
-                                    break;
-                                }
-                                currentType = currentType.BaseType;
+                                declaringType = currentType as RuntimeType;
+                                break;
                             }
-
-                            // RCWs don't need to be "strongly-typed" in which case we don't find a base type
-                            // that matches the declaring type of the method. This is fine because interop needs
-                            // to work with exact methods anyway so declaringType is never shared at this point.
-                            Debug.Assert(currentType != null || _target.GetType().IsCOMObject, "The class hierarchy should declare the method");
-                        }
-                        else
-                        {
-                            // it's an open one, need to fetch the first arg of the instantiation
-                            MethodInfo invoke = this.GetType().GetMethod("Invoke")!;
-                            declaringType = (RuntimeType)invoke.GetParametersAsSpan()[0].ParameterType;
+                            currentType = currentType.BaseType;
                         }
+
+                        // RCWs don't need to be "strongly-typed" in which case we don't find a base type
+                        // that matches the declaring type of the method. This is fine because interop needs
+                        // to work with exact methods anyway so declaringType is never shared at this point.
+                        Debug.Assert(currentType != null || _target.GetType().IsCOMObject, "The class hierarchy should declare the method");
+                    }
+                    else
+                    {
+                        // it's an open one, need to fetch the first arg of the instantiation
+                        MethodInfo invoke = this.GetType().GetMethod("Invoke")!;
+                        declaringType = (RuntimeType)invoke.GetParametersAsSpan()[0].ParameterType;
                     }
                 }
-                _methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
             }
+
+            _methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
             return (MethodInfo)_methodBase;
         }
 

src/coreclr/System.Private.CoreLib/src/System/MulticastDelegate.CoreCLR.cs

@@ -515,20 +515,23 @@ protected override MethodInfo GetMethodImpl()
             {
                 // we handle unmanaged function pointers here because the generic ones (used for WinRT) would otherwise
                 // be treated as open delegates by the base implementation, resulting in failure to get the MethodInfo
-                if (_methodBase is null or not MethodInfo)
+                if (_methodBase is MethodInfo methodInfo)
                 {
-                    IRuntimeMethodInfo method = FindMethodHandle();
-                    RuntimeType declaringType = RuntimeMethodHandle.GetDeclaringType(method);
+                    return methodInfo;
+                }
 
-                    // need a proper declaring type instance method on a generic type
-                    if (declaringType.IsGenericType)
-                    {
-                        // we are returning the 'Invoke' method of this delegate so use this.GetType() for the exact type
-                        RuntimeType reflectedType = (RuntimeType)GetType();
-                        declaringType = reflectedType;
-                    }
-                    _methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
+                IRuntimeMethodInfo method = FindMethodHandle();
+                RuntimeType declaringType = RuntimeMethodHandle.GetDeclaringType(method);
+
+                // need a proper declaring type instance method on a generic type
+                if (declaringType.IsGenericType)
+                {
+                    // we are returning the 'Invoke' method of this delegate so use this.GetType() for the exact type
+                    RuntimeType reflectedType = (RuntimeType)GetType();
+                    declaringType = reflectedType;
                 }
+
+                _methodBase = (MethodInfo)RuntimeType.GetMethodBase(declaringType, method)!;
                 return (MethodInfo)_methodBase;
             }
 

src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/TypeNameParserHelpers.cs

@@ -34,7 +34,7 @@ internal static string GetGenericTypeFullName(ReadOnlySpan<char> fullTypeName, R
                 result.Append(']');
                 result.Append(',');
             }
-            result[^1] = ']'; // replace ',' with ']'
+            result[result.Length - 1] = ']'; // replace ',' with ']'
 
             return result.ToString();
         }