]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: cleanup closure.go
authorMatthew Dempsky <mdempsky@google.com>
Thu, 8 Mar 2018 14:25:04 +0000 (06:25 -0800)
committerMatthew Dempsky <mdempsky@google.com>
Wed, 14 Mar 2018 23:54:39 +0000 (23:54 +0000)
The main thing is we now eagerly create the ODCLFUNC node for
closures, immediately cross-link them, and assign fields (e.g., Nbody,
Dcl, Parents, Marks) directly on the ODCLFUNC (previously they were
assigned on the OCLOSURE and later moved to the ODCLFUNC).

This allows us to set Curfn to the ODCLFUNC instead of the OCLOSURE,
which makes things more consistent with normal function declarations.
(Notably, this means Cvars now hang off the ODCLFUNC instead of the
OCLOSURE.)

Assignment of xfunc symbol names also now happens before typechecking
their body, which means debugging output now provides a more helpful
name than "<S>".

In golang.org/cl/66810, we changed "x := y" statements to avoid
creating false closure variables for x, but we still create them for
struct literals like "s{f: x}". Update comment in capturevars
accordingly.

More opportunity for cleanups still, but this makes some substantial
progress, IMO.

Passes toolstash-check.

Change-Id: I65a4efc91886e3dcd1000561348af88297775cd7
Reviewed-on: https://go-review.googlesource.com/100197
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/cmd/compile/internal/gc/closure.go
src/cmd/compile/internal/gc/esc.go
src/cmd/compile/internal/gc/inl.go
src/cmd/compile/internal/gc/order.go
src/cmd/compile/internal/gc/sizeof_test.go
src/cmd/compile/internal/gc/syntax.go

