Skip to content

Preserve collection initializer operands across exception regions #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jogibear9988 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Preserve collection initializer operands across exception regions #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 48 additions & 46 deletions src/FastExpressionCompiler/FastExpressionCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2066,14 +2066,27 @@ private static Result TryCollectBindingInfo(ref CompilerContext context, MemberB

var inits = ((MemberListBinding)b).Initializers;
for (var i = 0; i < inits.Count; ++i)
if ((r = TryCollectElementInitInfo(ref context, inits[i], nestedLambda, ref rootNestedLambdas, scopeIdx)) != Result.OK)
return r;
return r;
}
[MethodImpl((MethodImplOptions)256)]
private static Result TryCollectElementInitInfo(ref CompilerContext context, ElementInit init,
NestedLambdaInfo nestedLambda, ref SmallList<NestedLambdaInfo> rootNestedLambdas, short scopeIdx)
{
var args = init.GetArgExprs();
for (var a = 0; a < args.GetArgCount(); ++a)
{
var args = inits[i].GetArgExprs();
for (var a = 0; a < args.GetArgCount(); ++a)
if ((r = TryCollectInfo(ref context, args.GetArgument(a), nestedLambda, ref rootNestedLambdas, scopeIdx)) != Result.OK)
return r;
var argument = args.GetArgument(a);
var precedingTryCount = context.TryExpressionCount;
var r = TryCollectInfo(ref context, argument, nestedLambda, ref rootNestedLambdas, scopeIdx);
if (r != Result.OK) return r;
if (context.TryExpressionCount != precedingTryCount)
context.ArgsContainingComplexExpression.Set.AddOrGetEntryRef(argument, out _);
}
return r;
return Result.OK;
}

