exp/ssa: reimplement logic for field selection.

The previous approach desugared the ast.SelectorExpr
to make implicit field selections explicit.  But:
1) it was clunky since it required allocating temporary
   syntax trees.
2) it was not thread-safe since it required poking
   types into the shared type map for the new ASTs.
3) the desugared syntax had no place to represent the
   package lexically enclosing each implicit field
   selection, so it was as if they all occurred in the
   same package as the explicit field selection.
   This meant unexported field names changed meaning.

This CL does what I should have done all along: just
generate the SSA instructions directly from the original
AST and the promoted field information.

Also:
- add logStack util for paired start/end log messages.
  Useful for debugging crashes.

R=gri
CC=golang-dev
https://golang.org/cl/7395052

src/pkg/exp/ssa/builder.go

@@ -23,8 +23,6 @@ package ssa
 // The Builder's and Program's indices (maps) are populated and
 // mutated during the CREATE phase, but during the BUILD phase they
 // remain constant, with the following exceptions:
-// - demoteSelector mutates Builder.types during the BUILD phase.
-//   TODO(adonovan): fix: let's not do that.
 // - globalValueSpec mutates Builder.nTo1Vars.
 //   TODO(adonovan): make this a per-Package map so it's thread-safe.
 // - Program.methodSets is populated lazily across phases.
@@ -517,63 +515,100 @@ func (b *Builder) builtin(fn *Function, name string, args []ast.Expr, typ types.
 	return nil // treat all others as a regular function call
 }
 
-// demoteSelector returns a SelectorExpr syntax tree that is
-// equivalent to sel but contains no selections of promoted fields.
-// It returns the field index of the explicit (=outermost) selection.
+// selector evaluates the selector expression e and returns its value,
+// or if wantAddr is true, its address, in which case escaping
+// indicates whether the caller intends to use the resulting pointer
+// in a potentially escaping way.
 //
