Skip to content
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

Classify pattern-matching temp as long-lived #18756

Merged
merged 6 commits into from
Apr 19, 2017
+282 −95
Merged

Classify pattern-matching temp as long-lived #18756

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a142267
Classify pattern-matching temp as long-lived
gafter Apr 17, 2017
f83d864
Add test demonstrating correct code for spilling pattern termps.
gafter Apr 17, 2017
1e86626
Merge branch 'master' of https://github.com/dotnet/roslyn into master…
gafter Apr 17, 2017
ab4f844
Update test to reflect change in status of pattern temps.
gafter Apr 17, 2017
38a4af1
Test generated code for pattern switch (before temp change)
gafter Apr 18, 2017
437f0e8
Add test for precise IL for pattern switch.
gafter Apr 19, 2017
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
83 changes: 83 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAsyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4952,5 +4952,88 @@ namespace System.Runtime.CompilerServices { class AsyncMethodBuilderAttribute :
Diagnostic(ErrorCode.ERR_BadAsyncMethodBuilderTaskProperty, "{ await Task.Delay(5); throw null; }").WithArguments("MyTaskBuilder", "MyTask", "int").WithLocation(7, 29)
);
}

[Fact, WorkItem(18257, "https://github.com/dotnet/roslyn/issues/18257")]
public void PatternTempsAreLongLived()
{
var source = @"using System;

public class Foo {}

public class C {
public static void Main(string[] args)
{
var c = new C();
c.M(new Foo());
c.M(new object());
}
public async void M(object o) {
switch (o)
{
case Foo _:
Console.Write(0);
break;
default:
Console.Write(1);
break;
}
}
}";
var expectedOutput = @"01";
var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe);
base.CompileAndVerify(compilation, expectedOutput: expectedOutput);
}

[Fact, WorkItem(18257, "https://github.com/dotnet/roslyn/issues/18257")]
public void PatternTempsSpill()
{
// This test exercises the spilling machinery of async for pattern-matching temps
var source = @"using System;
using System.Threading.Tasks;

public class C {
public class Foo
{
public int Value;
}
public static void Main(string[] args)
{
var c = new C();
c.M(new Foo() { Value = 1 });
c.M(new Foo() { Value = 2 });
c.M(new Foo() { Value = 3 });
c.M(new object());
}
public void M(object o)
{
MAsync(o).Wait();
}
public async Task MAsync(object o) {
switch (o)
{
case Foo foo when await Copy(foo.Value) == 1:
Console.Write($""{foo.Value}=1 "");
break;
case Foo foo when await Copy(foo.Value) == 2:
Console.Write($""{foo.Value}=2 "");
break;
case Foo foo:
Console.Write($""{foo.Value} "");
break;
default:
Console.Write(""X "");
break;
}
}
public async Task<int> Copy(int i)
{
await Task.Delay(1);
return i;
}
}";
var expectedOutput = @"1=1 2=2 3 X";
var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe);
base.CompileAndVerify(compilation, expectedOutput: expectedOutput);
}
}
}
31 changes: 17 additions & 14 deletions src/Compilers/CSharp/Test/Emit/PDB/PDBTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2705,9 +2705,9 @@ class Student : Person { public double GPA; }
<customDebugInfo>
<forward declaringType=""Program"" methodName=""Main"" parameterNames=""args"" />
<encLocalSlotMap>
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""11"" />
<slot kind=""35"" offset=""46"" />
<slot kind=""35"" offset=""237"" />
<slot kind=""temp"" />
<slot kind=""0"" offset=""59"" />
<slot kind=""0"" offset=""163"" />
Expand Down Expand Up @@ -2798,9 +2798,9 @@ class Student : Person { public double GPA; }
<encLocalSlotMap>
<slot kind=""30"" offset=""0"" />
<slot kind=""30"" offset=""383"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""11"" />
<slot kind=""35"" offset=""46"" />
<slot kind=""35"" offset=""249"" />
<slot kind=""temp"" />
<slot kind=""1"" offset=""11"" />
<slot kind=""21"" offset=""0"" />
Expand Down Expand Up @@ -5836,13 +5836,16 @@ static void M(object o)
<namespace usingCount=""0"" />
</using>
<encLocalSlotMap>
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""11"" />
<slot kind=""35"" offset=""46"" />
<slot kind=""temp"" />
<slot kind=""1"" offset=""11"" />
<slot kind=""temp"" />
<slot kind=""temp"" />
<slot kind=""35"" offset=""378"" />
<slot kind=""35"" offset=""413"" />
<slot kind=""1"" offset=""378"" />
<slot kind=""35"" offset=""511"" />
<slot kind=""1"" offset=""511"" />
</encLocalSlotMap>
</customDebugInfo>
Expand All @@ -5860,12 +5863,12 @@ static void M(object o)
<entry offset=""0x7a"" startLine=""18"" startColumn=""17"" endLine=""18"" endColumn=""23"" />
<entry offset=""0x7c"" startLine=""20"" startColumn=""9"" endLine=""20"" endColumn=""19"" />
<entry offset=""0x81"" hidden=""true"" />
<entry offset=""0xb5"" startLine=""23"" startColumn=""17"" endLine=""23"" endColumn=""23"" />
<entry offset=""0xb7"" startLine=""25"" startColumn=""17"" endLine=""25"" endColumn=""23"" />
<entry offset=""0xb9"" startLine=""27"" startColumn=""9"" endLine=""27"" endColumn=""19"" />
<entry offset=""0xbe"" hidden=""true"" />
<entry offset=""0xc8"" startLine=""30"" startColumn=""17"" endLine=""30"" endColumn=""23"" />
<entry offset=""0xca"" startLine=""32"" startColumn=""5"" endLine=""32"" endColumn=""6"" />
<entry offset=""0xba"" startLine=""23"" startColumn=""17"" endLine=""23"" endColumn=""23"" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in sequence points suggest there are changes in IL. Seems like no other test covers the difference in IL. It would be good to have one.

<entry offset=""0xbc"" startLine=""25"" startColumn=""17"" endLine=""25"" endColumn=""23"" />
<entry offset=""0xbe"" startLine=""27"" startColumn=""9"" endLine=""27"" endColumn=""19"" />
<entry offset=""0xc3"" hidden=""true"" />
<entry offset=""0xcf"" startLine=""30"" startColumn=""17"" endLine=""30"" endColumn=""23"" />
<entry offset=""0xd1"" startLine=""32"" startColumn=""5"" endLine=""32"" endColumn=""6"" />
</sequencePoints>
</method>
</methods>
Expand Down
14 changes: 8 additions & 6 deletions src/Compilers/Core/Portable/SynthesizedLocalKind.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ internal enum SynthesizedLocalKind
/// </summary>
FrameCache = -5,

/// <summary>
/// Temp created for pattern matching by type.
/// </summary>
PatternMatchingTemp = -4,

/// <summary>
/// Temp variable created by the optimizer.
/// </summary>
Expand All @@ -53,7 +48,9 @@ internal enum SynthesizedLocalKind
EmitterTemp = -1,

/// <summary>
/// The variable is not synthesized (C#, VB).
/// The variable is not synthesized (C#, VB). Note that SynthesizedLocalKind values
/// greater than or equal to this are considered long-lived;
/// see <see cref="SynthesizedLocalKindExtensions.IsLongLived"/>.
/// </summary>
UserDefined = 0,

Expand Down Expand Up @@ -206,6 +203,11 @@ internal enum SynthesizedLocalKind
/// </summary>
InstrumentationPayload = 34,

/// <summary>
/// Temp created for pattern matching by type.
/// </summary>
PatternMatchingTemp = 35,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove "Temp" from the name. "Temp" suggests it's temporary, not long lived.
It would also be useful to mention in the comment what value is specifically stored in this variable. Is it the type object we are switching on?


/// <summary>
/// All values have to be less than or equal to <see cref="MaxValidValueForLocalVariableSerializedToDebugInformation"/>
/// (<see cref="EditAndContinueMethodDebugInformation"/>)
Expand Down
126 changes: 54 additions & 72 deletions src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,15 +607,13 @@ .maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""C C.<M>d__1.<>4__this""
IL_0006: ret
Expand All @@ -627,15 +625,13 @@ .maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""object C.<M>d__1.o""
IL_0006: ret
Expand All @@ -647,38 +643,33 @@ .maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""string C.<M>d__1.<a>5__1""
IL_0001: ldfld ""string C.<M>d__1.<a>5__4""
IL_0006: ret
}
");
}");
VerifyLocal(testData, typeName, locals[3], "<>m3", "s", expectedILOpt:
@"{
// Code size 7 (0x7)
.maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""string C.<M>d__1.<s>5__2""
IL_0001: ldfld ""string C.<M>d__1.<s>5__5""
IL_0006: ret
}");
locals.Free();
Expand All @@ -699,15 +690,13 @@ .maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""C C.<M>d__1.<>4__this""
IL_0006: ret
Expand All @@ -719,15 +708,13 @@ .maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""object C.<M>d__1.o""
IL_0006: ret
Expand All @@ -739,38 +726,33 @@ .maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""string C.<M>d__1.<a>5__1""
IL_0001: ldfld ""string C.<M>d__1.<a>5__4""
IL_0006: ret
}
");
}");
VerifyLocal(testData, typeName, locals[3], "<>m3", "s", expectedILOpt:
@"{
// Code size 7 (0x7)
.maxstack 1
.locals init (int V_0,
object V_1,
object V_2,
string V_3,
int V_4,
object V_5,
object V_6,
int? V_7,
object V_3,
string V_4,
int? V_5,
System.Runtime.CompilerServices.TaskAwaiter<object> V_6,
C.<M>d__1 V_7,
System.Runtime.CompilerServices.TaskAwaiter<object> V_8,
C.<M>d__1 V_9,
System.Runtime.CompilerServices.TaskAwaiter<object> V_10,
System.Exception V_11)
System.Exception V_9)
IL_0000: ldarg.0
IL_0001: ldfld ""int C.<M>d__1.<s>5__3""
IL_0001: ldfld ""int C.<M>d__1.<s>5__6""
IL_0006: ret
}");
locals.Free();
Expand Down