index 6f9025609034248826cda527c5c6d96139e34140..fd728fd7bfe6ff2c321136149db81e08c3751183 100644 (file)
@@ -11,61 +11,39 @@ import (
 )
 
 func (p *noder) funcLit(expr *syntax.FuncLit) *Node {
+       xtype := p.typeExpr(expr.Type)
        ntype := p.typeExpr(expr.Type)
 
-       n := p.nod(expr, OCLOSURE, nil, nil)
-       n.Func.SetIsHiddenClosure(Curfn != nil)
-       n.Func.Ntype = ntype
-       n.Func.Outerfunc = Curfn
-
-       old := p.funchdr(n)
-
-       // steal ntype's argument names and
-       // leave a fresh copy in their place.
-       // references to these variables need to
-       // refer to the variables in the external
-       // function declared below; see walkclosure.
-       n.List.Set(ntype.List.Slice())
-       n.Rlist.Set(ntype.Rlist.Slice())
-
-       ntype.List.Set(nil)
-       ntype.Rlist.Set(nil)
-       for _, n1 := range n.List.Slice() {
-               name := n1.Left
-               if name != nil {
-                       name = newname(name.Sym)
-               }
-               a := nod(ODCLFIELD, name, n1.Right)
-               a.SetIsddd(n1.Isddd())
-               if name != nil {
-                       name.SetIsddd(a.Isddd())
-               }
-               ntype.List.Append(a)
-       }
-       for _, n2 := range n.Rlist.Slice() {
-               name := n2.Left
-               if name != nil {
-                       name = newname(name.Sym)
-               }
-               ntype.Rlist.Append(nod(ODCLFIELD, name, n2.Right))
-       }
+       xfunc := p.nod(expr, ODCLFUNC, nil, nil)
+       xfunc.Func.SetIsHiddenClosure(Curfn != nil)
+       xfunc.Func.Nname = newfuncname(nblank.Sym) // filled in by typecheckclosure
+       xfunc.Func.Nname.Name.Param.Ntype = xtype
+       xfunc.Func.Nname.Name.Defn = xfunc
 
-       body := p.stmts(expr.Body.List)
+       clo := p.nod(expr, OCLOSURE, nil, nil)
+       clo.Func.Ntype = ntype
 
-       lineno = p.makeXPos(expr.Body.Rbrace)
-       if len(body) == 0 {
+       xfunc.Func.Closure = clo
+       clo.Func.Closure = xfunc
+
+       oldScope := p.funchdr(xfunc)
+
+       body := p.stmts(expr.Body.List)
+       if body == nil {
                body = []*Node{nod(OEMPTY, nil, nil)}
        }
+       xfunc.Nbody.Set(body)
+
+       lineno = p.makeXPos(expr.Body.Rbrace)
+       xfunc.Func.Endlineno = lineno
 
-       n.Nbody.Set(body)
-       n.Func.Endlineno = lineno
-       p.funcbody(old)
+       p.funcbody(oldScope)
 
        // closure-specific variables are hanging off the
        // ordinary ones in the symbol table; see oldname.
        // unhook them.
        // make the list of pointers for the closure call.
-       for _, v := range n.Func.Cvars.Slice() {
+       for _, v := range xfunc.Func.Cvars.Slice() {
                // Unlink from v1; see comment in syntax.go type Param for these fields.
                v1 := v.Name.Defn
                v1.Name.Param.Innermost = v.Name.Param.Outer
@@ -101,11 +79,13 @@ func (p *noder) funcLit(expr *syntax.FuncLit) *Node {
                v.Name.Param.Outer = oldname(v.Sym)
        }
 
-       return n
+       return clo
 }
 
-func typecheckclosure(func_ *Node, top int) {
-       for _, ln := range func_.Func.Cvars.Slice() {
+func typecheckclosure(clo *Node, top int) {
+       xfunc := clo.Func.Closure
+
+       for _, ln := range xfunc.Func.Cvars.Slice() {
                n := ln.Name.Defn
                if !n.Name.Captured() {
                        n.Name.SetCaptured(true)
@@ -121,129 +101,59 @@ func typecheckclosure(func_ *Node, top int) {
                }
        }
 
-       for _, ln := range func_.Func.Dcl {
-               if ln.Op == ONAME && (ln.Class() == PPARAM || ln.Class() == PPARAMOUT) {
-                       ln.Name.Decldepth = 1
-               }
-       }
+       xfunc.Func.Nname.Sym = closurename(Curfn)
+       xfunc.Func.Nname.Sym.SetExported(true) // disable export
+       declare(xfunc.Func.Nname, PFUNC)
+       xfunc = typecheck(xfunc, Etop)
 
-       oldfn := Curfn
-       func_.Func.Ntype = typecheck(func_.Func.Ntype, Etype)
-       func_.Type = func_.Func.Ntype.Type
-       func_.Func.Top = top
+       clo.Func.Ntype = typecheck(clo.Func.Ntype, Etype)
+       clo.Type = clo.Func.Ntype.Type
+       clo.Func.Top = top
 
        // Type check the body now, but only if we're inside a function.
        // At top level (in a variable initialization: curfn==nil) we're not
        // ready to type check code yet; we'll check it later, because the
        // underlying closure function we create is added to xtop.
-       if Curfn != nil && func_.Type != nil {
-               Curfn = func_
+       if Curfn != nil && clo.Type != nil {
+               oldfn := Curfn
+               Curfn = xfunc
                olddd := decldepth
                decldepth = 1
-               typecheckslice(func_.Nbody.Slice(), Etop)
+               typecheckslice(xfunc.Nbody.Slice(), Etop)
                decldepth = olddd
                Curfn = oldfn
        }
 
-       // Create top-level function
-       xtop = append(xtop, makeclosure(func_))
+       xtop = append(xtop, xfunc)
 }
 
-// closurename returns name for OCLOSURE n.
-// It is not as simple as it ought to be, because we typecheck nested closures
-// starting from the innermost one. So when we check the inner closure,
-// we don't yet have name for the outer closure. This function uses recursion
-// to generate names all the way up if necessary.
+// globClosgen is like Func.Closgen, but for the global scope.
+var globClosgen int
 
-var closurename_closgen int
+// closurename generates a new unique name for a closure within
+// outerfunc.
+func closurename(outerfunc *Node) *types.Sym {
+       outer := "glob."
+       prefix := "func"
+       gen := &globClosgen
 
-func closurename(n *Node) *types.Sym {
-       if n.Sym != nil {
-               return n.Sym
-       }
-       gen := 0
-       outer := ""
-       prefix := ""
-       switch {
-       case n.Func.Outerfunc == nil:
-               // Global closure.
-               outer = "glob."
-
-               prefix = "func"
-               closurename_closgen++
-               gen = closurename_closgen
-       case n.Func.Outerfunc.Op == ODCLFUNC:
-               // The outermost closure inside of a named function.
-               outer = n.Func.Outerfunc.funcname()
-
-               prefix = "func"
-
-               // Yes, functions can be named _.
-               // Can't use function closgen in such case,
-               // because it would lead to name clashes.
-               if !isblank(n.Func.Outerfunc.Func.Nname) {
-                       n.Func.Outerfunc.Func.Closgen++
-                       gen = n.Func.Outerfunc.Func.Closgen
-               } else {
-                       closurename_closgen++
-                       gen = closurename_closgen
+       if outerfunc != nil {
+               if outerfunc.Func.Closure != nil {
+                       prefix = ""
                }
-       case n.Func.Outerfunc.Op == OCLOSURE:
-               // Nested closure, recurse.
-               outer = closurename(n.Func.Outerfunc).Name
 
-               prefix = ""
-               n.Func.Outerfunc.Func.Closgen++
-               gen = n.Func.Outerfunc.Func.Closgen
-       default:
-               Fatalf("closurename called for %S", n)
-       }
-       n.Sym = lookup(fmt.Sprintf("%s.%s%d", outer, prefix, gen))
-       return n.Sym
-}
-
-func makeclosure(func_ *Node) *Node {
-       // wrap body in external function
-       // that begins by reading closure parameters.
-       xtype := nod(OTFUNC, nil, nil)
-
-       xtype.List.Set(func_.List.Slice())
-       xtype.Rlist.Set(func_.Rlist.Slice())
+               outer = outerfunc.funcname()
 
-       // create the function
-       xfunc := nod(ODCLFUNC, nil, nil)
-       xfunc.Func.SetIsHiddenClosure(Curfn != nil)
-
-       xfunc.Func.Nname = newfuncname(closurename(func_))
-       xfunc.Func.Nname.Sym.SetExported(true) // disable export
-       xfunc.Func.Nname.Name.Param.Ntype = xtype
-       xfunc.Func.Nname.Name.Defn = xfunc
-       declare(xfunc.Func.Nname, PFUNC)
-       xfunc.Func.Endlineno = func_.Func.Endlineno
-       if Ctxt.Flag_dynlink {
-               makefuncsym(xfunc.Func.Nname.Sym)
-       }
-
-       xfunc.Nbody.Set(func_.Nbody.Slice())
-       xfunc.Func.Dcl = append(func_.Func.Dcl, xfunc.Func.Dcl...)
-       xfunc.Func.Parents = func_.Func.Parents
-       xfunc.Func.Marks = func_.Func.Marks
-       func_.Func.Dcl = nil
-       func_.Func.Parents = nil
-       func_.Func.Marks = nil
-       if xfunc.Nbody.Len() == 0 {
-               Fatalf("empty body - won't generate any code")
+               // There may be multiple functions named "_". In those
+               // cases, we can't use their individual Closgens as it
+               // would lead to name clashes.
+               if !isblank(outerfunc.Func.Nname) {
+                       gen = &outerfunc.Func.Closgen
+               }
        }
-       xfunc = typecheck(xfunc, Etop)
-
-       xfunc.Func.Closure = func_
-       func_.Func.Closure = xfunc
 
-       func_.Nbody.Set(nil)
-       func_.List.Set(nil)
-       func_.Rlist.Set(nil)
-
-       return xfunc
+       *gen++
+       return lookup(fmt.Sprintf("%s.%s%d", outer, prefix, *gen))
 }
 
 // capturevarscomplete is set to true when the capturevars phase is done.
@@ -258,20 +168,20 @@ func capturevars(xfunc *Node) {
        lno := lineno
        lineno = xfunc.Pos
 
-       func_ := xfunc.Func.Closure
-       func_.Func.Enter.Set(nil)
-       for _, v := range func_.Func.Cvars.Slice() {
+       clo := xfunc.Func.Closure
+       cvars := xfunc.Func.Cvars.Slice()
+       out := cvars[:0]
+       for _, v := range cvars {
                if v.Type == nil {
-                       // if v->type is nil, it means v looked like it was
-                       // going to be used in the closure but wasn't.
-                       // this happens because when parsing a, b, c := f()
-                       // the a, b, c gets parsed as references to older
-                       // a, b, c before the parser figures out this is a
-                       // declaration.
-                       v.Op = OXXX
-
+                       // If v.Type is nil, it means v looked like it
+                       // was going to be used in the closure, but
+                       // isn't. This happens in struct literals like
+                       // s{f: x} where we can't distinguish whether
+                       // f is a field identifier or expression until
+                       // resolving s.
                        continue
                }
+               out = append(out, v)
 
                // type check the & of closed variables outside the closure,
                // so that the outer frame also grabs them and knows they escape.
@@ -301,9 +211,10 @@ func capturevars(xfunc *Node) {
                }
 
                outer = typecheck(outer, Erv)
-               func_.Func.Enter.Append(outer)
+               clo.Func.Enter.Append(outer)
        }
 
+       xfunc.Func.Cvars.Set(out)
        lineno = lno
 }
 
@@ -312,9 +223,9 @@ func capturevars(xfunc *Node) {
 func transformclosure(xfunc *Node) {
        lno := lineno
        lineno = xfunc.Pos
-       func_ := xfunc.Func.Closure
+       clo := xfunc.Func.Closure
 
-       if func_.Func.Top&Ecall != 0 {
+       if clo.Func.Top&Ecall != 0 {
                // If the closure is directly called, we transform it to a plain function call
                // with variables passed as args. This avoids allocation of a closure object.
                // Here we do only a part of the transformation. Walk of OCALLFUNC(OCLOSURE)
@@ -336,33 +247,27 @@ func transformclosure(xfunc *Node) {
                // We are going to insert captured variables before input args.
                var params []*types.Field
                var decls []*Node
-               for _, v := range func_.Func.Cvars.Slice() {
-                       if v.Op == OXXX {
-                               continue
-                       }
-                       fld := types.NewField()
-                       fld.Funarg = types.FunargParams
-                       if v.Name.Byval() {
-                               // If v is captured by value, we merely downgrade it to PPARAM.
-                               v.SetClass(PPARAM)
-                               fld.Nname = asTypesNode(v)
-                       } else {
+               for _, v := range xfunc.Func.Cvars.Slice() {
+                       if !v.Name.Byval() {
                                // If v of type T is captured by reference,
                                // we introduce function param &v *T
                                // and v remains PAUTOHEAP with &v heapaddr
                                // (accesses will implicitly deref &v).
                                addr := newname(lookup("&" + v.Sym.Name))
                                addr.Type = types.NewPtr(v.Type)
-                               addr.SetClass(PPARAM)
                                v.Name.Param.Heapaddr = addr
-                               fld.Nname = asTypesNode(addr)
+                               v = addr
                        }
 
-                       fld.Type = asNode(fld.Nname).Type
-                       fld.Sym = asNode(fld.Nname).Sym
+                       v.SetClass(PPARAM)
+                       decls = append(decls, v)
 
+                       fld := types.NewField()
+                       fld.Funarg = types.FunargParams
+                       fld.Nname = asTypesNode(v)
+                       fld.Type = v.Type
+                       fld.Sym = v.Sym
                        params = append(params, fld)
-                       decls = append(decls, asNode(fld.Nname))
                }
 
                if len(params) > 0 {
@@ -377,11 +282,7 @@ func transformclosure(xfunc *Node) {
                // The closure is not called, so it is going to stay as closure.
                var body []*Node
                offset := int64(Widthptr)
-               for _, v := range func_.Func.Cvars.Slice() {
-                       if v.Op == OXXX {
-                               continue
-                       }
-
+               for _, v := range xfunc.Func.Cvars.Slice() {
                        // cv refers to the field inside of closure OSTRUCTLIT.
                        cv := nod(OCLOSUREVAR, nil, nil)
 
@@ -425,42 +326,40 @@ func transformclosure(xfunc *Node) {
        lineno = lno
 }
 
-// hasemptycvars returns true iff closure func_ has an
-// empty list of captured vars. OXXX nodes don't count.
-func hasemptycvars(func_ *Node) bool {
-       for _, v := range func_.Func.Cvars.Slice() {
-               if v.Op == OXXX {
-                       continue
-               }
-               return false
-       }
-       return true
+// hasemptycvars returns true iff closure clo has an
+// empty list of captured vars.
+func hasemptycvars(clo *Node) bool {
+       xfunc := clo.Func.Closure
+       return xfunc.Func.Cvars.Len() == 0
 }
 
 // closuredebugruntimecheck applies boilerplate checks for debug flags
 // and compiling runtime
-func closuredebugruntimecheck(r *Node) {
+func closuredebugruntimecheck(clo *Node) {
        if Debug_closure > 0 {
-               if r.Esc == EscHeap {
-                       Warnl(r.Pos, "heap closure, captured vars = %v", r.Func.Cvars)
+               xfunc := clo.Func.Closure
+               if clo.Esc == EscHeap {
+                       Warnl(clo.Pos, "heap closure, captured vars = %v", xfunc.Func.Cvars)
                } else {
-                       Warnl(r.Pos, "stack closure, captured vars = %v", r.Func.Cvars)
+                       Warnl(clo.Pos, "stack closure, captured vars = %v", xfunc.Func.Cvars)
                }
        }
-       if compiling_runtime && r.Esc == EscHeap {
-               yyerrorl(r.Pos, "heap-allocated closure, not allowed in runtime.")
+       if compiling_runtime && clo.Esc == EscHeap {
+               yyerrorl(clo.Pos, "heap-allocated closure, not allowed in runtime.")
        }
 }
 
-func walkclosure(func_ *Node, init *Nodes) *Node {
+func walkclosure(clo *Node, init *Nodes) *Node {
+       xfunc := clo.Func.Closure
+
        // If no closure vars, don't bother wrapping.
-       if hasemptycvars(func_) {
+       if hasemptycvars(clo) {
                if Debug_closure > 0 {
-                       Warnl(func_.Pos, "closure converted to global")
+                       Warnl(clo.Pos, "closure converted to global")
                }
-               return func_.Func.Closure.Func.Nname
+               return xfunc.Func.Nname
        }
-       closuredebugruntimecheck(func_)
+       closuredebugruntimecheck(clo)
 
        // Create closure in the form of a composite literal.
        // supposing the closure captures an int i and a string s
@@ -479,10 +378,7 @@ func walkclosure(func_ *Node, init *Nodes) *Node {
        fields := []*Node{
                namedfield(".F", types.Types[TUINTPTR]),
        }
-       for _, v := range func_.Func.Cvars.Slice() {
-               if v.Op == OXXX {
-                       continue
-               }
+       for _, v := range xfunc.Func.Cvars.Slice() {
                typ := v.Type
                if !v.Name.Byval() {
                        typ = types.NewPtr(typ)
@@ -493,27 +389,27 @@ func walkclosure(func_ *Node, init *Nodes) *Node {
        typ.SetNoalg(true)
 
        clos := nod(OCOMPLIT, nil, nod(OIND, typenod(typ), nil))
-       clos.Esc = func_.Esc
+       clos.Esc = clo.Esc
        clos.Right.SetImplicit(true)
-       clos.List.Set(append([]*Node{nod(OCFUNC, func_.Func.Closure.Func.Nname, nil)}, func_.Func.Enter.Slice()...))
+       clos.List.Set(append([]*Node{nod(OCFUNC, xfunc.Func.Nname, nil)}, clo.Func.Enter.Slice()...))
 
        // Force type conversion from *struct to the func type.
        clos = nod(OCONVNOP, clos, nil)
-       clos.Type = func_.Type
+       clos.Type = clo.Type
 
        clos = typecheck(clos, Erv)
 
        // typecheck will insert a PTRLIT node under CONVNOP,
        // tag it with escape analysis result.
-       clos.Left.Esc = func_.Esc
+       clos.Left.Esc = clo.Esc
 
        // non-escaping temp to use, if any.
        // orderexpr did not compute the type; fill it in now.
-       if x := prealloc[func_]; x != nil {
+       if x := prealloc[clo]; x != nil {
                x.Type = clos.Left.Left.Type
                x.Orig.Type = x.Type
                clos.Left.Right = x
-               delete(prealloc, func_)
+               delete(prealloc, clo)
        }
 
        return walkexpr(clos, init)
@@ -619,12 +515,11 @@ func makepartialcall(fn *Node, t0 *types.Type, meth *types.Sym) *Node {
        // Declare and initialize variable holding receiver.
 
        xfunc.Func.SetNeedctxt(true)
+
        cv := nod(OCLOSUREVAR, nil, nil)
-       cv.Xoffset = int64(Widthptr)
        cv.Type = rcvrtype
-       if int(cv.Type.Align) > Widthptr {
-               cv.Xoffset = int64(cv.Type.Align)
-       }
+       cv.Xoffset = Rnd(int64(Widthptr), int64(cv.Type.Align))
+
        ptr := newname(lookup("rcvr"))
        ptr.SetClass(PAUTO)
        ptr.Name.SetUsed(true)
index 48945e2868f4003a33a81e03b741ab66ba050953..c5021f4a48eb6c17621685fe8007a3f9f8f9ad62 100644 (file)
@@ -948,7 +948,7 @@ func (e *EscState) esc(n *Node, parent *Node) {
 
        case OCLOSURE:
                // Link addresses of captured variables to closure.
-               for _, v := range n.Func.Cvars.Slice() {
+               for _, v := range n.Func.Closure.Func.Cvars.Slice() {
                        if v.Op == OXXX { // unnamed out argument; see dcl.go:/^funcargs
                                continue
                        }
index 32981c0f2ccf13aa326c18e97aadb357090586aa..d622307c63167551f4240ce7116e6f62ef5f3157 100644 (file)
@@ -816,7 +816,7 @@ func mkinlcall1(n, fn *Node) *Node {
 
                // handle captured variables when inlining closures
                if c := fn.Name.Defn.Func.Closure; c != nil {
-                       for _, v := range c.Func.Cvars.Slice() {
+                       for _, v := range c.Func.Closure.Func.Cvars.Slice() {
                                if v.Op == OXXX {
                                        continue
                                }
index 0e88d0f67c137216d3ac4fae5f0c1276d067396e..d3f76953ccafb77477a3aa85f8d13b37f51a3e15 100644 (file)
@@ -1124,7 +1124,7 @@ func (o *Order) expr(n, lhs *Node) *Node {
                }
 
        case OCLOSURE:
-               if n.Noescape() && n.Func.Cvars.Len() > 0 {
+               if n.Noescape() && n.Func.Closure.Func.Cvars.Len() > 0 {
                        prealloc[n] = o.newTemp(types.Types[TUINT8], false) // walk will fill in correct type
                }
 
index 358814c5ce3a14eac14e3a6e881af83c693f31d8..c7104030bdc169af15b5940746a583e21ed55fe5 100644 (file)
@@ -22,7 +22,7 @@ func TestSizeof(t *testing.T) {
                _32bit uintptr     // size on 32bit platforms
                _64bit uintptr     // size on 64bit platforms
        }{
-               {Func{}, 128, 232},
+               {Func{}, 124, 224},
                {Name{}, 32, 56},
                {Param{}, 24, 48},
                {Node{}, 76, 128},
index 182f93da14da87d91b9f8491b701afac11bbfe66..28befbeb857cbe94b33c2d9e1e060162046b4842 100644 (file)
@@ -471,8 +471,11 @@ type Func struct {
        // Marks records scope boundary changes.
        Marks []Mark
 
-       Closgen    int
-       Outerfunc  *Node // outer function (for closure)
+       // Closgen tracks how many closures have been generated within
+       // this function. Used by closurename for creating unique
+       // function names.
+       Closgen int
+
        FieldTrack map[*types.Sym]struct{}
        DebugInfo  *ssa.FuncDebug
        Ntype      *Node // signature