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

Conversation

333fred
Copy link
Member

@333fred 333fred commented Nov 20, 2020

Fixes #49315.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
333fred Fred Silberberg
Fixes dotnet#49315.
@333fred 333fred requested a review from a team as a code owner November 20, 2020 19:42
@333fred 333fred added the Feature - Function Pointers Adding Function Pointers label Nov 20, 2020
@333fred
Copy link
Member Author

333fred commented Nov 20, 2020

@dotnet/roslyn-compiler for review.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
333fred Fred Silberberg
@@ -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:``

@333fred
Copy link
Member Author

333fred commented Nov 21, 2020

Note to self: change the title to "Handle passing a ref returned from a function pointer invocation from a ref returning method" before merging.

@@ -10677,6 +10677,178 @@ .maxstack 4
");
}

[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.

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 2), with tests suggestion.

@333fred
Copy link
Member Author

333fred commented Nov 30, 2020

@dotnet/roslyn-compiler for a second review. I'll work on adding Aleksey's test suggestions, but it would be good to address any other feedback at the same time.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
333fred Fred Silberberg
@333fred
Copy link
Member Author

333fred commented Dec 1, 2020

@dotnet/roslyn-compiler for another review please.

@333fred
Copy link
Member Author

333fred commented Dec 2, 2020

@dotnet/roslyn-compiler for a second review please.

@333fred 333fred merged commit 4faa3b3 into dotnet:master Dec 7, 2020
@333fred 333fred deleted the func-ptr-ref branch December 7, 2020 17:50
@ghost ghost added this to the Next milestone Dec 7, 2020
@dibarbet dibarbet modified the milestones: Next, 16.9.P3 Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't use function pointer to return by reference
6 participants