Skip to content

Commit

Permalink
Change GetParametersNoCopy to GetParametersAsSpan and use it in a few…
Browse files Browse the repository at this point in the history
… more places (#92310)

* Change GetParametersNoCopy to GetParametersAsSpan and use it  in a few more places

The internal GetParametersNoCopy avoids a defensive ParameterInfo[] copy, but it requires the callers to promise not to mutate the array. I'm changing the method to return a `ReadOnlySpan<ParameterInfo>`, which means a caller can't mutate it without using unsafe code, and enabling the compiler to flag any misuse.  I've then used it in a few more places. We might subsequently want to consider exposing it publicly.

* Update src/libraries/Common/src/System/Runtime/InteropServices/ComEventsMethod.cs
  • Loading branch information
stephentoub committed Sep 21, 2023
1 parent 493a702 commit c853fc1
Show file tree
Hide file tree
Showing 52 changed files with 112 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ private static void ClassRegistrationScenarioForType(ComActivationContext cxt, b
}

// Finally validate signature
ParameterInfo[] methParams = method.GetParameters();
ReadOnlySpan<ParameterInfo> methParams = method.GetParametersAsSpan();
if (method.ReturnType != typeof(void)
|| methParams == null
|| methParams.Length != 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,7 @@ private static bool InternalIsDefined(EventInfo element, Type attributeType, boo
}
else
{
ParameterInfo[] parameters = rtMethod.GetParameters();
return parameters[position]; // Point to the correct ParameterInfo of the method
return rtMethod.GetParametersAsSpan()[position]; // Point to the correct ParameterInfo of the method
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ protected virtual MethodInfo GetMethodImpl()
{
// it's an open one, need to fetch the first arg of the instantiation
MethodInfo invoke = this.GetType().GetMethod("Invoke")!;
declaringType = (RuntimeType)invoke.GetParameters()[0].ParameterType;
declaringType = (RuntimeType)invoke.GetParametersAsSpan()[0].ParameterType;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public override void Emit(OpCode opcode, MethodInfo meth)
}
if (opcode.StackBehaviourPop == StackBehaviour.Varpop)
{
stackchange -= meth.GetParametersNoCopy().Length;
stackchange -= meth.GetParametersAsSpan().Length;
}
// Pop the "this" parameter if the method is non-static,
// and the instruction is not newobj/ldtoken/ldftn.
Expand Down Expand Up @@ -416,7 +416,7 @@ private int GetMemberRefToken(MethodInfo methodInfo, Type[]? optionalParameterTy
if (rtMeth == null && dm == null)
throw new ArgumentException(SR.Argument_MustBeRuntimeMethodInfo, nameof(methodInfo));

ParameterInfo[] paramInfo = methodInfo.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> paramInfo = methodInfo.GetParametersAsSpan();
if (paramInfo != null && paramInfo.Length != 0)
{
parameterTypes = new Type[paramInfo.Length];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ internal SignatureHelper GetMemberRefSignature(MethodBase? method, int cGenericP
}

Debug.Assert(method is RuntimeMethodInfo || method is RuntimeConstructorInfo);
ParameterInfo[] parameters = method.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameters = method.GetParametersAsSpan();

Type[] parameterTypes = new Type[parameters.Length];
Type[][] requiredCustomModifiers = new Type[parameterTypes.Length][];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public abstract partial class MethodBase : MemberInfo
// used by EE
private IntPtr GetMethodDesc() { return MethodHandle.Value; }

internal virtual ParameterInfo[] GetParametersNoCopy() { return GetParameters(); }
internal virtual ReadOnlySpan<ParameterInfo> GetParametersAsSpan() { return GetParameters(); }
#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -180,20 +180,11 @@ public override IList<CustomAttributeData> GetCustomAttributesData()
// This seems to always returns System.Void.
internal override Type GetReturnType() { return Signature.ReturnType; }

internal override ParameterInfo[] GetParametersNoCopy() =>
internal override ReadOnlySpan<ParameterInfo> GetParametersAsSpan() =>
m_parameters ??= RuntimeParameterInfo.GetParameters(this, this, Signature);

public override ParameterInfo[] GetParameters()
{
ParameterInfo[] parameters = GetParametersNoCopy();

if (parameters.Length == 0)
return parameters;

ParameterInfo[] ret = new ParameterInfo[parameters.Length];
Array.Copy(parameters, ret, parameters.Length);
return ret;
}
public override ParameterInfo[] GetParameters() =>
GetParametersAsSpan().ToArray();

public override MethodImplAttributes GetMethodImplementationFlags()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ private RuntimeCustomAttributeData(RuntimeModule scope, MetadataToken caCtorToke
m_ctor = (RuntimeConstructorInfo)scope.ResolveMethod(caCtorToken, attributeType.GenericTypeArguments, null)!.MethodHandle.GetMethodInfo();
}

ParameterInfo[] parameters = m_ctor.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameters = m_ctor.GetParametersAsSpan();
if (parameters.Length != 0)
{
m_ctorParams = new CustomAttributeCtorParameter[parameters.Length];
Expand Down Expand Up @@ -412,7 +412,7 @@ private void Init(object pca)
// Ensure there is only a single constructor for 'pca', so it is safe to suppress IL2075
ConstructorInfo[] allCtors = type.GetConstructors(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
Debug.Assert(allCtors.Length == 1);
Debug.Assert(allCtors[0].GetParameters().Length == 0);
Debug.Assert(allCtors[0].GetParametersAsSpan().Length == 0);
#endif

m_ctor = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance)[0];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,14 @@ o is RuntimeEventInfo m &&
#region Object Overrides
public override string ToString()
{
if (m_addMethod == null || m_addMethod.GetParametersNoCopy().Length == 0)
ReadOnlySpan<ParameterInfo> parameters;
if (m_addMethod == null ||
(parameters = m_addMethod.GetParametersAsSpan()).Length == 0)
{
throw new InvalidOperationException(SR.InvalidOperation_NoPublicAddMethod);
}

return m_addMethod.GetParametersNoCopy()[0].ParameterType.FormatTypeName() + " " + Name;
return parameters[0].ParameterType.FormatTypeName() + " " + Name;
}

public override bool Equals(object? obj) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public override Type? ReflectedType
#endregion

#region MethodBase Overrides
internal override ParameterInfo[] GetParametersNoCopy() =>
internal override ReadOnlySpan<ParameterInfo> GetParametersAsSpan() =>
FetchNonReturnParameters();

public override ParameterInfo[] GetParameters()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,38 +261,25 @@ public override MethodInfo[] GetAccessors(bool nonPublic)
return m_setterMethod;
}

public override ParameterInfo[] GetIndexParameters()
{
ParameterInfo[] indexParams = GetIndexParametersNoCopy();

int numParams = indexParams.Length;

if (numParams == 0)
return indexParams;

ParameterInfo[] ret = new ParameterInfo[numParams];

Array.Copy(indexParams, ret, numParams);

return ret;
}
public override ParameterInfo[] GetIndexParameters() =>
GetIndexParametersSpan().ToArray();

internal ParameterInfo[] GetIndexParametersNoCopy()
internal ReadOnlySpan<ParameterInfo> GetIndexParametersSpan()
{
// @History - Logic ported from RTM

// No need to lock because we don't guarantee the uniqueness of ParameterInfo objects
if (m_parameters == null)
{
int numParams = 0;
ParameterInfo[]? methParams = null;
ReadOnlySpan<ParameterInfo> methParams = default;

// First try to get the Get method.
RuntimeMethodInfo? m = GetGetMethod(true);
if (m != null)
{
// There is a Get method so use it.
methParams = m.GetParametersNoCopy();
methParams = m.GetParametersAsSpan();
numParams = methParams.Length;
}
else
Expand All @@ -302,7 +289,7 @@ internal ParameterInfo[] GetIndexParametersNoCopy()

if (m != null)
{
methParams = m.GetParametersNoCopy();
methParams = m.GetParametersAsSpan();
numParams = methParams.Length - 1;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2290,7 +2290,7 @@ private static bool FilterApplyMethodBase(
// Check if argumentTypes supplied
if (argumentTypes != null)
{
ParameterInfo[] parameterInfos = methodBase.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameterInfos = methodBase.GetParametersAsSpan();

if (argumentTypes.Length != parameterInfos.Length)
{
Expand Down Expand Up @@ -2848,8 +2848,7 @@ public override InterfaceMapping GetInterfaceMap([DynamicallyAccessedMembers(Dyn
{
ConstructorInfo firstCandidate = candidates[0];

ParameterInfo[] parameters = firstCandidate.GetParametersNoCopy();
if (parameters == null || parameters.Length == 0)
if (firstCandidate.GetParametersAsSpan().IsEmpty)
{
return firstCandidate;
}
Expand Down Expand Up @@ -3817,7 +3816,7 @@ private void CreateInstanceCheckThis()
throw new MissingMethodException(SR.Format(SR.MissingConstructor_Name, FullName));
}

if (invokeMethod.GetParametersNoCopy().Length == 0)
if (invokeMethod.GetParametersAsSpan().Length == 0)
{
if (args.Length != 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:System.Reflection.MethodBase.GetParametersNoCopy</Target>
<Target>M:System.Reflection.MethodBase.GetParametersAsSpan</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ public sealed override ParameterInfo GetParent(ParameterInfo e)

if (e.Position >= 0)
{
return methodParent.GetParametersNoCopy()[e.Position];
return methodParent.GetParametersAsSpan()[e.Position];
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ public static Attribute Instantiate(this CustomAttributeData cad)
// Find the public constructor that matches the supplied arguments.
//
ConstructorInfo? matchingCtor = null;
ParameterInfo[]? matchingParameters = null;
ReadOnlySpan<ParameterInfo> matchingParameters = default;
IList<CustomAttributeTypedArgument> constructorArguments = cad.ConstructorArguments;
foreach (ConstructorInfo ctor in attributeType.GetConstructors(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly))
{
ParameterInfo[] parameters = ctor.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameters = ctor.GetParametersAsSpan();
if (parameters.Length != constructorArguments.Count)
continue;
int i;
Expand All @@ -68,7 +68,7 @@ public static Attribute Instantiate(this CustomAttributeData cad)
//
// Found the right constructor. Instantiate the Attribute.
//
int arity = matchingParameters!.Length;
int arity = matchingParameters.Length;
object?[] invokeArguments = new object[arity];
for (int i = 0; i < arity; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public static object CreateInstance(
binder ??= Type.DefaultBinder;

MethodBase invokeMethod = binder.BindToMethod(bindingAttr, matches.ToArray(), ref args, null, culture, null, out object? state);
if (invokeMethod.GetParametersNoCopy().Length == 0)
if (invokeMethod.GetParametersAsSpan().Length == 0)
{
if (args.Length != 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public sealed class ConstructorInvoker
internal ConstructorInvoker(RuntimeConstructorInfo constructor)
{
_methodBaseInvoker = constructor.MethodInvoker;
_parameterCount = constructor.GetParametersNoCopy().Length;
_parameterCount = constructor.GetParametersAsSpan().Length;
_declaringTypeHandle = constructor.DeclaringType.TypeHandle;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public DynamicInvokeInfo(MethodBase method, IntPtr invokeThunk)

// _isValueTypeInstanceMethod = method.DeclaringType?.IsValueType ?? false;

ParameterInfo[] parameters = method.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameters = method.GetParametersAsSpan();

_argumentCount = parameters.Length;

Expand Down Expand Up @@ -579,7 +579,7 @@ private unsafe ref byte InvokeDirectWithFewArguments(

private unsafe object? GetCoercedDefaultValue(int index, in ArgumentInfo argumentInfo)
{
object? defaultValue = Method.GetParametersNoCopy()[index].DefaultValue;
object? defaultValue = Method.GetParametersAsSpan()[index].DefaultValue;
if (defaultValue == DBNull.Value)
throw new ArgumentException(SR.Arg_VarMissNull, "parameters");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public abstract partial class MethodBase : MemberInfo
public static MethodBase GetCurrentMethod() { throw NotImplemented.ByDesign; } //Implemented by toolchain.

// This is not an api but needs to be declared public so that System.Private.Reflection.Core can access (and override it)
public virtual ParameterInfo[] GetParametersNoCopy() => GetParameters();
public virtual ReadOnlySpan<ParameterInfo> GetParametersAsSpan() => GetParameters();

//
// MethodBase MetadataDefinitionMethod { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public sealed class MethodInvoker
internal MethodInvoker(RuntimeMethodInfo method)
{
_methodBaseInvoker = method.MethodInvoker;
_parameterCount = method.GetParametersNoCopy().Length;
_parameterCount = method.GetParametersAsSpan().Length;
}

internal MethodInvoker(RuntimeConstructorInfo constructor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ protected static bool AreNamesAndSignaturesEqual(MethodInfo method1, MethodInfo
if (method1.Name != method2.Name)
return false;

ParameterInfo[] p1 = method1.GetParametersNoCopy();
ParameterInfo[] p2 = method2.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> p1 = method1.GetParametersAsSpan();
ReadOnlySpan<ParameterInfo> p2 = method2.GetParametersAsSpan();
if (p1.Length != p2.Length)
return false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static bool QualifiesBasedOnParameterCount(this MethodBase methodBase, Bi
#endregion

#region ArgumentTypes
ParameterInfo[] parameterInfos = methodBase.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameterInfos = methodBase.GetParametersAsSpan();

if (argumentTypes.Length != parameterInfos.Length)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ protected static ConstructorInfo ResolveAttributeConstructor(
int parameterCount = parameterTypes.Length;
foreach (ConstructorInfo candidate in attributeType.GetConstructors(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly))
{
ParameterInfo[] candidateParameters = candidate.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> candidateParameters = candidate.GetParametersAsSpan();
if (parameterCount != candidateParameters.Length)
continue;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public sealed override MethodInfo RemoveMethod
public sealed override string ToString()
{
MethodInfo addMethod = this.AddMethod;
ParameterInfo[] parameters = addMethod.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameters = addMethod.GetParametersAsSpan();
if (parameters.Length == 0)
throw new InvalidOperationException(); // Legacy: Why is a ToString() intentionally throwing an exception?
RuntimeParameterInfo runtimeParameterInfo = (RuntimeParameterInfo)(parameters[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ private static RuntimeMethodInfo LookupMethodForCreateDelegate(RuntimeTypeInfo r
bindingFlags |= BindingFlags.IgnoreCase;
}
RuntimeMethodInfo invokeMethod = runtimeDelegateType.GetInvokeMethod();
ParameterInfo[] parameters = invokeMethod.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameters = invokeMethod.GetParametersAsSpan();
int numParameters = parameters.Length;
Type[] parameterTypes = new Type[numParameters];
for (int i = 0; i < numParameters; i++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static MethodBaseInvoker GetCustomMethodInvokerIfNeeded(this MethodBase m
if (!map.TryGetValue(methodBase.MetadataDefinitionMethod, out CustomMethodInvokerAction? action))
return null;

ParameterInfo[] parameterInfos = methodBase.GetParametersNoCopy();
ReadOnlySpan<ParameterInfo> parameterInfos = methodBase.GetParametersAsSpan();
Type[] parameterTypes = new Type[parameterInfos.Length];
for (int i = 0; i < parameterInfos.Length; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public sealed override ParameterInfo[] GetParameters()
return result;
}

public sealed override ParameterInfo[] GetParametersNoCopy()
public sealed override ReadOnlySpan<ParameterInfo> GetParametersAsSpan()
{
return RuntimeParameters;
}
Expand Down
Loading

0 comments on commit c853fc1

Please sign in to comment.