private static Result TryCollectListInitExprConstants(ref CompilerContext context, ListInitExpression expr,
NestedLambdaInfo nestedLambda, ref SmallList<NestedLambdaInfo> rootNestedLambdas, short scopeIdx)
{
Expand All @@ -2085,15 +2098,12 @@ private static Result TryCollectListInitExprConstants(ref CompilerContext contex
if ((r = TryCollectInfo(ref context, newExpr, nestedLambda, ref rootNestedLambdas, scopeIdx)) != Result.OK)
return r;

var precedingTryCount = context.TryExpressionCount;
for (var i = 0; i < count; ++i)
{
var elemInit = inits.GetArgument(i);
var args = elemInit.Arguments;
var argCount = args.Count;
for (var a = 0; a < argCount; ++a)
if ((r = TryCollectInfo(ref context, args.GetArgument(a), nestedLambda, ref rootNestedLambdas, scopeIdx)) != Result.OK)
return r;
}
if ((r = TryCollectElementInitInfo(ref context, inits.GetArgument(i), nestedLambda, ref rootNestedLambdas, scopeIdx)) != Result.OK)
return r;
if (context.TryExpressionCount != precedingTryCount)
context.ArgsContainingComplexExpression.Set.AddOrGetEntryRef(expr, out _);
return r;
}

Expand Down Expand Up @@ -4635,7 +4645,7 @@ private static bool EmitNewArrayInit(NewArrayExpression expr, ILGenerator il, re
il.Demit(OpCodes.Newarr, elemType);

var isElemOfValueType = elemType.IsValueType;
if (context.ArgsContainingComplexExpression.Set.ContainsKey(expr))
if (context.HasComplexArguments(expr))
{
var array = EmitStoreLocalVariable(il, arrayType);
var value = il.GetNextLocalVarIndex(elemType);
Expand Down Expand Up @@ -4761,40 +4771,19 @@ private static bool EmitMemberSet(ILGenerator il, MemberInfo member) =>

private static bool TryEmitListInit(ListInitExpression expr, Type exprType, ILGenerator il, ref CompilerContext context, ParentFlags parent)
{
var valueVarIndex = -1;
if (exprType.IsValueType)
valueVarIndex = il.GetNextLocalVarIndex(exprType);

var newExpr = expr.NewExpression;
var argExprs = newExpr.GetArgExprs();
var argCount = argExprs.GetArgCount();
if (argCount > 0)
{
var args = newExpr.Constructor.GetParameters();
for (var i = 0; i < argCount; i++)
if (!TryEmit(argExprs.GetArgument(i), il, ref context, parent, args[i].ParameterType.IsByRef ? i : -1))
return false;
}

if (newExpr.Constructor != null)
{
il.Demit(OpCodes.Newobj, newExpr.Constructor);
if (valueVarIndex != -1)
EmitStoreLocalVariable(il, valueVarIndex);
}
else if (valueVarIndex != -1)
InitValueTypeVariable(il, exprType, valueVarIndex);
else
return false; // null constructor and not a value type, better to fallback

var valueVarIndex = exprType.IsValueType || context.HasComplexArguments(expr) ? il.GetNextLocalVarIndex(exprType) : -1;
if (!TryEmitNew(expr.NewExpression, il, ref context, ParentFlags.CtorCall))
return false;
if (valueVarIndex != -1)
EmitStoreLocalVariable(il, valueVarIndex);
var inits = expr.Initializers;
var initCount = inits.Count;
MethodInfo lastAddMethod = null;
ParameterInfo[] methodParams = null;
for (var i = 0; i < initCount; ++i)
{
if (valueVarIndex != -1) // load local value address, to set its members
EmitLoadLocalVariableAddress(il, valueVarIndex);
if (valueVarIndex != -1)
EmitLoadLocalVariableOrAddress(il, valueVarIndex, exprType.IsValueType);
else
il.Demit(OpCodes.Dup); // duplicate member owner on stack

Expand All @@ -4808,7 +4797,6 @@ private static bool TryEmitListInit(ListInitExpression expr, Type exprType, ILGe
if (!EmitElementInit(elemInit, exprType, il, ref context, parent, methodParams))
return false;
}

if (valueVarIndex != -1)
EmitLoadLocalVariable(il, valueVarIndex);
context.LastEmitIsAddress = false;
Expand All @@ -4819,16 +4807,30 @@ private static bool TryEmitListInit(ListInitExpression expr, Type exprType, ILGe
private static bool EmitElementInit(ElementInit elemInit, Type ownerType, ILGenerator il, ref CompilerContext context, ParentFlags parent, ParameterInfo[] methodParams = null)
{
// see the TryEmitMethodCall for the reason of the callFlags
var callFlags = (parent & ~CallArgumentFlagsToClear) | ParentFlags.Call;
var callFlags = (parent & ~CallArgumentFlagsToClear) | ParentFlags.Call | ParentFlags.AssignmentRightValue;
var method = elemInit.AddMethod;
if (methodParams == null)
methodParams = method.GetParameters();
var addArgs = elemInit.GetArgExprs();
var addArgCount = addArgs.GetArgCount();

var spillArguments = false;
if (context.TryExpressionCount != 0)
for (var a = 0; a < addArgCount && !spillArguments; ++a)
spillArguments = context.HasComplexArguments(addArgs.GetArgument(a));
var receiver = spillArguments ? EmitStoreLocalVariable(il, ownerType.IsValueType ? ownerType.MakeByRefType() : ownerType) : -1;
SmallList<int, Stack8<int>, NoArrayPool<int>> argVars = default;
for (var a = 0; a < addArgCount; ++a)
if (!TryEmit(addArgs.GetArgument(a), il, ref context, callFlags, methodParams[a].ParameterType.IsByRef ? a : -1))
return false;
{
var type = methodParams[a].ParameterType;
if (!TryEmit(addArgs.GetArgument(a), il, ref context, callFlags, type.IsByRef ? a : -1)) return false;
if (spillArguments) argVars.Add(EmitStoreLocalVariable(il, type));
}
if (spillArguments)
{
EmitLoadLocalVariable(il, receiver);
for (var a = 0; a < addArgCount; ++a) EmitLoadLocalVariable(il, argVars[a]);
}

var ok = EmitInstanceMethodCall(il, method, ownerType);
if (ok && method.ReturnType != typeof(void))
Expand Down
2 changes: 2 additions & 0 deletions test/FastExpressionCompiler.TestsRunner.Net472/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ static void RunLightExpressionTests(object state)
t.Run(new LightExpression.UnitTests.IndexerReadExceptionRegressionTests());
t.Run(new LightExpression.UnitTests.ArrayBoundConversionRegressionTests());
t.Run(new LightExpression.UnitTests.CheckedFloatingArithmeticRegressionTests());
t.Run(new LightExpression.UnitTests.CollectionInitializerExceptionTests());
t.Run(new LightExpression.UnitTests.OperandValueContextRegressionTests());
t.Run(new LightExpression.UnitTests.CompoundIndexExceptionRegressionTests());
t.Run(new LightExpression.UnitTests.ExcessiveArityRegressionTests());
Expand Down Expand Up @@ -121,6 +122,7 @@ static void RunLightExpressionTests(object state)
fecTests.Run(new IndexerReadExceptionRegressionTests());
fecTests.Run(new ArrayBoundConversionRegressionTests());
fecTests.Run(new CheckedFloatingArithmeticRegressionTests());
fecTests.Run(new CollectionInitializerExceptionTests());
fecTests.Run(new OperandValueContextRegressionTests());
fecTests.Run(new CompoundIndexExceptionRegressionTests());
fecTests.Run(new ExcessiveArityRegressionTests());
Expand Down
2 changes: 2 additions & 0 deletions test/FastExpressionCompiler.TestsRunner/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public static void Main(string[] args)
lt.Run(new LightExpression.UnitTests.IndexerReadExceptionRegressionTests());
lt.Run(new LightExpression.UnitTests.ArrayBoundConversionRegressionTests());
lt.Run(new LightExpression.UnitTests.CheckedFloatingArithmeticRegressionTests());
lt.Run(new LightExpression.UnitTests.CollectionInitializerExceptionTests());
lt.Run(new LightExpression.UnitTests.OperandValueContextRegressionTests());
lt.Run(new LightExpression.UnitTests.CompoundIndexExceptionRegressionTests());
lt.Run(new LightExpression.UnitTests.ExcessiveArityRegressionTests());
Expand Down Expand Up @@ -147,6 +148,7 @@ public static void Main(string[] args)
st.Run(new IndexerReadExceptionRegressionTests());
st.Run(new ArrayBoundConversionRegressionTests());
st.Run(new CheckedFloatingArithmeticRegressionTests());
st.Run(new CollectionInitializerExceptionTests());
st.Run(new OperandValueContextRegressionTests());
st.Run(new CompoundIndexExceptionRegressionTests());
st.Run(new ExcessiveArityRegressionTests());
Expand Down
44 changes: 44 additions & 0 deletions test/FastExpressionCompiler.UnitTests/CollectionInitializerExceptionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using System;
using System.Collections;
using System.Collections.Generic;
#if LIGHT_EXPRESSION
using E = FastExpressionCompiler.LightExpression.Expression;
namespace FastExpressionCompiler.LightExpression.UnitTests;
#else
using E = System.Linq.Expressions.Expression;
namespace FastExpressionCompiler.UnitTests;
#endif
public struct CollectionInitializerExceptionTests : ITestX
{
public void Run(TestRun t) { Reference_collection_arguments_keep_order(t); Value_collection_arguments_keep_order(t); Throwing_argument_prevents_Add_and_later_elements(t); Constructor_arguments_support_exception_regions(t); }
public class Items : IEnumerable { public int Total; public Items() {} public Items(int a,int b){Total=a+b;} public int Add(int a,int b){Total+=a+b;return Total;} public IEnumerator GetEnumerator()=>throw new NotImplementedException(); }
public struct ValueItems : IEnumerable { public int Total; public int Add(int a,int b){Total+=a+b;return Total;} public IEnumerator GetEnumerator()=>throw new NotImplementedException(); }
public void Reference_collection_arguments_keep_order(TestContext t) { Check(t,typeof(Items)); }
public void Value_collection_arguments_keep_order(TestContext t) { Check(t,typeof(ValueItems)); }
static void Check(TestContext t,Type type) {
foreach(var ignored in new[]{false,true}) {
var log=new List<int>(); Func<int,int> mark=x=>{log.Add(x);return x;};
Func<int,E> arg=x=>E.Invoke(E.Constant(mark),E.Constant(x));
var init=E.ListInit(E.New(type),E.ElementInit(type.GetMethod("Add"),arg(1),E.TryFinally(arg(2),E.Block(arg(3),E.Empty()))),E.ElementInit(type.GetMethod("Add"),E.TryFinally(arg(4),E.Empty()),arg(5)));
E body=ignored ? E.Block(init,E.Constant(12)) : E.Field(init,"Total");
var f=E.Lambda<Func<int>>(body).CompileFast(true);
if(!t.IsNotNull(f))return;t.AreEqual(12,f());t.AreEqual("1,2,3,4,5",string.Join(",",log));
}
}
public void Throwing_argument_prevents_Add_and_later_elements(TestContext t) {
var log=new List<int>(); Func<int,int> mark=x=>{log.Add(x);if(x==2)throw new InvalidOperationException();return x;};
Func<int,E> arg=x=>E.Invoke(E.Constant(mark),E.Constant(x));
var init=E.ListInit(E.New(typeof(Items)),E.ElementInit(typeof(Items).GetMethod("Add"),arg(1),E.TryFinally(arg(2),E.Block(arg(3),E.Empty()))),E.ElementInit(typeof(Items).GetMethod("Add"),arg(4),arg(5)));
var f=E.Lambda<Func<Items>>(init).CompileFast(true);if(!t.IsNotNull(f))return;t.Throws<InvalidOperationException>(()=>f());t.AreEqual("1,2,3",string.Join(",",log));
}
public void Constructor_arguments_support_exception_regions(TestContext t) {
foreach(var ignored in new[]{false,true}) {
var log=new List<int>(); Func<int,int> mark=x=>{log.Add(x);return x;};
Func<int,E> arg=x=>E.Invoke(E.Constant(mark),E.Constant(x));
var init=E.ListInit(E.New(typeof(Items).GetConstructor(new[]{typeof(int),typeof(int)}),arg(7),E.TryFinally(arg(9),E.Empty())),E.ElementInit(typeof(Items).GetMethod("Add"),arg(2),E.TryFinally(arg(3),E.Empty())));
E body=ignored?E.Block(init,E.Constant(21)):E.Field(init,"Total");
var f=E.Lambda<Func<int>>(body).CompileFast(true);if(t.IsNotNull(f))t.AreEqual(21,f());t.AreEqual("7,9,2,3",string.Join(",",log));
}
}

}
Loading