qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-23 08:50:03 -07:00
Code Releases Activity

cmd/compile: fix lexical block of captured variables

Browse Source 
Variables captured by a closure were always assigned to the root scope
in their declaration function. Using decl.Name.Defn.Pos will result in
the correct scope for both the declaration function and the capturing
function.

Fixes #21515

Change-Id: I3960aface3c4fc97e15b36191a74a7bed5b5ebc1
Reviewed-on: https://go-review.googlesource.com/56830
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This commit is contained in:
Alessandro Arzilli 2017-08-18 10:22:19 +02:00 committed by Matthew Dempsky
parent a72e26f246
commit e1cf2be7a8
2 changed files with 31 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/cmd/compile/internal/gc/pgen.go
View File

@ -348,15 +348,25 @@ func debuginfo(fnsym *obj.LSym, curfn interface{}) []dwarf.Scope {
var varScopes []ScopeID
for _, decl := range decls {
var scope ScopeID
if !decl.Name.Captured() && !decl.Name.Byval() {
// n.Pos of captured variables is their first
// use in the closure but they should always
// be assigned to scope 0 instead.
// TODO(mdempsky): Verify this.
scope = findScope(fn.Func.Marks, decl.Pos)
pos := decl.Pos
if decl.Name.Defn != nil && (decl.Name.Captured() || decl.Name.Byval()) {
// It's not clear which position is correct for captured variables here:
// * decl.Pos is the wrong position for captured variables, in the inner
// function, but it is the right position in the outer function.
// * decl.Name.Defn is nil for captured variables that were arguments
// on the outer function, however the decl.Pos for those seems to be
// correct.
// * decl.Name.Defn is the "wrong" thing for variables declared in the
// header of a type switch, it's their position in the header, rather
// than the position of the case statement. In principle this is the
// right thing, but here we prefer the latter because it makes each
// instance of the header variable local to the lexical block of its
// case statement.
// This code is probably wrong for type switch variables that are also
// captured.
pos = decl.Name.Defn.Pos
}
varScopes = append(varScopes, scope)
varScopes = append(varScopes, findScope(fn.Func.Marks, pos))
}
return assembleScopes(fnsym, fn, dwarfVars, varScopes)
}

13
src/cmd/compile/internal/gc/scope_test.go
View File

@ -173,6 +173,18 @@ var testfile = []testline{
{line: " fi(p)", scopes: []int{1}},
{line: " }"},
{line: "}"},
{line: "func TestCaptureVar(flag bool) func() int {"},
{line: " a := 1", vars: []string{"arg flag bool", "arg ~r1 func() int", "var a int"}},
{line: " if flag {"},
{line: " b := 2", scopes: []int{1}, vars: []string{"var b int", "var f func() int"}},
{line: " f := func() int {", scopes: []int{1, 0}},
{line: " return b + 1"},
{line: " }"},
{line: " return f", scopes: []int{1}},
{line: " }"},
{line: " f1(a)"},
{line: " return nil"},
{line: "}"},
{line: "func main() {"},
{line: " TestNestedFor()"},
{line: " TestOas2()"},
@ -184,6 +196,7 @@ var testfile = []testline{
{line: " TestDiscontiguousRanges()"},
{line: " TestClosureScope()"},
{line: " TestEscape()"},
{line: " TestCaptureVar(true)"},
{line: "}"},
}