From 5c602fc671e217c1ceba97115eafcce1d3b9744f Mon Sep 17 00:00:00 2001 From: vsadov Date: Wed, 7 Oct 2015 16:56:14 -0700 Subject: [PATCH] Removed extra conversion emitted into expression trees for a lifted binary enum operator. Fixes #5734 --- .../ExpressionLambdaRewriter.cs | 37 +++- .../Emit/CodeGen/CodeGenExprLambdaTests.cs | 168 ++++++++++++++++++ 2 files changed, 198 insertions(+), 7 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs index 03211bcc07a..521130f1a03 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs @@ -414,15 +414,13 @@ private BoundExpression VisitBinaryOperator(BinaryOperatorKind opKind, MethodSym right = _bound.Default(left.Type); } - var loweredLeft = Visit(left); - var loweredRight = Visit(right); // Enums are handled as per their promoted underlying type switch (opKind.OperandTypes()) { - case BinaryOperatorKind.Enum: case BinaryOperatorKind.EnumAndUnderlying: case BinaryOperatorKind.UnderlyingAndEnum: + case BinaryOperatorKind.Enum: { var enumOperand = (opKind.OperandTypes() == BinaryOperatorKind.UnderlyingAndEnum) ? right : left; var promotedType = PromotedType(enumOperand.Type.StrippedType().GetEnumUnderlyingType()); @@ -431,18 +429,38 @@ private BoundExpression VisitBinaryOperator(BinaryOperatorKind opKind, MethodSym promotedType = _nullableType.Construct(promotedType); } - loweredLeft = PromoteEnumOperand(left, loweredLeft, promotedType, isChecked); - loweredRight = PromoteEnumOperand(right, loweredRight, promotedType, isChecked); + var loweredLeft = VisitAndPromoteEnumOperand(left, promotedType, isChecked); + var loweredRight = VisitAndPromoteEnumOperand(right, promotedType, isChecked); var result = MakeBinary(methodOpt, type, isLifted, requiresLifted, opName, loweredLeft, loweredRight); return Demote(result, type, isChecked); } default: - return MakeBinary(methodOpt, type, isLifted, requiresLifted, opName, loweredLeft, loweredRight); + { + var loweredLeft = Visit(left); + var loweredRight = Visit(right); + return MakeBinary(methodOpt, type, isLifted, requiresLifted, opName, loweredLeft, loweredRight); + } + } + } + + private static BoundExpression DemoteEnumOperand(BoundExpression left) + { + if (left.Kind == BoundKind.Conversion) + { + var conversion = (BoundConversion)left; + if (!conversion.ConversionKind.IsUserDefinedConversion() && + conversion.ConversionKind.IsImplicitConversion() && + conversion.Type.StrippedType().IsEnumType()) + { + left = conversion.Operand; + } } + + return left; } - private BoundExpression PromoteEnumOperand(BoundExpression operand, BoundExpression loweredOperand, TypeSymbol promotedType, bool isChecked) + private BoundExpression VisitAndPromoteEnumOperand(BoundExpression operand, TypeSymbol promotedType, bool isChecked) { var literal = operand as BoundLiteral; if (literal != null) @@ -452,6 +470,11 @@ private BoundExpression PromoteEnumOperand(BoundExpression operand, BoundExpress } else { + // COMPAT: if we have an operand converted to enum, we should unconvert it first + // Otherwise we will have an extra conversion in the tree: op -> enum -> underlying + // where native compiler would just directly convert to underlying + var demotedOperand = DemoteEnumOperand(operand); + var loweredOperand = Visit(demotedOperand); return Convert(loweredOperand, operand.Type, promotedType, isChecked, false); } } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambdaTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambdaTests.cs index 55f8ecb624e..70b39e5fb0a 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambdaTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambdaTests.cs @@ -4536,6 +4536,174 @@ public static void Main() expectedOutput: expectedOutput); } + [Fact, WorkItem(4471, "https://github.com/dotnet/roslyn/issues/5734")] + public void EnumEquality001() + { + string source = +@" +using System; +using System.Linq.Expressions; + +namespace ConsoleApplication1 +{ + enum YesNo + { + Yes, + No + } + + class MyType + { + public string Name { get; set; } + public YesNo? YesNo { get; set; } + + public int? Age { get; set; } + } + + class Program + { + static void Main(string[] args) + { + + Expression> expr = (MyType x) => x.YesNo == YesNo.Yes; + Console.WriteLine(expr.Dump()); + } + } + +}"; + string expectedOutput = "Equal(Convert(MemberAccess(Parameter(x Type:ConsoleApplication1.MyType).YesNo Type:System.Nullable`1[ConsoleApplication1.YesNo]) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Convert(Constant(Yes Type:ConsoleApplication1.YesNo) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Lifted Type:System.Boolean)"; + CompileAndVerify( + new[] { source, ExpressionTestLibrary }, + new[] { ExpressionAssemblyRef }, + expectedOutput: expectedOutput); + } + + [Fact, WorkItem(4471, "https://github.com/dotnet/roslyn/issues/5734")] + public void EnumEquality002() + { + string source = +@" +using System; +using System.Linq.Expressions; + +namespace ConsoleApplication1 +{ + enum YesNo + { + Yes, + No + } + + class MyType + { + public string Name { get; set; } + public YesNo? YesNo { get; set; } + + public int? Age { get; set; } + } + + class Program + { + static void Main(string[] args) + { + + Expression> expr = (MyType x) => x.YesNo == x.YesNo; + Console.WriteLine(expr.Dump()); + } + } + +}"; + string expectedOutput = "Equal(Convert(MemberAccess(Parameter(x Type:ConsoleApplication1.MyType).YesNo Type:System.Nullable`1[ConsoleApplication1.YesNo]) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Convert(MemberAccess(Parameter(x Type:ConsoleApplication1.MyType).YesNo Type:System.Nullable`1[ConsoleApplication1.YesNo]) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Lifted Type:System.Boolean)"; + CompileAndVerify( + new[] { source, ExpressionTestLibrary }, + new[] { ExpressionAssemblyRef }, + expectedOutput: expectedOutput); + } + + [Fact, WorkItem(4471, "https://github.com/dotnet/roslyn/issues/5734")] + public void EnumEquality003() + { + string source = +@" +using System; +using System.Linq.Expressions; + +namespace ConsoleApplication1 +{ + enum YesNo + { + Yes, + No + } + + class MyType + { + public string Name { get; set; } + public YesNo YesNo { get; set; } + + public int? Age { get; set; } + } + + class Program + { + static void Main(string[] args) + { + + Expression> expr = (MyType x) => x.YesNo == x.YesNo; + Console.WriteLine(expr.Dump()); + } + } + +}"; + string expectedOutput = "Equal(Convert(MemberAccess(Parameter(x Type:ConsoleApplication1.MyType).YesNo Type:ConsoleApplication1.YesNo) Type:System.Int32) Convert(MemberAccess(Parameter(x Type:ConsoleApplication1.MyType).YesNo Type:ConsoleApplication1.YesNo) Type:System.Int32) Type:System.Boolean)"; + CompileAndVerify( + new[] { source, ExpressionTestLibrary }, + new[] { ExpressionAssemblyRef }, + expectedOutput: expectedOutput); + } + + [Fact, WorkItem(4471, "https://github.com/dotnet/roslyn/issues/5734")] + public void EnumEquality004() + { + string source = +@" +using System; +using System.Linq.Expressions; + +namespace ConsoleApplication1 +{ + enum YesNo + { + Yes, + No + } + + class MyType + { + public string Name { get; set; } + public YesNo? YesNo { get; set; } + + public int? Age { get; set; } + } + + class Program + { + static void Main(string[] args) + { + + Expression> expr = (MyType x) => x.YesNo == (YesNo)1; + Console.WriteLine(expr.Dump()); + } + } + +}"; + string expectedOutput = "Equal(Convert(MemberAccess(Parameter(x Type:ConsoleApplication1.MyType).YesNo Type:System.Nullable`1[ConsoleApplication1.YesNo]) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Convert(Constant(No Type:ConsoleApplication1.YesNo) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Lifted Type:System.Boolean)"; + CompileAndVerify( + new[] { source, ExpressionTestLibrary }, + new[] { ExpressionAssemblyRef }, + expectedOutput: expectedOutput); + } + [WorkItem(546618, "DevDiv")] [Fact] public void TildeNullableEnum() -- GitLab