[dev.regabi] cmd/compile: clean up Node.Func

The original meaning of type Func was "extra fields factored out
of a few cases of type Node having to do with functions",
but those specific cases didn't necessarily have any relation.
A typical declared function is represented by an ODCLFUNC Node
at its declaration and an ONAME node at its uses, and both those
have a .Func field, but they are *different* Funcs.
Similarly, a closure is represented both by an OCLOSURE Node for
the value itself and an ODCLFUNC Node for the underlying function
implementing the closure. Those too have *different* Funcs,
and the Func.Closure field in one points to the other and vice versa.
This has led to no end of confusion over the years.

This CL elevates type Func to be the canonical identifier for
a given Go function.

This looks like a trivial CL but in fact is the result of a lot of
scaffolding and rewriting, discarded once the result was achieved, to
separate out the three different kinds of Func nodes into three
separate fields, limited in use to each specific Node type, to
understand which Func fields are used by which Node types and what the
possible overlaps are. There were a few overlaps, most notably around
closures, which led to more fields being added to type Func to keep
them separate even though there is now a single Func instead of two
different ones for each function.

A future CL can and should change Curfn to be a *Func instead of
a *Node, finally eliminating the confusion about whether Curfn
is an ODCLFUNC node (as it is most of the time) or an ONAME node
(as it is when type-checking an inlined function body).

Although sizeof_test.go makes it look like Func is growing by two
words, there are now half as many Funcs in a running compilation,
so the memory footprint has actually been reduced substantially.

Change-Id: I598bd96c95728093dc769a835d48f2154a406a61
Reviewed-on: https://go-review.googlesource.com/c/go/+/272253
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

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

@@ -404,30 +404,59 @@ func debuginfo(fnsym *obj.LSym, infosym *obj.LSym, curfn interface{}) ([]dwarf.S
 		}
 	}
 
+	// Back when there were two different *Funcs for a function, this code
+	// was not consistent about whether a particular *Node being processed
+	// was an ODCLFUNC or ONAME node. Partly this is because inlined function
+	// bodies have no ODCLFUNC node, which was it's own inconsistency.
+	// In any event, the handling of the two different nodes for DWARF purposes
+	// was subtly different, likely in unintended ways. CL 272253 merged the
+	// two nodes' Func fields, so that code sees the same *Func whether it is
+	// holding the ODCLFUNC or the ONAME. This resulted in changes in the
+	// DWARF output. To preserve the existing DWARF output and leave an
+	// intentional change for a future CL, this code does the following when
+	// fn.Op == ONAME:
+	//
+	// 1. Disallow use of createComplexVars in createDwarfVars.
+	//    It was not possible to reach that code for an ONAME before,
+	//    because the DebugInfo was set only on the ODCLFUNC Func.
+	//    Calling into it in the ONAME case causes an index out of bounds panic.
+	//
+	// 2. Do not populate apdecls. fn.Func.Dcl was in the ODCLFUNC Func,
+	//    not the ONAME Func. Populating apdecls for the ONAME case results
+	//    in selected being populated after createSimpleVars is called in
+	//    createDwarfVars, and then that causes the loop to skip all the entries
+	//    in dcl, meaning that the RecordAutoType calls don't happen.
+	//
+	// These two adjustments keep toolstash -cmp working for now.
+	// Deciding the right answer is, as they say, future work.
+	isODCLFUNC := fn.Op == ODCLFUNC
+
 	var apdecls []*Node
 	// Populate decls for fn.
-	for _, n := range fn.Func.Dcl {
-		if n.Op != ONAME { // might be OTYPE or OLITERAL
-			continue
-		}
-		switch n.Class() {
-		case PAUTO:
-			if !n.Name.Used() {
-				// Text == nil -> generating abstract function
-				if fnsym.Func().Text != nil {
-					Fatalf("debuginfo unused node (AllocFrame should truncate fn.Func.Dcl)")
-				}
+	if isODCLFUNC {
+		for _, n := range fn.Func.Dcl {
+			if n.Op != ONAME { // might be OTYPE or OLITERAL
 				continue
 			}
-		case PPARAM, PPARAMOUT:
-		default:
-			continue
+			switch n.Class() {
+			case PAUTO:
+				if !n.Name.Used() {
+					// Text == nil -> generating abstract function
+					if fnsym.Func().Text != nil {
+						Fatalf("debuginfo unused node (AllocFrame should truncate fn.Func.Dcl)")
+					}
+					continue
+				}
+			case PPARAM, PPARAMOUT:
+			default:
+				continue
+			}
+			apdecls = append(apdecls, n)
+			fnsym.Func().RecordAutoType(ngotype(n).Linksym())
 		}
-		apdecls = append(apdecls, n)
-		fnsym.Func().RecordAutoType(ngotype(n).Linksym())
 	}
 
-	decls, dwarfVars := createDwarfVars(fnsym, fn.Func, apdecls)
+	decls, dwarfVars := createDwarfVars(fnsym, isODCLFUNC, fn.Func, apdecls)
 
 	// For each type referenced by the functions auto vars but not
 	// already referenced by a dwarf var, attach a dummy relocation to
@@ -575,12 +604,12 @@ func createComplexVars(fnsym *obj.LSym, fn *Func) ([]*Node, []*dwarf.Var, map[*N
 
 // createDwarfVars process fn, returning a list of DWARF variables and the
 // Nodes they represent.
-func createDwarfVars(fnsym *obj.LSym, fn *Func, apDecls []*Node) ([]*Node, []*dwarf.Var) {
+func createDwarfVars(fnsym *obj.LSym, complexOK bool, fn *Func, apDecls []*Node) ([]*Node, []*dwarf.Var) {
 	// Collect a raw list of DWARF vars.
 	var vars []*dwarf.Var
 	var decls []*Node
 	var selected map[*Node]bool
-	if Ctxt.Flag_locationlists && Ctxt.Flag_optimize && fn.DebugInfo != nil {
+	if Ctxt.Flag_locationlists && Ctxt.Flag_optimize && fn.DebugInfo != nil && complexOK {
 		decls, vars, selected = createComplexVars(fnsym, fn)
 	} else {
 		decls, vars, selected = createSimpleVars(fnsym, apDecls)