cmd/compile: cleanup closure.go

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

@@ -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
+	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
 
-	old := p.funchdr(n)
+	clo := p.nod(expr, OCLOSURE, nil, nil)
+	clo.Func.Ntype = ntype
 
-	// 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())
+	xfunc.Func.Closure = clo
+	clo.Func.Closure = xfunc
 
-	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))
-	}
+	oldScope := p.funchdr(xfunc)
 
 	body := p.stmts(expr.Body.List)
-
-	lineno = p.makeXPos(expr.Body.Rbrace)
-	if len(body) == 0 {
+	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.
-
-var closurename_closgen int
-
-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
-		}
-	case n.Func.Outerfunc.Op == OCLOSURE:
-		// Nested closure, recurse.
-		outer = closurename(n.Func.Outerfunc).Name
+// globClosgen is like Func.Closgen, but for the global scope.
+var globClosgen int
+
+// closurename generates a new unique name for a closure within
+// outerfunc.
+func closurename(outerfunc *Node) *types.Sym {
+	outer := "glob."
+	prefix := "func"
+	gen := &globClosgen
 
+	if outerfunc != nil {
+		if outerfunc.Func.Closure != nil {
 			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)
+		// 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.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")
 	}
-	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)

src/cmd/compile/internal/gc/esc.go

@@ -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
 			}

src/cmd/compile/internal/gc/inl.go

@@ -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
 				}

src/cmd/compile/internal/gc/order.go

@@ -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
 		}
 

src/cmd/compile/internal/gc/sizeof_test.go

@@ -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},

src/cmd/compile/internal/gc/syntax.go

@@ -471,8 +471,11 @@ type Func struct {
 	// Marks records scope boundary changes.
 	Marks []Mark
 
+	// Closgen tracks how many closures have been generated within
+	// this function. Used by closurename for creating unique
+	// function names.
 	Closgen int
-	Outerfunc  *Node // outer function (for closure)
+
 	FieldTrack map[*types.Sym]struct{}
 	DebugInfo  *ssa.FuncDebug
 	Ntype      *Node // signature