Skip to content

Go: Fix missing flow through receiver for function variable #13767

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

Merged
merged 4 commits into from
Jul 19, 2023

Conversation

owen-mc
Copy link
Contributor

@owen-mc owen-mc commented Jul 19, 2023

When assigning a method to a function variable (f := a.b) and calling it using the variable (f(arg1, arg2)), we were losing any possible flow to the receiver of the method.

@owen-mc owen-mc requested a review from a team as a code owner July 19, 2023 06:27
@github-actions github-actions bot added the Go label Jul 19, 2023
When a function is assigned to a variable and called through that
variable then we previously didn't realise it was a function. With
this change we try use local flow to determine if the function being
called is a method.
@owen-mc owen-mc force-pushed the go/missing-flow-through-receiver branch from 70004aa to de8794e Compare July 19, 2023 09:26
@smowton
Copy link
Contributor

smowton commented Jul 19, 2023

Don't mind the change; either defining method-call-node to be a thing with calling syntax x.M(...), or defining it to be a call where we know a receiver value (i.e. CallNode.getReceiver has a value), both seem like reasonable choices. If we take the latter we should document that some calls that look syntactically like function calls can be MethodCallNodes.

We shouldn't modify Method.getTarget, because this explicitly refers to the method that is syntactically addressed at the callsite, and in the case of a function pointer there is no such method.

owen-mc added 2 commits July 19, 2023 11:17
This is not required, because getReceiver is still defined on CallNode,
but is done for consistency.
smowton
smowton previously approved these changes Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants