]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: fix lexical block of captured variables
authorAlessandro Arzilli <alessandro.arzilli@gmail.com>
Fri, 18 Aug 2017 08:22:19 +0000 (10:22 +0200)
committerMatthew Dempsky <mdempsky@google.com>
Fri, 15 Sep 2017 21:02:59 +0000 (21:02 +0000)
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>
src/cmd/compile/internal/gc/pgen.go
src/cmd/compile/internal/gc/scope_test.go

index 25bb0ae683224479be095cced5d5dcbf16c90a6e..79155f9ad605964bacd998bd96cd88a946f49f3d 100644 (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)
-               }
-               varScopes = append(varScopes, scope)
+               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, findScope(fn.Func.Marks, pos))
        }
        return assembleScopes(fnsym, fn, dwarfVars, varScopes)
 }
index 9113afe279bebe64d8a7d8028e05166c20931e06..5d44b7a4f4f1d861b623bc58f645c900deaf6382 100644 (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: "}"},
 }