Skip to content

Commit 95d1d39

Browse files
committed
#293 - alternative expression for the recursive func to pass the test
1 parent da1ddd4 commit 95d1d39

File tree

3 files changed

+52
-11
lines changed

3 files changed

+52
-11
lines changed

src/FastExpressionCompiler/FastExpressionCompiler.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ public void AddNonPassedParam(ParameterExpression expr)
610610

611611
if (NonPassedParameters.Length == 0)
612612
{
613-
NonPassedParameters = new[] { expr };
613+
NonPassedParameters = new[] { expr }; // todo: @perf optimize for a single non passed parameter
614614
return;
615615
}
616616

@@ -1044,7 +1044,7 @@ private static bool TryCollectBoundConstants(ref ClosureInfo closure, Expression
10441044
private static bool TryCollectBoundConstants(ref ClosureInfo closure, Expression expr, IReadOnlyList<PE> paramExprs, bool isNestedLambda,
10451045
ref ClosureInfo rootClosure, CompilerFlags flags)
10461046
{
1047-
var paramCount = paramExprs.Count;
1047+
var paramCount = paramExprs.Count; // todo: @perf move closer to usage - don't need it in the most cases
10481048
#endif
10491049
while (true)
10501050
{
@@ -1070,7 +1070,7 @@ private static bool TryCollectBoundConstants(ref ClosureInfo closure, Expression
10701070
{
10711071
// if parameter is used BUT is not in passed parameters and not in local variables,
10721072
// it means parameter is provided by outer lambda and should be put in closure for current lambda
1073-
var p = paramCount - 1;
1073+
var p = paramCount - 1; // todo: @perf optimize for the 1 parameter - we don't need the while loop or call the methods
10741074
while (p != -1 && !ReferenceEquals(paramExprs.GetParameter(p), expr)) --p;
10751075
if (p == -1 && !closure.IsLocalVar(expr))
10761076
{

test/FastExpressionCompiler.IssueTests/Issue293_Recursive_Methods.cs

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,65 @@ public int Run()
2222
[Test]
2323
public void Test_Recursive_Expression()
2424
{
25-
var expr = MakeFactorialExpression<int>();
25+
var expr = MakeFactorialExpressionWithTheTrick<int>();
2626

2727
expr.PrintCSharp();
2828
var fs = expr.CompileSys();
2929
fs.PrintIL();
3030
var res = fs(4);
3131

32-
var f = expr.CompileFast(true, CompilerFlags.NoInvocationLambdaInlining);
32+
var f = expr.CompileFast(true);
3333
f.PrintIL();
3434
var res2 = f(4);
3535

36-
// f = expr.CompileFast(true);
37-
// f.PrintIL();
38-
// res2 = f(4);
39-
4036
Assert.AreEqual(res, res2);
4137
}
4238

39+
public Expression<Func<T, T>> MakeFactorialExpressionWithTheTrick<T>()
40+
{
41+
var nParam = Expression.Parameter(typeof(T), "n");
42+
var methodVar = Expression.Variable(typeof(Func<T, T>), "fac");
43+
var methodsVar = Expression.Variable(typeof(Func<T, T>[]), "facs");
44+
var one = Expression.Constant(1, typeof(T));
45+
46+
// This does not work:
47+
// Func<int, int> rec = null;
48+
// Func<int, int> tmp = n => n <= 1 ? 1 : n * rec(n - 1);
49+
// rec = tmp; // setting the closure variable! means that this closure variable is not readonly
50+
51+
// This should work:
52+
// var recs = new Func<int, int>[1];
53+
// Func<int, int> tmp = n => n <= 1 ? 1 : n * recs[0](n - 1);
54+
// recs[0] = tmp; // setting the item inside the closure variable of array type should work because of reference semantic
55+
56+
57+
return Expression.Lambda<Func<T, T>>(
58+
Expression.Block(
59+
new[] { methodsVar, methodVar },
60+
Expression.Assign(methodsVar, Expression.NewArrayBounds(typeof(Func<T, T>), Expression.Constant(1))),
61+
Expression.Assign(
62+
methodVar,
63+
Expression.Lambda<Func<T, T>>(
64+
Expression.Condition(
65+
// ( n <= 1 )
66+
Expression.LessThanOrEqual(nParam, one),
67+
// 1
68+
one,
69+
// n * method( n - 1 )
70+
Expression.Multiply(
71+
// n
72+
nParam,
73+
// method( n - 1 )
74+
Expression.Invoke(
75+
Expression.ArrayIndex(methodsVar, Expression.Constant(0)),
76+
Expression.Subtract(nParam, one)))),
77+
nParam)),
78+
Expression.Assign(Expression.ArrayAccess(methodsVar, Expression.Constant(0)), methodVar),
79+
// return method( n );
80+
Expression.Invoke(methodVar, nParam)),
81+
nParam);
82+
}
83+
4384
//from https://chriscavanagh.wordpress.com/2012/06/18/recursive-methods-in-expression-trees/
4485
public Expression<Func<T, T>> MakeFactorialExpression<T>()
4586
{

test/FastExpressionCompiler.TestsRunner/Program.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ void Run(Func<int> run, string name = null)
196196
Run(new Issue284_Invalid_Program_after_Coalesce().Run);
197197
Run(new FastExpressionCompiler.LightExpression.IssueTests.Issue284_Invalid_Program_after_Coalesce().Run);
198198

199-
// Run(new Issue293_Recursive_Methods().Run);
200-
// Run(new FastExpressionCompiler.LightExpression.IssueTests.Issue293_Recursive_Methods().Run);
199+
Run(new Issue293_Recursive_Methods().Run);
200+
Run(new FastExpressionCompiler.LightExpression.IssueTests.Issue293_Recursive_Methods().Run);
201201

202202
Console.WriteLine($"============={Environment.NewLine}IssueTests are passing in {sw.ElapsedMilliseconds} ms.");
203203
});

0 commit comments

Comments
 (0)