cmd/compile: clean up, document Node closure fields

Requested during CL 23431.

Change-Id: I513ae42166b3a9fcfe51231ff55c163ab672e7d2
Reviewed-on: https://go-review.googlesource.com/23485Reviewed-by: David Chase <drchase@google.com>

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

@@ -66,8 +66,39 @@ func closurebody(body []*Node) *Node {
 	// unhook them.
 	// make the list of pointers for the closure call.
 	for _, v := range func_.Func.Cvars.Slice() {
-		v.Name.Param.Closure.Name.Param.Closure = v.Name.Param.Outer
-		v.Name.Param.Outerexpr = oldname(v.Sym)
+		// 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
+		
+		// If the closure usage of v is not dense,
+		// we need to make it dense; now that we're out
+		// of the function in which v appeared,
+		// look up v.Sym in the enclosing function
+		// and keep it around for use in the compiled code.
+		//
+		// That is, suppose we just finished parsing the innermost
+		// closure f4 in this code:
+		//
+		//	func f() {
+		//		v := 1
+		//		func() { // f2
+		//			use(v)
+		//			func() { // f3
+		//				func() { // f4
+		//					use(v)
+		//				}()
+		//			}()
+		//		}()
+		//	}
+		//
+		// At this point v.Outer is f2's v; there is no f3's v.
+		// To construct the closure f4 from within f3,
+		// we need to use f3's v and in this case we need to create f3's v.
+		// We are now in the context of f3, so calling oldname(v.Sym)
+		// obtains f3's v, creating it if necessary (as it is in the example).
+		//
+		// capturevars will decide whether to use v directly or &v.
+		v.Name.Param.Outer = oldname(v.Sym)
 	}
 
 	return func_
@@ -75,7 +106,7 @@ func closurebody(body []*Node) *Node {
 
 func typecheckclosure(func_ *Node, top int) {
 	for _, ln := range func_.Func.Cvars.Slice() {
-		n := ln.Name.Param.Closure
+		n := ln.Name.Defn
 		if !n.Name.Captured {
 			n.Name.Captured = true
 			if n.Name.Decldepth == 0 {
@@ -215,8 +246,6 @@ func makeclosure(func_ *Node) *Node {
 // We use value capturing for values <= 128 bytes that are never reassigned
 // after capturing (effectively constant).
 func capturevars(xfunc *Node) {
-	var outer *Node
-
 	lno := lineno
 	lineno = xfunc.Lineno
 
@@ -239,14 +268,14 @@ func capturevars(xfunc *Node) {
 		// so that the outer frame also grabs them and knows they escape.
 		dowidth(v.Type)
 
-		outer = v.Name.Param.Outerexpr
-		v.Name.Param.Outerexpr = nil
+		outer := v.Name.Param.Outer
+		outermost := v.Name.Defn
 
 		// out parameters will be assigned to implicitly upon return.
-		if outer.Class != PPARAMOUT && !v.Name.Param.Closure.Addrtaken && !v.Name.Param.Closure.Assigned && v.Type.Width <= 128 {
+		if outer.Class != PPARAMOUT && !outermost.Addrtaken && !outermost.Assigned && v.Type.Width <= 128 {
 			v.Name.Byval = true
 		} else {
-			v.Name.Param.Closure.Addrtaken = true
+			outermost.Addrtaken = true
 			outer = Nod(OADDR, outer, nil)
 		}
 
@@ -259,7 +288,7 @@ func capturevars(xfunc *Node) {
 			if v.Name.Byval {
 				how = "value"
 			}
-			Warnl(v.Lineno, "%v capturing by %s: %v (addr=%v assign=%v width=%d)", name, how, v.Sym, v.Name.Param.Closure.Addrtaken, v.Name.Param.Closure.Assigned, int32(v.Type.Width))
+			Warnl(v.Lineno, "%v capturing by %s: %v (addr=%v assign=%v width=%d)", name, how, v.Sym, outermost.Addrtaken, outermost.Assigned, int32(v.Type.Width))
 		}
 
 		outer = typecheck(outer, Erv)

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

@@ -385,33 +385,36 @@ func oldname(s *Sym) *Node {
 	}
 
 	if Curfn != nil && n.Op == ONAME && n.Name.Funcdepth > 0 && n.Name.Funcdepth != Funcdepth {
-		// inner func is referring to var in outer func.
+		// Inner func is referring to var in outer func.
 		//
 		// TODO(rsc): If there is an outer variable x and we
 		// are parsing x := 5 inside the closure, until we get to
 		// the := it looks like a reference to the outer x so we'll
 		// make x a closure variable unnecessarily.
-		if n.Name.Param.Closure == nil || n.Name.Param.Closure.Name.Funcdepth != Funcdepth {
-			// create new closure var.
-			c := Nod(ONAME, nil, nil)
-
+		c := n.Name.Param.Innermost
+		if c == nil || c.Name.Funcdepth != Funcdepth {
+			// Do not have a closure var for the active closure yet; make one.
+			c = Nod(ONAME, nil, nil)
 			c.Sym = s
 			c.Class = PAUTOHEAP
-			c.setIsClosureParam(true)
+			c.setIsClosureVar(true)
 			c.Isddd = n.Isddd
 			c.Name.Defn = n
 			c.Addable = false
 			c.Ullman = 2
 			c.Name.Funcdepth = Funcdepth
-			c.Name.Param.Outer = n.Name.Param.Closure
-			n.Name.Param.Closure = c
-			c.Name.Param.Closure = n
+			
+			// Link into list of active closure variables.
+			// Popped from list in func closurebody.
+			c.Name.Param.Outer = n.Name.Param.Innermost
+			n.Name.Param.Innermost = c
+
 			c.Xoffset = 0
 			Curfn.Func.Cvars.Append(c)
 		}
 
 		// return ref to closure var, not original
-		return n.Name.Param.Closure
+		return c
 	}
 
 	return n

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

@@ -900,13 +900,13 @@ func esc(e *EscState, n *Node, up *Node) {
 			escassignSinkNilWhy(e, n, n7.Right, "map literal value")
 		}
 
-		// Link addresses of captured variables to closure.
 	case OCLOSURE:
+		// Link addresses of captured variables to closure.
 		for _, v := range n.Func.Cvars.Slice() {
 			if v.Op == OXXX { // unnamed out argument; see dcl.go:/^funcargs
 				continue
 			}
-			a := v.Name.Param.Closure
+			a := v.Name.Defn
 			if !v.Name.Byval {
 				a = Nod(OADDR, a, nil)
 				a.Lineno = v.Lineno
@@ -1819,12 +1819,12 @@ func escwalkBody(e *EscState, level Level, dst *Node, src *Node, step *EscStep,
 
 		// Treat a captured closure variable as equivalent to the
 		// original variable.
-		if src.isClosureParam() {
+		if src.isClosureVar() {
 			if leaks && Debug['m'] != 0 {
 				Warnl(src.Lineno, "leaking closure reference %v", Nconv(src, FmtShort))
 				step.describe(src)
 			}
-			escwalk(e, level, dst, src.Name.Param.Closure, e.stepWalk(dst, src.Name.Param.Closure, "closure-var", step))
+			escwalk(e, level, dst, src.Name.Defn, e.stepWalk(dst, src.Name.Defn, "closure-var", step))
 		}
 
 	case OPTRLIT, OADDR:

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

@@ -1193,7 +1193,7 @@ func exprfmt(n *Node, prec int) string {
 		if n.Nbody.Len() != 0 {
 			return fmt.Sprintf("%v { %v }", n.Type, n.Nbody)
 		}
-		return fmt.Sprintf("%v { %v }", n.Type, n.Name.Param.Closure.Nbody)
+		return fmt.Sprintf("%v { %v }", n.Type, n.Func.Closure.Nbody)
 
 	case OCOMPLIT:
 		ptrlit := n.Right != nil && n.Right.Implicit && n.Right.Type != nil && n.Right.Type.IsPtr()

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

@@ -44,7 +44,7 @@ func addrescapes(n *Node) {
 		}
 
 		// If a closure reference escapes, mark the outer variable as escaping.
-		if n.isClosureParam() {
+		if n.isClosureVar() {
 			addrescapes(n.Name.Defn)
 			break
 		}

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

@@ -78,7 +78,7 @@ type Node struct {
 const (
 	hasBreak = 1 << iota
 	notLiveAtEnd
-	isClosureParam
+	isClosureVar
 )
 
 func (n *Node) HasBreak() bool {
@@ -101,14 +101,14 @@ func (n *Node) SetNotLiveAtEnd(b bool) {
 		n.flags &^= notLiveAtEnd
 	}
 }
-func (n *Node) isClosureParam() bool {
-	return n.flags&isClosureParam != 0
+func (n *Node) isClosureVar() bool {
+	return n.flags&isClosureVar != 0
 }
-func (n *Node) setIsClosureParam(b bool) {
+func (n *Node) setIsClosureVar(b bool) {
 	if b {
-		n.flags |= isClosureParam
+		n.flags |= isClosureVar
 	} else {
-		n.flags &^= isClosureParam
+		n.flags &^= isClosureVar
 	}
 }
 
@@ -158,8 +158,8 @@ func (n *Node) SetOpt(x interface{}) {
 type Name struct {
 	Pack      *Node // real package for import . names
 	Pkg       *Pkg  // pkg for OPACK nodes
-	Heapaddr  *Node // temp holding heap address of param
-	Inlvar    *Node // ONAME substitute while inlining
+	Heapaddr  *Node // temp holding heap address of param (could move to Param?)
+	Inlvar    *Node // ONAME substitute while inlining (could move to Param?)
 	Defn      *Node // initializing assignment
 	Curfn     *Node // function for local variables
 	Param     *Param // additional fields for ONAME, ODCLFIELD
@@ -179,15 +179,82 @@ type Param struct {
 	Ntype *Node
 
 	// ONAME PAUTOHEAP
-	Outerexpr *Node // expression copied into closure for variable
 	Stackcopy *Node // the PPARAM/PPARAMOUT on-stack slot (moved func params only)
 
 	// ONAME PPARAM
 	Field *Field // TFIELD in arg struct
 
 	// ONAME closure linkage
-	Outer   *Node
-	Closure *Node
+	// Consider:
+	//
+	//	func f() {
+	//		x := 1 // x1
+	//		func() {
+	//			use(x) // x2
+	//			func() {
+	//				use(x) // x3
+	//				--- parser is here ---
+	//			}()
+	//		}()
+	//	}
+	//
+	// There is an original declaration of x and then a chain of mentions of x
+	// leading into the current function. Each time x is mentioned in a new closure,
+	// we create a variable representing x for use in that specific closure,
+	// since the way you get to x is different in each closure.
+	//
+	// Let's number the specific variables as shown in the code:
+	// x1 is the original x, x2 is when mentioned in the closure,
+	// and x3 is when mentioned in the closure in the closure.
+	//
+	// We keep these linked (assume N > 1):
+	//
+	//   - x1.Defn = original declaration statement for x (like most variables)
+	//   - x1.Innermost = current innermost closure x (in this case x3), or nil for none
+	//   - x1.isClosureVar() = false
+	//
+	//   - xN.Defn = x1, N > 1
+	//   - xN.isClosureVar() = true, N > 1
+	//   - x2.Outer = nil
+	//   - xN.Outer = x(N-1), N > 2
+	//
+	//
+	// When we look up x in the symbol table, we always get x1.
+	// Then we can use x1.Innermost (if not nil) to get the x
+	// for the innermost known closure function,
+	// but the first reference in a closure will find either no x1.Innermost
+	// or an x1.Innermost with .Funcdepth < Funcdepth.
+	// In that case, a new xN must be created, linked in with:
+	//
+	//     xN.Defn = x1
+	//     xN.Outer = x1.Innermost
+	//     x1.Innermost = xN
+	//
+	// When we finish the function, we'll process its closure variables
+	// and find xN and pop it off the list using:
+	//
+	//     x1 := xN.Defn
+	//     x1.Innermost = xN.Outer
+	//
+	// We leave xN.Innermost set so that we can still get to the original
+	// variable quickly. Not shown here, but once we're
+	// done parsing a function and no longer need xN.Outer for the
+	// lexical x reference links as described above, closurebody
+	// recomputes xN.Outer as the semantic x reference link tree,
+	// even filling in x in intermediate closures that might not
+	// have mentioned it along the way to inner closures that did.
+	// See closurebody for details.
+	//
+	// During the eventual compilation, then, for closure variables we have:
+	//
+	//     xN.Defn = original variable
+	//     xN.Outer = variable captured in next outward scope
+	//                to make closure where xN appears
+	//
+	// Because of the sharding of pieces of the node, x.Defn means x.Name.Defn
+	// and x.Innermost/Outer means x.Name.Param.Innermost/Outer.
+	Innermost *Node
+	Outer *Node
 }
 
 // Func holds Node fields used only with function-like nodes.

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

@@ -796,8 +796,8 @@ OpSwitch:
 		var l *Node
 		for l = n.Left; l != r; l = l.Left {
 			l.Addrtaken = true
-			if l.Name != nil && l.Name.Param != nil && l.Name.Param.Closure != nil {
-				l.Name.Param.Closure.Addrtaken = true
+			if l.isClosureVar() {
+				l.Name.Defn.Addrtaken = true
 			}
 		}
 
@@ -805,8 +805,8 @@ OpSwitch:
 			Fatalf("found non-orig name node %v", l)
 		}
 		l.Addrtaken = true
-		if l.Name != nil && l.Name.Param != nil && l.Name.Param.Closure != nil {
-			l.Name.Param.Closure.Addrtaken = true
+		if l.isClosureVar() {
+			l.Name.Defn.Addrtaken = true
 		}
 		n.Left = defaultlit(n.Left, nil)
 		l = n.Left
@@ -3128,14 +3128,14 @@ func checkassign(stmt *Node, n *Node) {
 		var l *Node
 		for l = n; l != r; l = l.Left {
 			l.Assigned = true
-			if l.Name != nil && l.Name.Param != nil && l.Name.Param.Closure != nil {
-				l.Name.Param.Closure.Assigned = true
+			if l.isClosureVar() {
+				l.Name.Defn.Assigned = true
 			}
 		}
 
 		l.Assigned = true
-		if l.Name != nil && l.Name.Param != nil && l.Name.Param.Closure != nil {
-			l.Name.Param.Closure.Assigned = true
+		if l.isClosureVar() {
+			l.Name.Defn.Assigned = true
 		}
 	}