correctly detect indirect uses of reflection #558
Conversation
There was a problem hiding this comment.
Please add high-level godocs to each one of these funcs, and especially to potentialReflectParam and its fields - I tried reviewing this code, but it's hard to try to reverse engineer what the code is doing. I understand you used descriptive names, but that often isn't enough to be able to read and understand the code easily.
| pkg := obj.Pkg() | ||
| if pkg == nil { | ||
| return "" | ||
| } |
There was a problem hiding this comment.
If the object is declared in the universe scope (buitin types).
main.go
Outdated
| } | ||
|
|
||
| inspectAssign := func(node ast.Node) { | ||
| spec, ok := node.(*ast.AssignStmt) |
There was a problem hiding this comment.
note that this will miss quite a few "related" edge cases like T{foo: bar} or foo = []Bar{bar}. I think this amount of code is still bearable, but any more code complexity than what you have here and I think we should just go for go/ssa instead.
There was a problem hiding this comment.
You are right I will also implement composite literals
There was a problem hiding this comment.
Maybe I wasn't clear :) I did not expect you to implement more edge cases, rather that we should try to keep the logic relatively short and simple, because attempting to cover all edge cases with go/ast rather than go/ssa is likely the wrong route. It's a lot of fragile code that won't do as well as SSA.
You've gone ahead and done it, but I really don't want any more control flow analysis based on go/ast - to my taste, the current amount is already more than what I can comfortably maintain.
|
@mvdan added some docs and support for composite literals |
|
Before I do another round of review - did you benchmark this? I generally assume that bug fixes come before performance, but this go/ast analysis is adding two more |
|
Another reason for (slight) worry is that you seem to be tracking most forms of assignments via either |
lu4p
force-pushed
the
master
branch
2 times, most recently
from
06c4ea4
to
55db109
Compare
(see commit message)