Skip to content

Commit

Permalink
Merge pull request #35664 from gafter/dev16.1-35584
Browse files Browse the repository at this point in the history 
Fix bug where pattern-matching treats a type parameter as if it is a reference (which it might not be)
  • Loading branch information
@jcouv
jcouv authored May 14, 2019
2 parents 9361041 + 794651d commit 04cca4a
Show file tree
Hide file tree
Showing 3 changed files with 336 additions and 94 deletions.
34 changes: 27 additions & 7 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,11 @@ void addArg(RefKind refKind, BoundExpression expression)
case BoundDagTypeEvaluation t:
{
TypeSymbol inputType = input.Type;
if (inputType.IsDynamic() || inputType.ContainsTypeParameter())
if (inputType.IsDynamic())
{
// Avoid using dynamic conversions for pattern-matching.
inputType = _factory.SpecialType(SpecialType.System_Object);
input = _factory.Convert(inputType, input);
}

TypeSymbol type = t.Type;
Expand Down Expand Up @@ -335,20 +337,38 @@ protected bool TryLowerTypeTestAndCast(
out BoundExpression sideEffect,
out BoundExpression testExpression)
{
HashSet<DiagnosticInfo> useSiteDiagnostics = null;

// case 1: type test followed by cast to that type
if (test is BoundDagTypeTest typeDecision &&
evaluation is BoundDagTypeEvaluation typeEvaluation &&
evaluation is BoundDagTypeEvaluation typeEvaluation1 &&
typeDecision.Type.IsReferenceType &&
typeEvaluation.Type.Equals(typeDecision.Type, TypeCompareKind.AllIgnoreOptions) &&
typeEvaluation.Input == typeDecision.Input
)
typeEvaluation1.Type.Equals(typeDecision.Type, TypeCompareKind.AllIgnoreOptions) &&
typeEvaluation1.Input == typeDecision.Input)
{
BoundExpression input = _tempAllocator.GetTemp(test.Input);
BoundExpression output = _tempAllocator.GetTemp(new BoundDagTemp(evaluation.Syntax, typeEvaluation.Type, evaluation));
sideEffect = _factory.AssignmentExpression(output, _factory.As(input, typeEvaluation.Type));
BoundExpression output = _tempAllocator.GetTemp(new BoundDagTemp(evaluation.Syntax, typeEvaluation1.Type, evaluation));
sideEffect = _factory.AssignmentExpression(output, _factory.As(input, typeEvaluation1.Type));
testExpression = _factory.ObjectNotEqual(output, _factory.Null(output.Type));
return true;
}

// case 2: null check followed by cast to a base type
if (test is BoundDagNonNullTest nonNullTest &&
evaluation is BoundDagTypeEvaluation typeEvaluation2 &&
_factory.Compilation.Conversions.ClassifyBuiltInConversion(test.Input.Type, typeEvaluation2.Type, ref useSiteDiagnostics) is Conversion conv &&
(conv.IsIdentity || conv.Kind == ConversionKind.ImplicitReference || conv.IsBoxing) &&
typeEvaluation2.Input == nonNullTest.Input)
{
BoundExpression input = _tempAllocator.GetTemp(test.Input);
var baseType = typeEvaluation2.Type;
BoundExpression output = _tempAllocator.GetTemp(new BoundDagTemp(evaluation.Syntax, baseType, evaluation));
sideEffect = _factory.AssignmentExpression(output, _factory.Convert(baseType, input));
testExpression = _factory.ObjectNotEqual(output, _factory.Null(baseType));
_localRewriter._diagnostics.Add(test.Syntax, useSiteDiagnostics);
return true;
}

sideEffect = testExpression = null;
return false;
}
Expand Down
247 changes: 229 additions & 18 deletions src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1544,21 +1544,23 @@ .maxstack 2
}");
compVerifier.VerifyIL("Program.Test2<T>(T)",
@"{
// Code size 30 (0x1e)
// Code size 35 (0x23)
.maxstack 2
IL_0000: ldarg.0
IL_0001: box ""T""
IL_0006: isinst ""int""
IL_000b: brfalse.s IL_001c
IL_000b: brfalse.s IL_0021
IL_000d: ldarg.0
IL_000e: box ""T""
IL_0013: unbox.any ""int""
IL_0018: ldc.i4.0
IL_0019: ceq
IL_001b: ret
IL_001c: ldc.i4.0
IL_001d: ret
}");
IL_0013: isinst ""int""
IL_0018: unbox.any ""int""
IL_001d: ldc.i4.0
IL_001e: ceq
IL_0020: ret
IL_0021: ldc.i4.0
IL_0022: ret
}
");
compVerifier.VerifyIL("Program.Test3<T>(T)",
@"{
// Code size 29 (0x1d)
Expand Down Expand Up @@ -1598,21 +1600,23 @@ static bool Test(S s)
var compVerifier = CompileAndVerify(compilation);
compVerifier.VerifyIL("C<T>.Test(C<T>.S)",
@"{
// Code size 30 (0x1e)
// Code size 35 (0x23)
.maxstack 2
IL_0000: ldarg.0
IL_0001: box ""C<T>.S""
IL_0006: isinst ""int""
IL_000b: brfalse.s IL_001c
IL_000b: brfalse.s IL_0021
IL_000d: ldarg.0
IL_000e: box ""C<T>.S""
IL_0013: unbox.any ""int""
IL_0018: ldc.i4.1
IL_0019: ceq
IL_001b: ret
IL_001c: ldc.i4.0
IL_001d: ret
}");
IL_0013: isinst ""int""
IL_0018: unbox.any ""int""
IL_001d: ldc.i4.1
IL_001e: ceq
IL_0020: ret
IL_0021: ldc.i4.0
IL_0022: ret
}
");
}

[Fact]
Expand Down Expand Up @@ -2385,5 +2389,212 @@ .locals init (bool V_0)
}
}
}

[Fact, WorkItem(35584, "https://github.com/dotnet/roslyn/issues/35584")]
public void MatchToTypeParameterUnbox_01()
{
var source = @"
class Program
{
public static void Main() => System.Console.WriteLine(P<int>(0));
public static string P<T>(T t) => (t is object o) ? o.ToString() : string.Empty;
}
";
var expectedOutput = @"0";
foreach (var options in new[] { TestOptions.DebugExe, TestOptions.ReleaseExe })
{
var compilation = CreateCompilation(source, options: options);
compilation.VerifyDiagnostics();
var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
if (options.OptimizationLevel == OptimizationLevel.Debug)
{
compVerifier.VerifyIL("Program.P<T>",
@"{
// Code size 24 (0x18)
.maxstack 1
.locals init (object V_0) //o
IL_0000: ldarg.0
IL_0001: box ""T""
IL_0006: stloc.0
IL_0007: ldloc.0
IL_0008: brtrue.s IL_0011
IL_000a: ldsfld ""string string.Empty""
IL_000f: br.s IL_0017
IL_0011: ldloc.0
IL_0012: callvirt ""string object.ToString()""
IL_0017: ret
}
");
}
else
{
compVerifier.VerifyIL("Program.P<T>",
@"{
// Code size 23 (0x17)
.maxstack 1
.locals init (object V_0) //o
IL_0000: ldarg.0
IL_0001: box ""T""
IL_0006: stloc.0
IL_0007: ldloc.0
IL_0008: brtrue.s IL_0010
IL_000a: ldsfld ""string string.Empty""
IL_000f: ret
IL_0010: ldloc.0
IL_0011: callvirt ""string object.ToString()""
IL_0016: ret
}
");
}
}
}

[Fact, WorkItem(35584, "https://github.com/dotnet/roslyn/issues/35584")]
public void MatchToTypeParameterUnbox_02()
{
var source =
@"using System;
class Program
{
public static void Main()
{
var generic = new Generic<int>(0);
}
}
public class Generic<T>
{
public Generic(T value)
{
if (value is object obj && obj == null)
{
throw new Exception(""Kaboom!"");
}
}
}
";
var expectedOutput = @"";
foreach (var options in new[] { TestOptions.DebugExe, TestOptions.ReleaseExe })
{
var compilation = CreateCompilation(source, options: options);
compilation.VerifyDiagnostics();
var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
if (options.OptimizationLevel == OptimizationLevel.Debug)
{
compVerifier.VerifyIL("Generic<T>..ctor(T)",
@"{
// Code size 42 (0x2a)
.maxstack 2
.locals init (object V_0, //obj
bool V_1)
IL_0000: ldarg.0
IL_0001: call ""object..ctor()""
IL_0006: nop
IL_0007: nop
IL_0008: ldarg.1
IL_0009: box ""T""
IL_000e: stloc.0
IL_000f: ldloc.0
IL_0010: brfalse.s IL_0018
IL_0012: ldloc.0
IL_0013: ldnull
IL_0014: ceq
IL_0016: br.s IL_0019
IL_0018: ldc.i4.0
IL_0019: stloc.1
IL_001a: ldloc.1
IL_001b: brfalse.s IL_0029
IL_001d: nop
IL_001e: ldstr ""Kaboom!""
IL_0023: newobj ""System.Exception..ctor(string)""
IL_0028: throw
IL_0029: ret
}
");
}
else
{
compVerifier.VerifyIL("Generic<T>..ctor(T)",
@"{
// Code size 31 (0x1f)
.maxstack 1
.locals init (object V_0) //obj
IL_0000: ldarg.0
IL_0001: call ""object..ctor()""
IL_0006: ldarg.1
IL_0007: box ""T""
IL_000c: stloc.0
IL_000d: ldloc.0
IL_000e: brfalse.s IL_001e
IL_0010: ldloc.0
IL_0011: brtrue.s IL_001e
IL_0013: ldstr ""Kaboom!""
IL_0018: newobj ""System.Exception..ctor(string)""
IL_001d: throw
IL_001e: ret
}
");
}
}
}

[Fact, WorkItem(35584, "https://github.com/dotnet/roslyn/issues/35584")]
public void MatchToTypeParameterUnbox_03()
{
var source =
@"using System;
class Program
{
public static void Main() => System.Console.WriteLine(P<Enum>(null));
public static string P<T>(T t) where T: Enum => (t is ValueType o) ? o.ToString() : ""1"";
}
";
var expectedOutput = @"1";
foreach (var options in new[] { TestOptions.DebugExe, TestOptions.ReleaseExe })
{
var compilation = CreateCompilation(source, options: options);
compilation.VerifyDiagnostics();
var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
if (options.OptimizationLevel == OptimizationLevel.Debug)
{
compVerifier.VerifyIL("Program.P<T>",
@"{
// Code size 24 (0x18)
.maxstack 1
.locals init (System.ValueType V_0) //o
IL_0000: ldarg.0
IL_0001: box ""T""
IL_0006: stloc.0
IL_0007: ldloc.0
IL_0008: brtrue.s IL_0011
IL_000a: ldstr ""1""
IL_000f: br.s IL_0017
IL_0011: ldloc.0
IL_0012: callvirt ""string object.ToString()""
IL_0017: ret
}
");
}
else
{
compVerifier.VerifyIL("Program.P<T>",
@"{
// Code size 23 (0x17)
.maxstack 1
.locals init (System.ValueType V_0) //o
IL_0000: ldarg.0
IL_0001: box ""T""
IL_0006: stloc.0
IL_0007: ldloc.0
IL_0008: brtrue.s IL_0010
IL_000a: ldstr ""1""
IL_000f: ret
IL_0010: ldloc.0
IL_0011: callvirt ""string object.ToString()""
IL_0016: ret
}
");
}
}
}
}
}
Loading

0 comments on commit 04cca4a

Please sign in to comment.