-// pkg is the package in which the reference occurs.  This is
-// significant because a non-exported field is considered distinct
-// from a field of that name in any other package.
-//
-// This is a rather clunky and inefficient implementation, but it
-// (a) is simple and hopefully self-evidently correct and
-// (b) permits us to decouple the demotion from the code generation,
-// the latter being performed in two modes: addr() for lvalues,
-// expr() for rvalues.
-// It does require mutation of Builder.types though; if we want to
-// make the Builder concurrent we'll have to avoid that.
-// TODO(adonovan): opt: emit code directly rather than desugaring the AST.
-//
-func (b *Builder) demoteSelector(sel *ast.SelectorExpr, pkg *Package) (sel2 *ast.SelectorExpr, index int) {
-	id := makeId(sel.Sel.Name, pkg.Types)
-	xtype := b.exprType(sel.X)
-	// fmt.Fprintln(os.Stderr, xtype, id) // debugging
-	st := underlyingType(deref(xtype)).(*types.Struct)
+func (b *Builder) selector(fn *Function, e *ast.SelectorExpr, wantAddr, escaping bool) Value {
+	id := makeId(e.Sel.Name, fn.Pkg.Types)
+	st := underlyingType(deref(b.exprType(e.X))).(*types.Struct)
+	index := -1
 	for i, f := range st.Fields {
 		if IdFromQualifiedName(f.QualifiedName) == id {
-			return sel, i
+			index = i
+			break
 		}
 	}
-	// Not a named field.  Use breadth-first algorithm.
-	path, index := findPromotedField(st, id)
-	if path == nil {
-		panic("field not found, even with promotion: " + sel.Sel.Name)
+	var path *anonFieldPath
+	if index == -1 {
+		// Not a named field.  Use breadth-first algorithm.
+		path, index = findPromotedField(st, id)
+		if path == nil {
+			panic("field not found, even with promotion: " + e.Sel.Name)
+		}
 	}
+	fieldType := b.exprType(e)
+	if wantAddr {
+		return b.fieldAddr(fn, e.X, path, index, fieldType, escaping)
+	}
+	return b.fieldExpr(fn, e.X, path, index, fieldType)
+}
 
-	// makeSelector(e, [C,B,A]) returns (((e.A).B).C).
-	// e is the original selector's base.
-	// This function has no free variables.
-	var makeSelector func(b *Builder, e ast.Expr, path *anonFieldPath) *ast.SelectorExpr
-	makeSelector = func(b *Builder, e ast.Expr, path *anonFieldPath) *ast.SelectorExpr {
-		x := e
-		if path.tail != nil {
-			x = makeSelector(b, e, path.tail)
+// fieldAddr evaluates the base expression (a struct or *struct),
+// applies to it any implicit field selections from path, and then
+// selects the field #index of type fieldType.
+// Its address is returned.
+//
+// (fieldType can be derived from base+index.)
+//
+func (b *Builder) fieldAddr(fn *Function, base ast.Expr, path *anonFieldPath, index int, fieldType types.Type, escaping bool) Value {
+	var x Value
+	if path != nil {
+		switch underlyingType(path.field.Type).(type) {
+		case *types.Struct:
+			x = b.fieldAddr(fn, base, path.tail, path.index, path.field.Type, escaping)
+		case *types.Pointer:
+			x = b.fieldExpr(fn, base, path.tail, path.index, path.field.Type)
 		}
-		sel := &ast.SelectorExpr{
-			X:   x,
-			Sel: &ast.Ident{Name: path.field.Name},
+	} else {
+		switch underlyingType(b.exprType(base)).(type) {
+		case *types.Struct:
+			x = b.addr(fn, base, escaping).(address).addr
+		case *types.Pointer:
+			x = b.expr(fn, base)
 		}
-		b.types[sel] = path.field.Type // TODO(adonovan): opt: not thread-safe
-		return sel
 	}
+	v := &FieldAddr{
+		X:     x,
+		Field: index,
+	}
+	v.setType(pointer(fieldType))
+	return fn.emit(v)
+}
 
-	// Construct new SelectorExpr, bottom up.
-	sel2 = &ast.SelectorExpr{
-		X:   makeSelector(b, sel.X, path),
-		Sel: sel.Sel,
+// fieldExpr evaluates the base expression (a struct or *struct),
+// applies to it any implicit field selections from path, and then
+// selects the field #index of type fieldType.
+// Its value is returned.
+//
+// (fieldType can be derived from base+index.)
+//
+func (b *Builder) fieldExpr(fn *Function, base ast.Expr, path *anonFieldPath, index int, fieldType types.Type) Value {
+	var x Value
+	if path != nil {
+		x = b.fieldExpr(fn, base, path.tail, path.index, path.field.Type)
+	} else {
+		x = b.expr(fn, base)
 	}
-	b.types[sel2] = b.exprType(sel) // TODO(adonovan): opt: not thread-safe
-	return
+	switch underlyingType(x.Type()).(type) {
+	case *types.Struct:
+		v := &Field{
+			X:     x,
+			Field: index,
+		}
+		v.setType(fieldType)
+		return fn.emit(v)
+
+	case *types.Pointer: // *struct
+		v := &FieldAddr{
+			X:     x,
+			Field: index,
+		}
+		v.setType(pointer(fieldType))
+		return emitLoad(fn, fn.emit(v))
+	}
+	panic("unreachable")
 }
 
 // addr lowers a single-result addressable expression e to SSA form,
@@ -633,20 +668,7 @@ func (b *Builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue {
 		}
 
 		// e.f where e is an expression.
-		e, index := b.demoteSelector(e, fn.Pkg)
-		var x Value
-		switch underlyingType(b.exprType(e.X)).(type) {
-		case *types.Struct:
-			x = b.addr(fn, e.X, escaping).(address).addr
-		case *types.Pointer:
-			x = b.expr(fn, e.X)
-		}
-		v := &FieldAddr{
-			X:     x,
-			Field: index,
-		}
-		v.setType(pointer(b.exprType(e)))
-		return address{fn.emit(v)}
+		return address{b.selector(fn, e, true, escaping)}
 
 	case *ast.IndexExpr:
 		var x Value
@@ -867,33 +889,19 @@ func (b *Builder) expr(fn *Function, e ast.Expr) Value {
 		}
 
 		// (*T).f or T.f, the method f from the method-set of type T.
+		xtype := b.exprType(e.X)
 		if b.isType(e.X) {
 			id := makeId(e.Sel.Name, fn.Pkg.Types)
-			typ := b.exprType(e.X)
-			if m := b.Prog.MethodSet(typ)[id]; m != nil {
+			if m := b.Prog.MethodSet(xtype)[id]; m != nil {
 				return m
 			}
 
 			// T must be an interface; return method thunk.
-			return makeImethodThunk(b.Prog, typ, id)
+			return makeImethodThunk(b.Prog, xtype, id)
 		}
 
 		// e.f where e is an expression.
-		e, index := b.demoteSelector(e, fn.Pkg)
-		switch underlyingType(b.exprType(e.X)).(type) {
-		case *types.Struct:
-			// Non-addressable struct in a register.
-			v := &Field{
-				X:     b.expr(fn, e.X),
-				Field: index,
-			}
-			v.setType(b.exprType(e))
-			return fn.emit(v)
-
-		case *types.Pointer: // *struct
-			// Addressable structs; use FieldAddr and Load.
-			return b.addr(fn, e, false).load(fn)
-		}
+		return b.selector(fn, e, false, false)
 
 	case *ast.IndexExpr:
 		switch t := underlyingType(b.exprType(e.X)).(type) {
@@ -1338,7 +1346,7 @@ func (b *Builder) globalValueSpec(init *Function, spec *ast.ValueSpec, g *Global
 		if !b.nTo1Vars[spec] {
 			b.nTo1Vars[spec] = true
 			if b.mode&LogSource != 0 {
-				fmt.Fprintln(os.Stderr, "build globals", spec.Names) // ugly...
+				defer logStack("build globals %s", spec.Names)()
 			}
 			tuple := b.exprN(init, spec.Values[0])
 			rtypes := tuple.Type().(*types.Result).Values
@@ -2287,6 +2295,10 @@ start:
 		}
 		var results []Value
 		// Per the spec, there are three distinct cases of return.
+		// TODO(adonovan): fix: the design of Ret is incorrect:
+		// deferred procedures may modify named result locations
+		// after "Ret" has loaded its operands, causing the calls's
+		// result to change.  Tricky... rethink.
 		switch {
 		case len(s.Results) == 0:
 			// Return with no arguments.
@@ -2412,6 +2424,10 @@ func (b *Builder) buildFunction(fn *Function) {
 	if fn.syntax.body == nil {
 		return // Go source function with no body (external)
 	}
+	if fn.Prog.mode&LogSource != 0 {
+		defer logStack("build function %s @ %s",
+			fn.FullName(), fn.Prog.Files.Position(fn.Pos))()
+	}
 	fn.start(b.idents)
 	b.stmt(fn, fn.syntax.body)
 	if cb := fn.currentBlock; cb != nil && (cb == fn.Blocks[0] || cb.Preds != nil) {
@@ -2676,7 +2692,8 @@ func (b *Builder) buildDecl(pkg *Package, decl ast.Decl) {
 		} else if decl.Recv == nil && id.Name == "init" {
 			// init() block
 			if b.mode&LogSource != 0 {
-				fmt.Fprintln(os.Stderr, "build init block @", b.Prog.Files.Position(decl.Pos()))
+				fmt.Fprintln(os.Stderr, "build init block @",
+					b.Prog.Files.Position(decl.Pos()))
 			}
 			init := pkg.Init
 
@@ -2711,7 +2728,7 @@ func (b *Builder) BuildPackage(p *Package) {
 		return // already done (or nothing to do)
 	}
 	if b.mode&LogSource != 0 {
-		fmt.Fprintln(os.Stderr, "build package", p.Types.Path)
+		defer logStack("build package %s", p.Types.Path)()
 	}
 	init := p.Init
 	init.start(b.idents)

src/pkg/exp/ssa/func.go

@@ -195,9 +195,6 @@ func (f *Function) addSpilledParam(obj types.Object) {
 // functions is skipped.
 //
 func (f *Function) start(idents map[*ast.Ident]types.Object) {
-	if f.Prog.mode&LogSource != 0 {
-		fmt.Fprintf(os.Stderr, "build function %s @ %s\n", f.FullName(), f.Prog.Files.Position(f.Pos))
-	}
 	f.currentBlock = f.newBasicBlock("entry")
 	f.objects = make(map[types.Object]Value) // needed for some synthetics, e.g. init
 	if f.syntax == nil {
@@ -327,9 +324,6 @@ func (f *Function) finish() {
 	if f.Prog.mode&SanityCheckFunctions != 0 {
 		MustSanityCheck(f, nil)
 	}
-	if f.Prog.mode&LogSource != 0 {
-		fmt.Fprintf(os.Stderr, "build function %s done\n", f.FullName())
-	}
 }
 
 // removeNilBlocks eliminates nils from f.Blocks and updates each

src/pkg/exp/ssa/promote.go

@@ -263,7 +263,7 @@ func makeBridgeMethod(prog *Program, typ types.Type, cand *candidate) *Function
 	sig.Recv = &types.Var{Name: "recv", Type: typ}
 
 	if prog.mode&LogSource != 0 {
-		fmt.Fprintf(os.Stderr, "makeBridgeMethod %s, %s, type %s\n", typ, cand, &sig)
+		defer logStack("makeBridgeMethod %s, %s, type %s", typ, cand, &sig)()
 	}
 
 	fn := &Function{
@@ -361,7 +361,7 @@ func makeBridgeMethod(prog *Program, typ types.Type, cand *candidate) *Function
 //
 func makeImethodThunk(prog *Program, typ types.Type, id Id) *Function {
 	if prog.mode&LogSource != 0 {
-		fmt.Fprintf(os.Stderr, "makeImethodThunk %s.%s\n", typ, id)
+		defer logStack("makeImethodThunk %s.%s", typ, id)()
 	}
 	itf := underlyingType(typ).(*types.Interface)
 	index, meth := methodIndex(itf, itf.Methods, id)

src/pkg/exp/ssa/ssa.go

@@ -945,7 +945,7 @@ type Register struct {
 	referrers []Instruction
 }
 
-// AnInstruction is a mix-in embedded by all Instructions.
+// anInstruction is a mix-in embedded by all Instructions.
 // It provides the implementations of the Block and SetBlock methods.
 type anInstruction struct {
 	Block_ *BasicBlock // the basic block of this instruction

src/pkg/exp/ssa/util.go

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"go/ast"
 	"go/types"
+	"io"
+	"os"
 	"reflect"
 )
 
@@ -208,3 +210,18 @@ func (p ids) Less(i, j int) bool {
 		x.Pkg == y.Pkg && x.Name < y.Name
 }
 func (p ids) Swap(i, j int) { p[i], p[j] = p[j], p[i] }
+
+// logStack prints the formatted "start" message to stderr and
+// returns a closure that prints the corresponding "end" message.
+// Call using 'defer logStack(...)()' to show builder stack on panic.
+// Don't forget trailing parens!
+//
+func logStack(format string, args ...interface{}) func() {
+	msg := fmt.Sprintf(format, args...)
+	io.WriteString(os.Stderr, msg)
+	io.WriteString(os.Stderr, "\n")
+	return func() {
+		io.WriteString(os.Stderr, msg)
+		io.WriteString(os.Stderr, " end\n")
+	}
+}