-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
x/tools/go/ssa: method calls sometimes have different count of arguments because of closure #37253
Comments
/cc @ianthehat |
cc @findleyr If I understand this comment, the problem is that the arguments first function call, |
Thank you for your answer. I also confirmed that this behavior is correct and the receiver argument is in the However, for more simplification for users of the SSA interface, could the first argument of closure functions be the receiver and could the difference of arguments between methods and its closure functions be removed? For now, users of the SSA interface should concern about the difference of arguments even the functions object is the same. This difference causes potential The closure functions of methods are simply the curried functions with the partial application of only the first argument. So I think it is not problematic to unify their arguments. Luckily, the SSA interface is noted that This change also solves the problem of uncertainty of the receiver argument because we can't choose the actual receiver from |
I'm leaning against making this change, especially because I don't want to break anyone, but I'll let @findleyr be the final decider. |
I'm also leaning against this change. Yes, go/ssa is labeled as experimental, but it is being used and such a breaking changes carry a high cost/risk. I'm also not sure that this change would be philosophically correct: I don't think it makes sense for the receiver value to be both a bound FreeVar and an Arg. It seems to me that this is working as intended. This is consistent with the description of a bound closure as a synthetic wrapper, here: @Matts966 could you expand on the problems this is causing? You say it can cause a segmentation fault: how specifically does that arise? |
@findleyr Potential index out of range errors are like this. type argumentsFact []ssa.Value
// Export facts about function arguments here.
var fact argumentsFact = callOfF.Args
pass.ExportObjectFact(f, &fact)
...
// Import facts about function arguments in other place.
pass.ImportObjectFact(f.Object(), &fact)
for i, arg := range fact {
anotherCallOfF[i] // access arguments in the same index as in `callOfF`
// This can cause index out of range errors because the count of arguments
// can be different if the anotherCallOfF's function is method closure, whose
// receiver is not in arguments but in free variables.
} In order to handle these cases correctly, we, the users of ssa interface, for instance, writers of some linters, should always check the count of arguments and fetch the actual receivers from free variables, which is sometimes difficult to fetch because they are in the closure declaration, not in calls to closure. As we already regard methods as functions and receivers as the first argument, it's easier if the method closures and their source functions are dealt with in the same way. So I thought that the bound receivers should be in arguments not in free variables at first, but I admit that this can break somethings. |
How are you determining that callOfF and anotherCallOfF are the same? By associating Common().StaticCallee().Object()? I think the lesson here is that this is not sufficient. For example, consider the following package:
When I run your analyzer, all of these invocations look the same:
But if I instead print
I don't think we can change the meaning of Args to include a bound Freevars if that Freevar happens to be a receiver. I understand the API may be difficult to work with, but don't think this is the correct solution to that problem. Does that make sense? |
@findleyr |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
analysis code
target code
What did you expect to see?
What did you see instead?
Created
closure
does not have arguments though itsobject
is the same as(X).method
. When developing analysis tools, the consistency of the count of arguments is important for programmers so I thinkm
is better with a receiver argument.Is this behavior intended?
The text was updated successfully, but these errors were encountered: