go/types: incorrect scope positioning for variables defined by RangeStmt #15686
Labels
Milestone
Comments
|
Too late for Go 1.7. |
|
@mdempsky I agree that this looks non-intrusive, but I believe @alandonovan has code that looks at scope ranges for various analyses. There may be subtle follow-up effects/errors that we don't foresee at the moment. I think it's better to leave for 1.8. |
|
@griesemer Fair enough. I found a reasonably robust and non-intrusive workaround anyway, so I'm fine with deferring this issue to 1.8. |
|
CL https://golang.org/cl/27290 mentions this issue. |
mdempsky
added a commit
to mdempsky/gocode
that referenced
this issue
Apr 19, 2017
Go 1.8 included the fix for golang/go#15686, so no need to workaround it anymore. However, we also now have position information for imported objects, which interferes with scoping rules. go/types doesn't export Object.scopePos, so switch to using go/types.(*Scope).LookupParent instead.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
In go/types/stmt.go:818,819, there's this code to handle declaring for a RangeStmt's Key and Value variables:
This is wrong though, because s refers to the RangeStmt itself, so s.End() is the closing brace. The consequence of this is if you try to use scope.LookupParent or types.Eval with a position inside the RangeStmt's block, it can't find the Key or Value variables.
I think the correct fix is to use s.X.End() instead.
Reviewing other check.declare calls, I think type switches are technically mishandled too. The spec says "the variable is declared at the beginning of the implicit block in each clause". I believe that should be the position of the "case" or "default" keywords, or at least at or immediately after the colon; not at the start of the first statement within the body.
These are preexisting issues, but fixing them should be very non-intrusive, so I'm hopeful it can still make 1.7.
/cc @griesemer @alandonovan
The text was updated successfully, but these errors were encountered: