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.

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

Handle passing a function pointer ref through a ref returning method #49526

Merged
merged 3 commits into from
Dec 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2292,6 +2292,29 @@ internal static bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint
diagnostics,
isRefEscape: true);

case BoundKind.FunctionPointerInvocation:
Copy link
Contributor

Choose a reason for hiding this comment

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

case BoundKind.FunctionPointerInvocation: [](start = 16, length = 41)

Minor: I would put this case next to ```case BoundKind.Call:``

var functionPointerInvocation = (BoundFunctionPointerInvocation)expr;

FunctionPointerMethodSymbol signature = functionPointerInvocation.FunctionPointer.Signature;
if (signature.RefKind == RefKind.None)
{
break;
}

return CheckInvocationEscape(
functionPointerInvocation.Syntax,
signature,
functionPointerInvocation.InvokedExpression,
signature.Parameters,
functionPointerInvocation.Arguments,
functionPointerInvocation.ArgumentRefKindsOpt,
argsToParamsOpt: default,
checkingReceiver,
escapeFrom,
escapeTo,
diagnostics,
isRefEscape: true);

case BoundKind.PropertyAccess:
var propertyAccess = (BoundPropertyAccess)expr;
var propertySymbol = propertyAccess.PropertySymbol;
Expand Down
322 changes: 322 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10677,6 +10677,328 @@ .locals init (int V_0, //i1
");
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void ReturnByRefFromRefReturningMethod()
Copy link
Contributor

Choose a reason for hiding this comment

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

ReturnByRefFromRefReturningMethod [](start = 20, length = 33)

It looks like we are only testing ref pass-through for at return position. Were other possible ref pass-through constructs/positions not affected? Consider adding some tests for them too: assignment to ref local, passing to a byref parameter, ref ternary, etc.

{
var verifier = CompileAndVerifyFunctionPointers(@"
unsafe
{
int i = 1;
ref int iRef = ref ReturnPtrByRef(&ReturnByRef, ref i);
iRef = 2;
System.Console.WriteLine(i);

static ref int ReturnPtrByRef(delegate*<ref int, ref int> ptr, ref int i)
=> ref ptr(ref i);

static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, int>, ref int)", @"
{
// Code size 10 (0xa)
.maxstack 2
.locals init (delegate*<ref int, ref int> V_0)
IL_0000: ldarg.0
IL_0001: stloc.0
IL_0002: ldarg.1
IL_0003: ldloc.0
IL_0004: calli ""delegate*<ref int, ref int>""
IL_0009: ret
}
");
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void ReturnByRefFromRefReturningMethod_FunctionPointerDoesNotReturnByRefError()
{
var comp = CreateCompilationWithFunctionPointers(@"
unsafe
{
int i = 1;
ref int iRef = ref ReturnPtrByRef(&ReturnByRef, ref i);

static ref int ReturnPtrByRef(delegate*<ref int, int> ptr, ref int i)
=> ref ptr(ref i);

static int ReturnByRef(ref int i) => i;
}", options: TestOptions.UnsafeReleaseExe);

comp.VerifyDiagnostics(
// (8,16): error CS8156: An expression cannot be used in this context because it may not be passed or returned by reference
// => ref ptr(ref i);
Diagnostic(ErrorCode.ERR_RefReturnLvalueExpected, "ptr(ref i)").WithLocation(8, 16)
);
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void ReturnByRefFromRefReturningMethod_NotSafeToEscape()
{
var comp = CreateCompilationWithSpan(@"
using System;
unsafe
{
ref Span<int> spanRef = ref ReturnPtrByRef(&ReturnByRef);

static ref Span<int> ReturnPtrByRef(delegate*<ref Span<int>, ref Span<int>> ptr)
{
Span<int> span = stackalloc int[1];
return ref ptr(ref span);
}

static ref Span<int> ReturnByRef(ref Span<int> i) => ref i;
}", options: TestOptions.UnsafeReleaseExe);

comp.VerifyDiagnostics(
// (10,20): error CS8347: Cannot use a result of 'delegate*<ref Span<int>, Span<int>>' in this context because it may expose variables referenced by parameter '' outside of their declaration scope
// return ref ptr(ref span);
Diagnostic(ErrorCode.ERR_EscapeCall, "ptr(ref span)").WithArguments("delegate*<ref System.Span<int>, System.Span<int>>", "").WithLocation(10, 20),
// (10,28): error CS8168: Cannot return local 'span' by reference because it is not a ref local
// return ref ptr(ref span);
Diagnostic(ErrorCode.ERR_RefReturnLocal, "span").WithArguments("span").WithLocation(10, 28)
);
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void ReturnByRefFromRefReturningMethod_SafeToEscape()
{
var comp = CreateCompilationWithSpan(@"
using System;
unsafe
{
Span<int> s = stackalloc int[1];
s[0] = 1;
ref Span<int> sRef = ref ReturnPtrByRef(&ReturnByRef, ref s);
sRef[0] = 2;
Console.WriteLine(s[0]);

static ref Span<int> ReturnPtrByRef(delegate*<ref Span<int>, ref Span<int>> ptr, ref Span<int> s)
=> ref ptr(ref s);

static ref Span<int> ReturnByRef(ref Span<int> i) => ref i;
}", options: TestOptions.UnsafeReleaseExe);

var verifier = CompileAndVerify(comp, expectedOutput: "2", verify: Verification.Skipped);

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref System.Span<int>, System.Span<int>>, ref System.Span<int>)", @"
{
// Code size 10 (0xa)
.maxstack 2
.locals init (delegate*<ref System.Span<int>, ref System.Span<int>> V_0)
IL_0000: ldarg.0
IL_0001: stloc.0
IL_0002: ldarg.1
IL_0003: ldloc.0
IL_0004: calli ""delegate*<ref System.Span<int>, ref System.Span<int>>""
IL_0009: ret
}
");
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void ReturnByRefFromRefReturningMethod_RefReadonlyToRefError()
{
var comp = CreateCompilationWithFunctionPointers(@"
unsafe
{
int i = 1;
ref int iRef = ref ReturnPtrByRef(&ReturnByRef, ref i);

static ref int ReturnPtrByRef(delegate*<ref int, ref readonly int> ptr, ref int i)
=> ref ptr(ref i);

static ref readonly int ReturnByRef(ref int i) => ref i;
}", options: TestOptions.UnsafeReleaseExe);

comp.VerifyDiagnostics(
// (8,16): error CS8333: Cannot return method 'delegate*<ref int, int>' by writable reference because it is a readonly variable
// => ref ptr(ref i);
Diagnostic(ErrorCode.ERR_RefReturnReadonlyNotField, "ptr(ref i)").WithArguments("method", "delegate*<ref int, int>").WithLocation(8, 16)
);
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void ReturnByRefFromRefReturningMethod_RefToRefReadonly()
{
var verifier = CompileAndVerifyFunctionPointers(@"
unsafe
{
int i = 1;
ref readonly int iRef = ref ReturnPtrByRef(&ReturnByRef, ref i);
i = 2;
System.Console.WriteLine(iRef);

static ref readonly int ReturnPtrByRef(delegate*<ref int, ref int> ptr, ref int i)
=> ref ptr(ref i);

static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, int>, ref int)", @"
333fred marked this conversation as resolved.
Show resolved Hide resolved
{
// Code size 10 (0xa)
.maxstack 2
.locals init (delegate*<ref int, ref int> V_0)
IL_0000: ldarg.0
IL_0001: stloc.0
IL_0002: ldarg.1
IL_0003: ldloc.0
IL_0004: calli ""delegate*<ref int, ref int>""
IL_0009: ret
}
");
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void RefAssignment()
{
var verifier = CompileAndVerifyFunctionPointers(@"
unsafe
{
int i = 1;
delegate*<ref int, ref int> ptr = &ReturnByRef;
ref readonly int iRef = ref ptr(ref i);
i = 2;
System.Console.WriteLine(iRef);

static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<top-level-statements-entry-point>", @"
{
// Code size 26 (0x1a)
.maxstack 2
.locals init (int V_0, //i
delegate*<ref int, ref int> V_1)
IL_0000: ldc.i4.1
IL_0001: stloc.0
IL_0002: ldftn ""ref int <Program>$.<<Main>$>g__ReturnByRef|0_0(ref int)""
IL_0008: stloc.1
IL_0009: ldloca.s V_0
IL_000b: ldloc.1
IL_000c: calli ""delegate*<ref int, ref int>""
IL_0011: ldc.i4.2
IL_0012: stloc.0
IL_0013: ldind.i4
IL_0014: call ""void System.Console.WriteLine(int)""
IL_0019: ret
}
");
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void RefAssignmentThroughTernary()
{
var verifier = CompileAndVerifyFunctionPointers(@"
unsafe
{
int i = 1;
int i2 = 3;
delegate*<ref int, ref int> ptr = &ReturnByRef;
ref readonly int iRef = ref false ? ref i2 : ref ptr(ref i);
i = 2;
System.Console.WriteLine(iRef);

static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<top-level-statements-entry-point>", @"
{
// Code size 26 (0x1a)
.maxstack 2
.locals init (int V_0, //i
delegate*<ref int, ref int> V_1)
IL_0000: ldc.i4.1
IL_0001: stloc.0
IL_0002: ldftn ""ref int <Program>$.<<Main>$>g__ReturnByRef|0_0(ref int)""
IL_0008: stloc.1
IL_0009: ldloca.s V_0
IL_000b: ldloc.1
IL_000c: calli ""delegate*<ref int, ref int>""
IL_0011: ldc.i4.2
IL_0012: stloc.0
IL_0013: ldind.i4
IL_0014: call ""void System.Console.WriteLine(int)""
IL_0019: ret
}
");
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void RefReturnThroughTernary()
{
var verifier = CompileAndVerifyFunctionPointers(@"
unsafe
{
int i = 1;
int i2 = 3;
ref int iRef = ref ReturnPtrByRef(&ReturnByRef, ref i, ref i2);
iRef = 2;
System.Console.WriteLine(i);

static ref int ReturnPtrByRef(delegate*<ref int, ref int> ptr, ref int i, ref int i2)
=> ref false ? ref i2 : ref ptr(ref i);

static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<Program>$.<<Main>$>g__ReturnPtrByRef|0_0(delegate*<ref int, int>, ref int, ref int)", @"
{
// Code size 10 (0xa)
.maxstack 2
.locals init (delegate*<ref int, ref int> V_0)
IL_0000: ldarg.0
IL_0001: stloc.0
IL_0002: ldarg.1
IL_0003: ldloc.0
IL_0004: calli ""delegate*<ref int, ref int>""
IL_0009: ret
}
");
}

[Fact, WorkItem(49315, "https://github.com/dotnet/roslyn/issues/49315")]
public void PassedAsByRefParameter()
{
var verifier = CompileAndVerifyFunctionPointers(@"
unsafe
{
int i = 1;
delegate*<ref int, ref int> ptr = &ReturnByRef;
ref readonly int iRef = ref ptr(ref ptr(ref i));
i = 2;
System.Console.WriteLine(iRef);

static ref int ReturnByRef(ref int i) => ref i;
}", expectedOutput: "2");

verifier.VerifyIL("<top-level-statements-entry-point>", @"
{
// Code size 34 (0x22)
.maxstack 2
.locals init (int V_0, //i
delegate*<ref int, ref int> V_1,
delegate*<ref int, ref int> V_2)
IL_0000: ldc.i4.1
IL_0001: stloc.0
IL_0002: ldftn ""ref int <Program>$.<<Main>$>g__ReturnByRef|0_0(ref int)""
IL_0008: dup
IL_0009: stloc.1
IL_000a: stloc.2
IL_000b: ldloca.s V_0
IL_000d: ldloc.2
IL_000e: calli ""delegate*<ref int, ref int>""
IL_0013: ldloc.1
IL_0014: calli ""delegate*<ref int, ref int>""
IL_0019: ldc.i4.2
IL_001a: stloc.0
IL_001b: ldind.i4
IL_001c: call ""void System.Console.WriteLine(int)""
IL_0021: ret
}
");
}

private static readonly Guid s_guid = new Guid("97F4DBD4-F6D1-4FAD-91B3-1001F92068E5");
private static readonly BlobContentId s_contentId = new BlobContentId(s_guid, 0x04030201);

Expand Down