Fix GenDelegate to produce clean delegates without hidden Closure parameter

Dangercoder · dmiller · commit 0a4d51c13145 · 2026-04-01T16:46:37.000-04:00
GenDelegate.Create previously used Expression.Lambda + Compile() which
injected a hidden System.Runtime.CompilerServices.Closure parameter into
the delegate's Invoke signature. Frameworks like ASP.NET Minimal API that
reflect on delegate parameters would fail with "parameter without a name".

Fix: generate a proper wrapper class via TypeBuilder with the IFn stored
as a private field. The Invoke method exactly matches the delegate signature
with no hidden parameters.

Includes 7 NUnit tests covering: no hidden parameters, parameter signature
matching, IFn invocation, value type boxing/unboxing, void delegates.
diff --git a/Clojure/Clojure/CljCompiler/GenDelegate.cs b/Clojure/Clojure/CljCompiler/GenDelegate.cs
@@ -1,4 +1,4 @@
-﻿/**
+/**
  *   Copyright (c) Rich Hickey. All rights reserved.
  *   The use and distribution terms for this software are covered by the
  *   Eclipse Public License 1.0 (http://opensource.org/licenses/eclipse-1.0.php)
@@ -10,9 +10,9 @@
 
 using clojure.lang.CljCompiler.Context;
 using System;
-using System.Collections.Generic;
-using System.Linq.Expressions;
 using System.Reflection;
+using System.Reflection.Emit;
+using System.Threading;
 
 
 namespace clojure.lang
@@ -22,6 +22,7 @@ public static class GenDelegate
         #region Data
 
         static GenContext _context = GenContext.CreateWithInternalAssembly("delegates", false);
+        static int _wrapperCount = 0;
 
         #endregion
 
@@ -42,48 +43,86 @@ public static Delegate Create(Type delegateType, IFn fn)
         {
             MethodInfo invokeMI = delegateType.GetMethod("Invoke");
             Type returnType = invokeMI.ReturnType;
-
             ParameterInfo[] delParams = invokeMI.GetParameters();
 
-            List<ParameterExpression> parms = new List<ParameterExpression>();
-            List<Expression> callArgs = new List<Expression>();
-
-            foreach (ParameterInfo pi in delParams)
+            // Generate a wrapper class with the IFn as a field and an Invoke
+            // method that exactly matches the delegate signature. This produces
+            // delegates whose Method.GetParameters() contains only the declared
+            // parameters — no hidden Closure or IFn parameter — making them
+            // compatible with frameworks like ASP.NET that reflect on delegates.
+
+            int id = Interlocked.Increment(ref _wrapperCount);
+            TypeBuilder tb = _context.ModuleBuilder.DefineType(
+                "clojure.delegate.Wrapper_" + id,
+                TypeAttributes.Public | TypeAttributes.Sealed,
+                typeof(object));
+
+            FieldBuilder fnField = tb.DefineField("_fn", typeof(IFn), FieldAttributes.Private);
+
+            // Constructor: takes IFn
+            ConstructorBuilder ctor = tb.DefineConstructor(
+                MethodAttributes.Public,
+                CallingConventions.Standard,
+                [typeof(IFn)]);
+            ILGenerator ctorIL = ctor.GetILGenerator();
+            ctorIL.Emit(OpCodes.Ldarg_0);
+            ctorIL.Emit(OpCodes.Call, typeof(object).GetConstructor(Type.EmptyTypes));
+            ctorIL.Emit(OpCodes.Ldarg_0);
+            ctorIL.Emit(OpCodes.Ldarg_1);
+            ctorIL.Emit(OpCodes.Stfld, fnField);
+            ctorIL.Emit(OpCodes.Ret);
+
+            // Invoke method: matches delegate signature exactly
+            Type[] paramTypes = new Type[delParams.Length];
+            for (int i = 0; i < delParams.Length; i++)
+                paramTypes[i] = delParams[i].ParameterType;
+
+            MethodBuilder mb = tb.DefineMethod(
+                "Invoke",
+                MethodAttributes.Public,
+                returnType,
+                paramTypes);
+
+            // Name parameters to match the delegate
+            for (int i = 0; i < delParams.Length; i++)
+                mb.DefineParameter(i + 1, delParams[i].Attributes, delParams[i].Name ?? ("p" + i));
+
+            ILGenerator ilg = mb.GetILGenerator();
+
+            // Load this._fn
+            ilg.Emit(OpCodes.Ldarg_0);
+            ilg.Emit(OpCodes.Ldfld, fnField);
+
+            // Load and box each parameter for IFn.invoke(object, object, ...)
+            for (int i = 0; i < delParams.Length; i++)
             {
-                ParameterExpression pe = Expression.Parameter(pi.ParameterType, pi.Name);
-                parms.Add(pe);
-                callArgs.Add(MaybeBox(pe));
+                ilg.Emit(OpCodes.Ldarg, i + 1);
+                if (delParams[i].ParameterType.IsValueType)
+                    ilg.Emit(OpCodes.Box, delParams[i].ParameterType);
             }
 
-            Expression call =                    
-                Expression.Call(
-                    Expression.Constant(fn),
-                    Compiler.Methods_IFn_invoke[parms.Count], 
-                    callArgs);
-
-            Expression body =  returnType == typeof(void)
-                ? (Expression)Expression.Block(call,Expression.Default(typeof(void)))
-                : (Expression)Expression.Convert(call, returnType);
-
-            LambdaExpression lambda = Expression.Lambda(delegateType, body, true, parms);
-
-            return lambda.Compile();
-        }
-
-
-        #endregion
+            // Call IFn.invoke(...)
+            ilg.Emit(OpCodes.Callvirt, Compiler.Methods_IFn_invoke[delParams.Length]);
 
-        #region Boxing arguments
+            if (returnType == typeof(void))
+            {
+                ilg.Emit(OpCodes.Pop);
+            }
+            else if (returnType.IsValueType)
+            {
+                ilg.Emit(OpCodes.Unbox_Any, returnType);
+            }
+            else if (returnType != typeof(object))
+            {
+                ilg.Emit(OpCodes.Castclass, returnType);
+            }
 
-        internal static Expression MaybeBox(Expression expr)
-        {
-            if (expr.Type == typeof(void))
-                // I guess we'll pass a void.  This happens when we have a throw, for example.
-                return Expression.Block(expr, Expression.Default(typeof(object)));
+            ilg.Emit(OpCodes.Ret);
 
-            return expr.Type.IsValueType
-                ? Expression.Convert(expr, typeof(object))
-                : expr;
+            // Create instance and delegate
+            Type wrapperType = tb.CreateType();
+            object wrapper = Activator.CreateInstance(wrapperType, fn);
+            return Delegate.CreateDelegate(delegateType, wrapper, "Invoke");
         }
 
         #endregion
diff --git a/Clojure/Csharp.Tests/LibTests/GenDelegateTests.cs b/Clojure/Csharp.Tests/LibTests/GenDelegateTests.cs
@@ -0,0 +1,132 @@
+using System;
+using System.Reflection;
+using clojure.lang;
+using NUnit.Framework;
+
+namespace Clojure.Tests.LibTests
+{
+    [TestFixture]
+    public class GenDelegateTests
+    {
+        [OneTimeSetUp]
+        public void Setup()
+        {
+            RT.Init();
+        }
+
+        // Helper IFn that records what it was called with
+        class RecordingFn : AFn
+        {
+            public object[] LastArgs;
+            public object ReturnValue;
+
+            public RecordingFn(object returnValue = null)
+            {
+                ReturnValue = returnValue;
+            }
+
+            public override object invoke()
+            {
+                LastArgs = Array.Empty<object>();
+                return ReturnValue;
+            }
+
+            public override object invoke(object arg1)
+            {
+                LastArgs = new[] { arg1 };
+                return ReturnValue;
+            }
+
+            public override object invoke(object arg1, object arg2)
+            {
+                LastArgs = new[] { arg1, arg2 };
+                return ReturnValue;
+            }
+        }
+
+        [Test]
+        public void DelegateInvokeHasNoHiddenParameters()
+        {
+            var fn = new RecordingFn("hello");
+            var del = GenDelegate.Create(typeof(Func<string>), fn);
+
+            // The delegate's Method.GetParameters() must have exactly 0 parameters.
+            // The old Expression.Lambda approach injected a hidden Closure parameter.
+            var parameters = del.Method.GetParameters();
+            Assert.That(parameters, Has.Length.EqualTo(0),
+                "Func<string> delegate should have no parameters (no hidden Closure)");
+        }
+
+        [Test]
+        public void DelegateWithParametersMatchesSignature()
+        {
+            var fn = new RecordingFn("result");
+            var del = GenDelegate.Create(typeof(Func<int, string, string>), fn);
+
+            var parameters = del.Method.GetParameters();
+            Assert.That(parameters, Has.Length.EqualTo(2));
+            Assert.That(parameters[0].ParameterType, Is.EqualTo(typeof(int)));
+            Assert.That(parameters[1].ParameterType, Is.EqualTo(typeof(string)));
+        }
+
+        [Test]
+        public void DelegateInvokesIFnCorrectly()
+        {
+            var fn = new RecordingFn("hello from IFn");
+            var del = (Func<string>)GenDelegate.Create(typeof(Func<string>), fn);
+
+            var result = del();
+
+            Assert.That(result, Is.EqualTo("hello from IFn"));
+            Assert.That(fn.LastArgs, Has.Length.EqualTo(0));
+        }
+
+        [Test]
+        public void DelegateBoxesValueTypeArgs()
+        {
+            var fn = new RecordingFn("ok");
+            var del = (Func<int, string>)GenDelegate.Create(typeof(Func<int, string>), fn);
+
+            del(42);
+
+            Assert.That(fn.LastArgs, Has.Length.EqualTo(1));
+            Assert.That(fn.LastArgs[0], Is.EqualTo(42));
+            Assert.That(fn.LastArgs[0], Is.InstanceOf<object>(),
+                "int argument should be boxed to object for IFn.invoke");
+        }
+
+        [Test]
+        public void DelegateUnboxesValueTypeReturn()
+        {
+            var fn = new RecordingFn(99);
+            var del = (Func<int>)GenDelegate.Create(typeof(Func<int>), fn);
+
+            var result = del();
+
+            Assert.That(result, Is.EqualTo(99));
+        }
+
+        [Test]
+        public void VoidDelegateWorks()
+        {
+            var fn = new RecordingFn(null);
+            var del = (Action)GenDelegate.Create(typeof(Action), fn);
+
+            Assert.DoesNotThrow(() => del());
+            Assert.That(fn.LastArgs, Has.Length.EqualTo(0));
+        }
+
+        [Test]
+        public void DelegateWithTwoArgsPassesBothCorrectly()
+        {
+            var fn = new RecordingFn("combined");
+            var del = (Func<string, int, string>)GenDelegate.Create(typeof(Func<string, int, string>), fn);
+
+            var result = del("hello", 7);
+
+            Assert.That(result, Is.EqualTo("combined"));
+            Assert.That(fn.LastArgs[0], Is.EqualTo("hello"));
+            Assert.That(fn.LastArgs[1], Is.EqualTo(7));
+        }
+    }
+}
