diff --git a/Directory.Build.props b/Directory.Build.props index de76c4c0..e693b4f4 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -3,6 +3,8 @@ true + 7.3 + Maksim Volkau Copyright © 2016-2019 Maksim Volkau en-US diff --git a/src/FastExpressionCompiler.LightExpression/Expression.cs b/src/FastExpressionCompiler.LightExpression/Expression.cs index 4e702170..4336b4b9 100644 --- a/src/FastExpressionCompiler.LightExpression/Expression.cs +++ b/src/FastExpressionCompiler.LightExpression/Expression.cs @@ -73,11 +73,18 @@ public static ParameterExpression Parameter(Type type, string name = null) => public static ParameterExpression Variable(Type type, string name = null) => Parameter(type, name); - public static ConstantExpression Constant(object value, Type type = null) => - value == null && type == null ? _nullExprInfo - : new ConstantExpression(value, type ?? value.GetType()); + public static ConstantExpression Constant(object value, Type type = null) + { + if (value is bool b) + return b ? _trueExpr : _falseExpr; + if (type == null) + return value != null ? new ConstantExpression(value, value.GetType()) : _nullExpr; + return new ConstantExpression(value, type); + } - private static readonly ConstantExpression _nullExprInfo = new ConstantExpression(null, typeof(object)); + private static readonly ConstantExpression _nullExpr = new ConstantExpression(null, typeof(object)); + private static readonly ConstantExpression _falseExpr = new ConstantExpression(false, typeof(bool)); + private static readonly ConstantExpression _trueExpr = new ConstantExpression(true, typeof(bool)); public static NewExpression New(Type type) => new NewExpression(type.GetTypeInfo().DeclaredConstructors.First(x => x.GetParameters().Length == 0), Tools.Empty()); diff --git a/src/FastExpressionCompiler/FastExpressionCompiler.cs b/src/FastExpressionCompiler/FastExpressionCompiler.cs index 17c5a981..3c3ab767 100644 --- a/src/FastExpressionCompiler/FastExpressionCompiler.cs +++ b/src/FastExpressionCompiler/FastExpressionCompiler.cs @@ -3282,6 +3282,7 @@ var methodName return false; EmitMethodCall(il, _objectEqualsMethod); + if (expressionType == ExpressionType.NotEqual) // invert result for not equal { il.Emit(OpCodes.Ldc_I4_0); @@ -3621,39 +3622,29 @@ private static bool TryEmitLogicalOperator(BinaryExpression expr, private static bool TryEmitConditional(ConditionalExpression expr, IReadOnlyList paramExprs, ILGenerator il, ref ClosureInfo closure, ParentFlags parent) { - var testExpr = expr.Test; - var usedInverted = false; + var testExpr = TryReduceCondition(expr.Test); + var comparedWithNull = false; - // optimization: special handling of comparing with null - if (testExpr is BinaryExpression b && - ((testExpr.NodeType == ExpressionType.Equal || testExpr.NodeType == ExpressionType.NotEqual) && - !(b.Left.Type.IsNullable() || b.Right.Type.IsNullable()) && - b.Right is ConstantExpression r && r.Value == null - ? TryEmit(b.Left, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult) - : b.Left is ConstantExpression l && l.Value == null && - TryEmit(b.Right, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult))) - { - usedInverted = true; - } + if (testExpr is BinaryExpression b && TryEmitComparisonWithNull(b, paramExprs, il, ref closure, parent)) + comparedWithNull = true; else if (!TryEmit(testExpr, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult)) return false; var labelIfFalse = il.DefineLabel(); - il.Emit(usedInverted && testExpr.NodeType == ExpressionType.Equal ? OpCodes.Brtrue : OpCodes.Brfalse, labelIfFalse); + il.Emit(comparedWithNull && testExpr.NodeType == ExpressionType.Equal ? OpCodes.Brtrue : OpCodes.Brfalse, labelIfFalse); var ifTrueExpr = expr.IfTrue; if (!TryEmit(ifTrueExpr, paramExprs, il, ref closure, parent & ParentFlags.IgnoreResult)) return false; var ifFalseExpr = expr.IfFalse; - if ((ifFalseExpr.NodeType == ExpressionType.Default) && (ifFalseExpr.Type == typeof(void))) + if (ifFalseExpr.NodeType == ExpressionType.Default && ifFalseExpr.Type == typeof(void)) { il.MarkLabel(labelIfFalse); return true; } var labelDone = il.DefineLabel(); - il.Emit(OpCodes.Br, labelDone); il.MarkLabel(labelIfFalse); @@ -3664,6 +3655,47 @@ private static bool TryEmitConditional(ConditionalExpression expr, return true; } + private static Expression TryReduceCondition(Expression testExpr) + { + if (testExpr is BinaryExpression b) + { + if (b.NodeType == ExpressionType.OrElse) + { + if (b.Left is ConstantExpression l && l.Value is bool lb) + return lb ? b.Left : TryReduceCondition(b.Right); + + if (b.Right is ConstantExpression r && r.Value is bool rb && rb == false) + return TryReduceCondition(b.Left); + } + else if (b.NodeType == ExpressionType.AndAlso) + { + if (b.Left is ConstantExpression l && l.Value is bool lb) + return !lb ? b.Left : TryReduceCondition(b.Right); + + if (b.Right is ConstantExpression r && r.Value is bool rb && rb == true) + return TryReduceCondition(b.Left); + } + } + + return testExpr; + } + + private static bool TryEmitComparisonWithNull(BinaryExpression b, + IReadOnlyList paramExprs, ILGenerator il, ref ClosureInfo closure, ParentFlags parent) + { + if (b.NodeType != ExpressionType.Equal && b.NodeType != ExpressionType.NotEqual && + (b.Left.Type.IsNullable() || b.Right.Type.IsNullable())) + return false; + + if (b.Right is ConstantExpression r && r.Value == null) + return TryEmit(b.Left, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult); + + if (b.Left is ConstantExpression l && l.Value == null) + return TryEmit(b.Right, paramExprs, il, ref closure, parent & ~ParentFlags.IgnoreResult); + + return false; + } + private static bool EmitMethodCall(ILGenerator il, MethodInfo method, ParentFlags parent = ParentFlags.Empty) { if (method == null) diff --git a/test/FastExpressionCompiler.IssueTests/FastExpressionCompiler.IssueTests.csproj b/test/FastExpressionCompiler.IssueTests/FastExpressionCompiler.IssueTests.csproj index e2aa2034..0a297403 100644 --- a/test/FastExpressionCompiler.IssueTests/FastExpressionCompiler.IssueTests.csproj +++ b/test/FastExpressionCompiler.IssueTests/FastExpressionCompiler.IssueTests.csproj @@ -1,8 +1,8 @@  net46;netcoreapp2.0 - netcoreapp2.0 - 7.1 + netcoreapp2.0 + @@ -10,6 +10,7 @@ + diff --git a/test/FastExpressionCompiler.IssueTests/Issue196_AutoMapper_tests_are_failing_when_using_FEC.cs b/test/FastExpressionCompiler.IssueTests/Issue196_AutoMapper_tests_are_failing_when_using_FEC.cs new file mode 100644 index 00000000..a9d02071 --- /dev/null +++ b/test/FastExpressionCompiler.IssueTests/Issue196_AutoMapper_tests_are_failing_when_using_FEC.cs @@ -0,0 +1,107 @@ + + +using static System.Linq.Expressions.Expression; +#if !LIGHT_EXPRESSION +using System.Linq; +using System.Reflection; +using System; +using System.Linq.Expressions; +using AutoMapper; +using NUnit.Framework; + +namespace FastExpressionCompiler.IssueTests +{ + [TestFixture] + public class Issue196_AutoMapper_tests_are_failing_when_using_FEC + { + public class FastExpressionCompilerBug + { + [Test] + public void ShouldWork() + { + var config = new MapperConfiguration(cfg => cfg.CreateMap()); + var mapper = config.CreateMapper(); + var expression = mapper.ConfigurationProvider.ExpressionBuilder.CreateMapExpression(typeof(Source), typeof(Dest)); + Assert.IsNotNull(((LambdaExpression)expression).CompileFast(true)); + + var source = new Source { Value = 5 }; + var dest = mapper.Map(source); + + Assert.AreEqual(source.Value, dest.Value); + } + + public class Source + { + public int Value { get; set; } + } + + public class Dest + { + public int Value { get; set; } + } + + [Test] + public void Comparison_with_null_should_produce_optimal_Brtrue_or_Brfalse_opcodes() + { + var sourceParam = Parameter(typeof(Source), "source"); + var body = Condition( + Equal(sourceParam, Constant(null, typeof(Source))), + Constant(null, typeof(Dest)), + MemberInit(New(typeof(Dest).GetTypeInfo().DeclaredConstructors.First()), + Bind(typeof(Dest).GetTypeInfo().DeclaredProperties.First(), + Property(sourceParam, nameof(Source.Value))))); + + var expr = Lambda>(body, sourceParam); + + //var fs = expr.Compile(); + var ff = expr.CompileFast(true); + + var dest = ff(new Source {Value = 42}); + Assert.AreEqual(42, dest.Value); + } + + [Test] + public void Logical_OrElse_should_be_reduced_if_one_of_operands_is_known_boolean_value() + { + var sourceParam = Parameter(typeof(Source), "source"); + var body = Condition( + OrElse(Equal(sourceParam, Constant(null, typeof(Source))), Constant(false)), + Constant(null, typeof(Dest)), + MemberInit(New(typeof(Dest).GetTypeInfo().DeclaredConstructors.First()), + Bind(typeof(Dest).GetTypeInfo().DeclaredProperties.First(), + Property(sourceParam, nameof(Source.Value))))); + + var expr = Lambda>(body, sourceParam); + + var fs = expr.Compile(); + var ff = expr.CompileFast(true); + + var dest = ff(new Source { Value = 42 }); + Assert.AreEqual(42, dest.Value); + } + + [Test] + public void Coalesce_should_produce_optimal_opcodes() + { + var sourceParam = Parameter(typeof(Source), "source"); + var body = Condition( + Equal(sourceParam, Constant(null)), + Constant(null, typeof(Dest)), + Coalesce( + Constant(new Dest { Value = 13 }), + MemberInit(New(typeof(Dest).GetTypeInfo().DeclaredConstructors.First()), + Bind(typeof(Dest).GetTypeInfo().DeclaredProperties.First(), + Property(sourceParam, nameof(Source.Value)))))); + + var expr = Lambda>(body, sourceParam); + + //var fs = expr.Compile(); + var ff = expr.CompileFast(true); + + var dest = ff(new Source { Value = 42 }); + Assert.AreEqual(13, dest.Value); + } + } + } +} +#endif \ No newline at end of file diff --git a/test/FastExpressionCompiler.LightExpression.IssueTests/FastExpressionCompiler.LightExpression.IssueTests.csproj b/test/FastExpressionCompiler.LightExpression.IssueTests/FastExpressionCompiler.LightExpression.IssueTests.csproj index a04b8e5b..d492f07e 100644 --- a/test/FastExpressionCompiler.LightExpression.IssueTests/FastExpressionCompiler.LightExpression.IssueTests.csproj +++ b/test/FastExpressionCompiler.LightExpression.IssueTests/FastExpressionCompiler.LightExpression.IssueTests.csproj @@ -2,7 +2,6 @@ net46;netcoreapp2.0 netcoreapp2.0 - 7.1 LIGHT_EXPRESSION diff --git a/test/FastExpressionCompiler.LightExpression.IssueTests/ObjectMethodExecutor/ObjectMethodExecutorCompiledFastClosure.cs b/test/FastExpressionCompiler.LightExpression.IssueTests/ObjectMethodExecutor/ObjectMethodExecutorCompiledFastClosure.cs index cdee145e..fcb605b2 100644 --- a/test/FastExpressionCompiler.LightExpression.IssueTests/ObjectMethodExecutor/ObjectMethodExecutorCompiledFastClosure.cs +++ b/test/FastExpressionCompiler.LightExpression.IssueTests/ObjectMethodExecutor/ObjectMethodExecutorCompiledFastClosure.cs @@ -26,12 +26,7 @@ public class ObjectMethodExecutorCompiledFastClosure private ObjectMethodExecutorCompiledFastClosure(MethodInfo methodInfo, TypeInfo targetTypeInfo, object[] parameterDefaultValues) { - if (methodInfo == null) - { - throw new ArgumentNullException(nameof(methodInfo)); - } - - MethodInfo = methodInfo; + MethodInfo = methodInfo ?? throw new ArgumentNullException(nameof(methodInfo)); MethodParameters = methodInfo.GetParameters(); TargetTypeInfo = targetTypeInfo; MethodReturnType = methodInfo.ReturnType; diff --git a/test/FastExpressionCompiler.LightExpression.UnitTests/FastExpressionCompiler.LightExpression.UnitTests.csproj b/test/FastExpressionCompiler.LightExpression.UnitTests/FastExpressionCompiler.LightExpression.UnitTests.csproj index 433001a6..418a7ca4 100644 --- a/test/FastExpressionCompiler.LightExpression.UnitTests/FastExpressionCompiler.LightExpression.UnitTests.csproj +++ b/test/FastExpressionCompiler.LightExpression.UnitTests/FastExpressionCompiler.LightExpression.UnitTests.csproj @@ -2,7 +2,6 @@ net46;netcoreapp2.0 netcoreapp2.0 - 7.1 LIGHT_EXPRESSION diff --git a/test/FastExpressionCompiler.UnitTests/FastExpressionCompiler.UnitTests.csproj b/test/FastExpressionCompiler.UnitTests/FastExpressionCompiler.UnitTests.csproj index 307f2657..1c95444b 100644 --- a/test/FastExpressionCompiler.UnitTests/FastExpressionCompiler.UnitTests.csproj +++ b/test/FastExpressionCompiler.UnitTests/FastExpressionCompiler.UnitTests.csproj @@ -2,7 +2,6 @@ net46;netcoreapp2.0 netcoreapp2.0 - 7.1