From a7c54253b49ee8064f105614b06b39165f3a4e7d Mon Sep 17 00:00:00 2001 From: Neal Gafter Date: Fri, 6 Apr 2018 10:20:09 -0700 Subject: [PATCH] Permit a constant pattern to be used with an open type as input Fixes #24550 Fixes https://github.com/dotnet/csharplang/issues/1284 --- .../CSharp/Portable/Binder/Binder_Patterns.cs | 63 ++++++----- .../LocalRewriter/LocalRewriter_Patterns.cs | 3 +- .../Test/Emit/CodeGen/CodeGenOperators.cs | 50 ++++++-- .../CSharp/Test/Emit/CodeGen/PatternTests.cs | 107 ++++++++++++++++++ .../CSharp/Test/Emit/CodeGen/SwitchTests.cs | 86 +++++++------- 5 files changed, 228 insertions(+), 81 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs index 28fe10a8410..7dd4405c739 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs @@ -168,35 +168,44 @@ private BoundPattern BindDiscardPattern(DiscardPatternSyntax node, TypeSymbol op internal BoundExpression ConvertPatternExpression(TypeSymbol inputType, CSharpSyntaxNode node, BoundExpression expression, ref ConstantValue constantValue, DiagnosticBag diagnostics) { - // NOTE: This will allow user-defined conversions, even though they're not allowed here. This is acceptable - // because the result of a user-defined conversion does not have a ConstantValue and we'll report a diagnostic - // to that effect later. - BoundExpression convertedExpression = GenerateConversionForAssignment(inputType, expression, diagnostics); - - if (convertedExpression.Kind == BoundKind.Conversion) + BoundExpression convertedExpression; + if (inputType.ContainsTypeParameter()) { - var conversion = (BoundConversion)convertedExpression; - BoundExpression operand = conversion.Operand; - if (inputType.IsNullableType() && (convertedExpression.ConstantValue == null || !convertedExpression.ConstantValue.IsNull)) - { - // Null is a special case here because we want to compare null to the Nullable itself, not to the underlying type. - var discardedDiagnostics = DiagnosticBag.GetInstance(); // We are not intested in the diagnostic that get created here - convertedExpression = CreateConversion(operand, inputType.GetNullableUnderlyingType(), discardedDiagnostics); - discardedDiagnostics.Free(); - } - else if ((conversion.ConversionKind == ConversionKind.Boxing || conversion.ConversionKind == ConversionKind.ImplicitReference) - && operand.ConstantValue != null && convertedExpression.ConstantValue == null) - { - // A boxed constant (or string converted to object) is a special case because we prefer - // to compare to the pre-converted value by casting the input value to the type of the constant - // (that is, unboxing or downcasting it) and then testing the resulting value using primitives. - // That is much more efficient than calling object.Equals(x, y), and we can share the downcasted - // input value among many constant tests. - convertedExpression = operand; - } - else if (conversion.ConversionKind == ConversionKind.NoConversion && convertedExpression.Type?.IsErrorType() == true) + // If we are pattern-matching against an open type, we do not convert the constant to the type of the input. + convertedExpression = expression; + } + else + { + // NOTE: This will allow user-defined conversions, even though they're not allowed here. This is acceptable + // because the result of a user-defined conversion does not have a ConstantValue and we'll report a diagnostic + // to that effect later. + convertedExpression = GenerateConversionForAssignment(inputType, expression, diagnostics); + + if (convertedExpression.Kind == BoundKind.Conversion) { - convertedExpression = operand; + var conversion = (BoundConversion)convertedExpression; + BoundExpression operand = conversion.Operand; + if (inputType.IsNullableType() && (convertedExpression.ConstantValue == null || !convertedExpression.ConstantValue.IsNull)) + { + // Null is a special case here because we want to compare null to the Nullable itself, not to the underlying type. + var discardedDiagnostics = DiagnosticBag.GetInstance(); // We are not intested in the diagnostic that get created here + convertedExpression = CreateConversion(operand, inputType.GetNullableUnderlyingType(), discardedDiagnostics); + discardedDiagnostics.Free(); + } + else if ((conversion.ConversionKind == ConversionKind.Boxing || conversion.ConversionKind == ConversionKind.ImplicitReference) + && operand.ConstantValue != null && convertedExpression.ConstantValue == null) + { + // A boxed constant (or string converted to object) is a special case because we prefer + // to compare to the pre-converted value by casting the input value to the type of the constant + // (that is, unboxing or downcasting it) and then testing the resulting value using primitives. + // That is much more efficient than calling object.Equals(x, y), and we can share the downcasted + // input value among many constant tests. + convertedExpression = operand; + } + else if (conversion.ConversionKind == ConversionKind.NoConversion && convertedExpression.Type?.IsErrorType() == true) + { + convertedExpression = operand; + } } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs index c90b4598ed1..92ef98e1e94 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs @@ -160,7 +160,7 @@ void addArg(RefKind refKind, BoundExpression expression) case BoundDagTypeEvaluation t: { TypeSymbol inputType = input.Type; - if (inputType.IsDynamic()) + if (inputType.IsDynamic() || inputType.ContainsTypeParameter()) { inputType = _factory.SpecialType(SpecialType.System_Object); } @@ -252,7 +252,6 @@ protected BoundExpression LowerTest(BoundDagTest test) if (test is BoundDagTypeTest typeDecision && evaluation is BoundDagTypeEvaluation typeEvaluation && typeDecision.Type.IsReferenceType && - typeDecision.Input.Type.IsReferenceType && typeEvaluation.Type == typeDecision.Type && typeEvaluation.Input == typeDecision.Input ) diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs index 9abeddb4f13..14423714adf 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOperators.cs @@ -80,13 +80,49 @@ public static void Main() } "; - CreateCompilation(source).VerifyDiagnostics( - // (7,28): error CS0403: Cannot convert null to type parameter 'T' because it could be a non-nullable value type. Consider using 'default(T)' instead. - // Console.Write(o is null); - Diagnostic(ErrorCode.ERR_TypeVarCantBeNull, "null").WithArguments("T").WithLocation(7, 28), - // (8,18): error CS0403: Cannot convert null to type parameter 'T' because it could be a non-nullable value type. Consider using 'default(T)' instead. - // if (o is null) - Diagnostic(ErrorCode.ERR_TypeVarCantBeNull, "null").WithArguments("T").WithLocation(8, 18)); + var compilation = CompileAndVerify(source, expectedOutput: "False", options: TestOptions.ReleaseExe); + compilation.VerifyIL("C.M(T)", @"{ + // Code size 33 (0x21) + .maxstack 2 + IL_0000: ldarg.0 + IL_0001: box ""T"" + IL_0006: ldnull + IL_0007: ceq + IL_0009: call ""void System.Console.Write(bool)"" + IL_000e: ldarg.0 + IL_000f: box ""T"" + IL_0014: brtrue.s IL_0020 + IL_0016: ldstr ""Branch taken"" + IL_001b: call ""void System.Console.Write(string)"" + IL_0020: ret +}"); + // Debug + compilation = CompileAndVerify(source, expectedOutput: "False", options: TestOptions.DebugExe); + compilation.VerifyIL("C.M(T)", @"{ + // Code size 43 (0x2b) + .maxstack 2 + .locals init (bool V_0) + IL_0000: nop + IL_0001: ldarg.0 + IL_0002: box ""T"" + IL_0007: ldnull + IL_0008: ceq + IL_000a: call ""void System.Console.Write(bool)"" + IL_000f: nop + IL_0010: ldarg.0 + IL_0011: box ""T"" + IL_0016: ldnull + IL_0017: ceq + IL_0019: stloc.0 + IL_001a: ldloc.0 + IL_001b: brfalse.s IL_002a + IL_001d: nop + IL_001e: ldstr ""Branch taken"" + IL_0023: call ""void System.Console.Write(string)"" + IL_0028: nop + IL_0029: nop + IL_002a: ret +}"); } [Fact] diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs index bd0310d4832..baf5182855f 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs @@ -1125,6 +1125,113 @@ .maxstack 2 IL_0013: ret IL_0014: ldc.i4.0 IL_0015: ret +}"); + } + + [Fact] + [WorkItem(24550, "https://github.com/dotnet/roslyn/issues/24550")] + [WorkItem(1284, "https://github.com/dotnet/csharplang/issues/1284")] + public void ConstantPatternVsUnconstrainedTypeParameter01() + { + var source = +@"using System; +class Program +{ + static void Main() + { + Console.WriteLine(Test1(null)); + Console.WriteLine(Test1(1)); + Console.WriteLine(Test1(null)); + Console.WriteLine(Test1(1)); + + Console.WriteLine(Test2(0)); + Console.WriteLine(Test2(1)); + Console.WriteLine(Test2(0)); + Console.WriteLine(Test2(""frog"")); + + Console.WriteLine(Test3(""frog"")); + Console.WriteLine(Test3(1)); + Console.WriteLine(Test3(""frog"")); + Console.WriteLine(Test3(1)); + } + + public static bool Test1(T t) + { + return t is null; + } + public static bool Test2(T t) + { + return t is 0; + } + public static bool Test3(T t) + { + return t is ""frog""; + } +}"; + var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe); + compilation.VerifyDiagnostics(); + var expectedOutput = +@"True +False +True +False +True +False +True +False +True +False +True +False +"; + var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput); + compVerifier.VerifyIL("Program.Test1(T)", +@"{ + // Code size 10 (0xa) + .maxstack 2 + IL_0000: ldarg.0 + IL_0001: box ""T"" + IL_0006: ldnull + IL_0007: ceq + IL_0009: ret +}"); + compVerifier.VerifyIL("Program.Test2(T)", +@"{ + // Code size 32 (0x20) + .maxstack 2 + .locals init (int V_0) + IL_0000: ldarg.0 + IL_0001: box ""T"" + IL_0006: isinst ""int"" + IL_000b: brfalse.s IL_001e + IL_000d: ldarg.0 + IL_000e: box ""T"" + IL_0013: unbox.any ""int"" + IL_0018: stloc.0 + IL_0019: ldloc.0 + IL_001a: ldc.i4.0 + IL_001b: ceq + IL_001d: ret + IL_001e: ldc.i4.0 + IL_001f: ret +}"); + compVerifier.VerifyIL("Program.Test3(T)", +@"{ + // Code size 29 (0x1d) + .maxstack 2 + .locals init (string V_0) + IL_0000: ldarg.0 + IL_0001: box ""T"" + IL_0006: isinst ""string"" + IL_000b: stloc.0 + IL_000c: ldloc.0 + IL_000d: brfalse.s IL_001b + IL_000f: ldstr ""frog"" + IL_0014: ldloc.0 + IL_0015: call ""bool string.op_Equality(string, string)"" + IL_001a: ret + IL_001b: ldc.i4.0 + IL_001c: ret }"); } } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/SwitchTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/SwitchTests.cs index 43983f793aa..d6a21781eb4 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/SwitchTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/SwitchTests.cs @@ -8508,40 +8508,38 @@ public static int M2(T o) expectedOutput: "2300"); compVerifier.VerifyIL("Program.M1", @"{ - // Code size 36 (0x24) + // Code size 31 (0x1f) .maxstack 1 .locals init (int V_0) //t IL_0000: ldarg.0 IL_0001: box ""T"" IL_0006: isinst ""int"" - IL_000b: brfalse.s IL_0020 + IL_000b: brfalse.s IL_001b IL_000d: ldarg.0 IL_000e: box ""T"" - IL_0013: isinst ""int"" - IL_0018: unbox.any ""int"" - IL_001d: stloc.0 - IL_001e: br.s IL_0022 - IL_0020: ldc.i4.0 - IL_0021: ret - IL_0022: ldloc.0 - IL_0023: ret + IL_0013: unbox.any ""int"" + IL_0018: stloc.0 + IL_0019: br.s IL_001d + IL_001b: ldc.i4.0 + IL_001c: ret + IL_001d: ldloc.0 + IL_001e: ret }" ); compVerifier.VerifyIL("Program.M2", @"{ - // Code size 32 (0x20) + // Code size 27 (0x1b) .maxstack 1 IL_0000: ldarg.0 IL_0001: box ""T"" IL_0006: isinst ""int"" - IL_000b: brfalse.s IL_001e + IL_000b: brfalse.s IL_0019 IL_000d: ldarg.0 IL_000e: box ""T"" - IL_0013: isinst ""int"" - IL_0018: unbox.any ""int"" - IL_001d: ret - IL_001e: ldc.i4.0 - IL_001f: ret + IL_0013: unbox.any ""int"" + IL_0018: ret + IL_0019: ldc.i4.0 + IL_001a: ret }" ); compVerifier = CompileAndVerify(source, @@ -8550,7 +8548,7 @@ .maxstack 1 expectedOutput: "2300"); compVerifier.VerifyIL("Program.M1", @"{ - // Code size 42 (0x2a) + // Code size 37 (0x25) .maxstack 1 .locals init (int V_0, //t int V_1) @@ -8558,25 +8556,24 @@ .maxstack 1 IL_0001: ldarg.0 IL_0002: box ""T"" IL_0007: isinst ""int"" - IL_000c: brfalse.s IL_0021 + IL_000c: brfalse.s IL_001c IL_000e: ldarg.0 IL_000f: box ""T"" - IL_0014: isinst ""int"" - IL_0019: unbox.any ""int"" - IL_001e: stloc.0 - IL_001f: br.s IL_0024 - IL_0021: ldc.i4.0 - IL_0022: br.s IL_0025 - IL_0024: ldloc.0 - IL_0025: stloc.1 - IL_0026: br.s IL_0028 - IL_0028: ldloc.1 - IL_0029: ret + IL_0014: unbox.any ""int"" + IL_0019: stloc.0 + IL_001a: br.s IL_001f + IL_001c: ldc.i4.0 + IL_001d: br.s IL_0020 + IL_001f: ldloc.0 + IL_0020: stloc.1 + IL_0021: br.s IL_0023 + IL_0023: ldloc.1 + IL_0024: ret }" ); compVerifier.VerifyIL("Program.M2", @"{ - // Code size 50 (0x32) + // Code size 45 (0x2d) .maxstack 1 .locals init (int V_0, //t T V_1, @@ -8589,22 +8586,21 @@ .maxstack 1 IL_0006: ldarg.0 IL_0007: box ""T"" IL_000c: isinst ""int"" - IL_0011: brfalse.s IL_002c + IL_0011: brfalse.s IL_0027 IL_0013: ldarg.0 IL_0014: box ""T"" - IL_0019: isinst ""int"" - IL_001e: unbox.any ""int"" - IL_0023: stloc.0 - IL_0024: br.s IL_0026 - IL_0026: br.s IL_0028 - IL_0028: ldloc.0 - IL_0029: stloc.2 - IL_002a: br.s IL_0030 - IL_002c: ldc.i4.0 - IL_002d: stloc.2 - IL_002e: br.s IL_0030 - IL_0030: ldloc.2 - IL_0031: ret + IL_0019: unbox.any ""int"" + IL_001e: stloc.0 + IL_001f: br.s IL_0021 + IL_0021: br.s IL_0023 + IL_0023: ldloc.0 + IL_0024: stloc.2 + IL_0025: br.s IL_002b + IL_0027: ldc.i4.0 + IL_0028: stloc.2 + IL_0029: br.s IL_002b + IL_002b: ldloc.2 + IL_002c: ret }" ); } -- GitLab