Skip to content

Commit 26bc908

Browse files
stakxstakx
authored
Refactor ArgumentsUtil (#729)
* remove unused `ConvertToArgumentReference` method overload * change `ConvertToArgumentReferenceExpression` name & return type * inline single-use `InitializeArgumentsByPosition` method * DynamicProxy does not emit static methods * prevent memory alloc in case of no args/params (use C# 14 `[]` in favor of `Array.Empty<>()`) * use more consistent names * switch to extension method syntax * bring in `ParameterInfo` helpers from `GeneratorUtil` and add unit tests for them
1 parent aa83725 commit 26bc908

8 files changed

Lines changed: 181 additions & 133 deletions

File tree

‎src/Castle.Core.Tests/DynamicProxy.Tests/ArgumentsUtilTestCase.cs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright 2004-2026 Castle Project - http://www.castleproject.org/
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
namespace Castle.DynamicProxy.Tests
16+
{
17+
using System.Linq;
18+
using System.Reflection;
19+
20+
using Castle.DynamicProxy.Generators.Emitters;
21+
using Castle.DynamicProxy.Internal;
22+
23+
using NUnit.Framework;
24+
25+
[TestFixture]
26+
public class ArgumentsUtilTestCase
27+
{
28+
public interface Methods
29+
{
30+
public void ByValue(int arg);
31+
public void In(in int arg);
32+
public void Out(out int arg);
33+
public void Ref(ref int arg);
34+
public void RefReadonly(ref readonly int arg);
35+
}
36+
37+
[TestCase(nameof(Methods.ByValue), false)]
38+
[TestCase(nameof(Methods.In), true)]
39+
[TestCase(nameof(Methods.Out), true)]
40+
[TestCase(nameof(Methods.Ref), true)]
41+
[TestCase(nameof(Methods.RefReadonly), true)]
42+
public void IsByRef(string methodName, bool expected)
43+
{
44+
var parameter = GetParameterOf(methodName);
45+
Assert.AreEqual(expected, parameter.IsByRef);
46+
}
47+
48+
[TestCase(nameof(Methods.In), true)]
49+
[TestCase(nameof(Methods.Out), false)]
50+
[TestCase(nameof(Methods.Ref), false)]
51+
[TestCase(nameof(Methods.RefReadonly), true)]
52+
public void IsReadOnly(string methodName, bool expected)
53+
{
54+
var parameter = GetParameterOf(methodName);
55+
Assume.That(parameter.ParameterType.IsByRef);
56+
Assert.AreEqual(expected, parameter.IsReadOnly);
57+
}
58+
59+
private ParameterInfo GetParameterOf(string methodName)
60+
{
61+
return typeof(Methods).GetMethod(methodName)!.GetParameters().Single();
62+
}
63+
}
64+
}

‎src/Castle.Core.Tests/DynamicProxy.Tests/ClassEmitterTestCase.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2004-2025 Castle Project - http://www.castleproject.org/
1+
// Copyright 2004-2026 Castle Project - http://www.castleproject.org/
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -41,18 +41,6 @@ public void AutomaticDefaultConstructorGenerationWithClosedGenericType()
4141
Activator.CreateInstance(t);
4242
}
4343

44-
[Test]
45-
public void StaticMethodArguments()
46-
{
47-
ClassEmitter emitter = new ClassEmitter(generator.ProxyBuilder.ModuleScope, "Foo", typeof (List<object>),
48-
Type.EmptyTypes);
49-
MethodEmitter methodEmitter = emitter.CreateMethod("StaticMethod", MethodAttributes.Public | MethodAttributes.Static,
50-
typeof (string), typeof (string));
51-
methodEmitter.CodeBuilder.AddStatement(new ReturnStatement(methodEmitter.Arguments[0]));
52-
Type t = emitter.BuildType();
53-
Assert.AreEqual("five", t.GetMethod("StaticMethod").Invoke(null, new object[] {"five"}));
54-
}
55-
5644
[Test]
5745
public void InstanceMethodArguments()
5846
{

‎src/Castle.Core/DynamicProxy/Contributors/CompositeTypeContributor.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,14 @@ private void ImplementMethod(MetaMethod method, ClassEmitter @class,
139139

140140
protected static Type[] GetCacheKeyTypes(MetaMethod method)
141141
{
142-
Type[] argumentTypes = ArgumentsUtil.GetTypes(method.MethodOnTarget.GetParameters());
143-
if (argumentTypes.Length < 1)
142+
Type[] parameterTypes = method.MethodOnTarget.GetParameters().GetTypes();
143+
if (parameterTypes.Length < 1)
144144
{
145145
return new[] { method.MethodOnTarget.ReturnType };
146146
}
147-
var types = new Type[argumentTypes.Length + 1];
147+
var types = new Type[parameterTypes.Length + 1];
148148
types[0] = method.MethodOnTarget.ReturnType;
149-
argumentTypes.CopyTo(types, 1);
149+
parameterTypes.CopyTo(types, 1);
150150
return types;
151151
}
152152

‎src/Castle.Core/DynamicProxy/Generators/Emitters/ArgumentsUtil.cs‎

Lines changed: 99 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2004-2025 Castle Project - http://www.castleproject.org/
1+
// Copyright 2004-2026 Castle Project - http://www.castleproject.org/
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -15,79 +15,135 @@
1515
namespace Castle.DynamicProxy.Generators.Emitters
1616
{
1717
using System;
18+
using System.Linq;
1819
using System.Reflection;
19-
using System.Reflection.Emit;
20+
using System.Runtime.InteropServices;
2021

2122
using Castle.DynamicProxy.Generators.Emitters.SimpleAST;
2223

23-
internal abstract class ArgumentsUtil
24+
internal static class ArgumentsUtil
2425
{
25-
public static ArgumentReference[] ConvertToArgumentReference(Type[] args)
26+
extension(ParameterInfo parameter)
2627
{
27-
var arguments = new ArgumentReference[args.Length];
28-
29-
for (var i = 0; i < args.Length; ++i)
28+
public bool IsByRef
3029
{
31-
arguments[i] = new ArgumentReference(args[i]);
30+
get
31+
{
32+
return parameter.ParameterType.IsByRef;
33+
}
3234
}
3335

34-
return arguments;
36+
public bool IsReadOnly
37+
{
38+
get
39+
{
40+
// C# `in` parameters are also by-ref, but meant to be read-only.
41+
// The section "Metadata representation of in parameters" on the following page
42+
// defines how such parameters are marked:
43+
//
44+
// https://github.com/dotnet/csharplang/blob/master/proposals/csharp-7.2/readonly-ref.md
45+
//
46+
// This poses three problems for detecting them:
47+
//
48+
// * The C# Roslyn compiler marks `in` parameters with an `[in]` IL modifier,
49+
// but this isn't specified, nor is it used uniquely for `in` params.
50+
//
51+
// * `System.Runtime.CompilerServices.IsReadOnlyAttribute` is not defined on all
52+
// .NET platforms, so the compiler sometimes recreates that type in the same
53+
// assembly that contains the method having an `in` parameter. In other words,
54+
// it's an attribute one must check for by name (which is slow, as it implies
55+
// use of a `GetCustomAttributes` enumeration instead of a faster `IsDefined`).
56+
//
57+
// * A required custom modifier `System.Runtime.InteropServices.InAttribute`
58+
// is always present in those cases relevant for DynamicProxy (proxyable methods),
59+
// but not all targeted platforms support reading custom modifiers. Also,
60+
// support for cmods is generally flaky (at this time of writing, mid-2018).
61+
//
62+
// The above points inform the following detection logic:
63+
64+
// First, fast-guard against non-`in` params by checking for an IL `[in]` modifier:
65+
if ((parameter.Attributes & (ParameterAttributes.In | ParameterAttributes.Out)) != ParameterAttributes.In)
66+
{
67+
return false;
68+
}
69+
70+
// Second, check for the required modifiers (hoping for good modreq support):
71+
if (parameter.GetRequiredCustomModifiers().Any(x => x == typeof(InAttribute)))
72+
{
73+
return true;
74+
}
75+
76+
// Third, check for `IsReadOnlyAttribute` by name (see explanation above).
77+
// This check is likely the slowest (despite being accurate) so we do it last:
78+
if (parameter.GetCustomAttributes(false).Any(x => x.GetType().FullName == "System.Runtime.CompilerServices.IsReadOnlyAttribute"))
79+
{
80+
return true;
81+
}
82+
83+
return false;
84+
}
85+
}
3586
}
3687

37-
public static ArgumentReference[] ConvertToArgumentReference(ParameterInfo[] args)
88+
extension(ParameterInfo[] parameters)
3889
{
39-
var arguments = new ArgumentReference[args.Length];
40-
41-
for (var i = 0; i < args.Length; ++i)
90+
public ArgumentReference[] ConvertToArgumentReferences()
4291
{
43-
arguments[i] = new ArgumentReference(args[i].ParameterType);
44-
}
92+
if (parameters.Length == 0) return [];
4593

46-
return arguments;
47-
}
94+
var arguments = new ArgumentReference[parameters.Length];
4895

49-
public static IExpression[] ConvertToArgumentReferenceExpression(ParameterInfo[] args)
50-
{
51-
var arguments = new IExpression[args.Length];
96+
for (var i = 0; i < parameters.Length; ++i)
97+
{
98+
arguments[i] = new ArgumentReference(parameters[i].ParameterType, i + 1);
99+
}
52100

53-
for (var i = 0; i < args.Length; ++i)
54-
{
55-
arguments[i] = new ArgumentReference(args[i].ParameterType, i + 1);
101+
return arguments;
56102
}
57103

58-
return arguments;
104+
public Type[] GetTypes()
105+
{
106+
if (parameters.Length == 0) return [];
107+
108+
var types = new Type[parameters.Length];
109+
for (var i = 0; i < parameters.Length; i++)
110+
{
111+
types[i] = parameters[i].ParameterType;
112+
}
113+
return types;
114+
}
59115
}
60116

61-
public static Type[] GetTypes(ParameterInfo[] parameters)
117+
extension(Type[] parameterTypes)
62118
{
63-
var types = new Type[parameters.Length];
64-
for (var i = 0; i < parameters.Length; i++)
119+
public ArgumentReference[] ConvertToArgumentReferences()
65120
{
66-
types[i] = parameters[i].ParameterType;
121+
if (parameterTypes.Length == 0) return [];
122+
123+
var arguments = new ArgumentReference[parameterTypes.Length];
124+
125+
for (var i = 0; i < parameterTypes.Length; ++i)
126+
{
127+
arguments[i] = new ArgumentReference(parameterTypes[i], i + 1);
128+
}
129+
130+
return arguments;
67131
}
68-
return types;
69132
}
70133

71-
public static Type[] InitializeAndConvert(ArgumentReference[] args)
134+
public static Type[] InitializeAndConvert(ArgumentReference[] arguments)
72135
{
73-
var types = new Type[args.Length];
136+
if (arguments.Length == 0) return [];
137+
138+
var types = new Type[arguments.Length];
74139

75-
for (var i = 0; i < args.Length; ++i)
140+
for (var i = 0; i < arguments.Length; ++i)
76141
{
77-
args[i].Position = i + 1;
78-
types[i] = args[i].Type;
142+
arguments[i].Position = i + 1;
143+
types[i] = arguments[i].Type;
79144
}
80145

81146
return types;
82147
}
83-
84-
public static void InitializeArgumentsByPosition(ArgumentReference[] args, bool isStatic)
85-
{
86-
var offset = isStatic ? 0 : 1;
87-
for (var i = 0; i < args.Length; ++i)
88-
{
89-
args[i].Position = i + offset;
90-
}
91-
}
92148
}
93149
}

‎src/Castle.Core/DynamicProxy/Generators/Emitters/MethodEmitter.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2004-2025 Castle Project - http://www.castleproject.org/
1+
// Copyright 2004-2026 Castle Project - http://www.castleproject.org/
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -64,12 +64,12 @@ internal MethodEmitter(ClassEmitter owner, string name,
6464

6565
var returnType = methodToUseAsATemplate.ReturnType;
6666
var baseMethodParameters = methodToUseAsATemplate.GetParameters();
67-
var parameters = ArgumentsUtil.GetTypes(baseMethodParameters);
67+
var parameterTypes = baseMethodParameters.GetTypes();
6868

6969
genericTypeParams = GenericUtil.CopyGenericArguments(methodToUseAsATemplate, builder);
70-
SetParameters(parameters);
70+
SetParameters(parameterTypes);
7171
SetReturnType(returnType);
72-
SetSignature(returnType, methodToUseAsATemplate.ReturnParameter, parameters, baseMethodParameters);
72+
SetSignature(returnType, methodToUseAsATemplate.ReturnParameter, parameterTypes, baseMethodParameters);
7373
DefineParameters(baseMethodParameters);
7474
}
7575

@@ -115,8 +115,9 @@ public void DefineCustomAttribute(CustomAttributeBuilder attribute)
115115
public void SetParameters(Type[] paramTypes)
116116
{
117117
builder.SetParameters(paramTypes);
118-
arguments = ArgumentsUtil.ConvertToArgumentReference(paramTypes);
119-
ArgumentsUtil.InitializeArgumentsByPosition(arguments, MethodBuilder.IsStatic);
118+
119+
Debug.Assert(MethodBuilder.IsStatic == false, "The following call to `ConvertToArgumentReferences` assumes the presence of a `this` parameter when assigning parameter positions.");
120+
arguments = paramTypes.ConvertToArgumentReferences();
120121
}
121122

122123
public virtual void Generate()

‎src/Castle.Core/DynamicProxy/Generators/ForwardingMethodGenerator.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2004-2025 Castle Project - http://www.castleproject.org/
1+
// Copyright 2004-2026 Castle Project - http://www.castleproject.org/
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -33,7 +33,7 @@ protected override MethodEmitter BuildProxiedMethodBody(MethodEmitter emitter, C
3333
INamingScope namingScope)
3434
{
3535
var target = getTarget(@class, MethodToOverride);
36-
var arguments = ArgumentsUtil.ConvertToArgumentReferenceExpression(MethodToOverride.GetParameters());
36+
var arguments = MethodToOverride.GetParameters().ConvertToArgumentReferences();
3737

3838
emitter.CodeBuilder.AddStatement(new ReturnStatement(
3939
new MethodInvocationExpression(

0 commit comments

Comments
 (0)