go.tools/ssa: fix bug in Program.VarValue.

Before, VarValue looked for the ssa.Value for the 'var' object
in the same package as the object was defined, but this is
(obviously) wrong for a cross-package FieldVal selection,
expr.f.  The caller must provide the package containing the
reference.

+ test.

Also:
- add 2 TODOs.
- split builder.expr into two functions so we don't need
  defer, which makes panic dumps harder to read.

R=golang-dev, crawshaw
CC=golang-dev
https://golang.org/cl/13257045

ssa/builder.go

@@ -482,18 +482,20 @@ func (b *builder) exprInPlace(fn *Function, loc lvalue, e ast.Expr) {
 // expr lowers a single-result expression e to SSA form, emitting code
 // to fn and returning the Value defined by the expression.
 //
-func (b *builder) expr(fn *Function, e ast.Expr) (result Value) {
+func (b *builder) expr(fn *Function, e ast.Expr) Value {
 	// Is expression a constant?
 	if v := fn.Pkg.info.ValueOf(e); v != nil {
 		return NewConst(v, fn.Pkg.typeOf(e))
 	}
-
 	e = unparen(e)
-
+	v := b.expr0(fn, e)
 	if fn.debugInfo() {
-		defer func() { emitDebugRef(fn, e, result) }()
+		emitDebugRef(fn, e, v)
 	}
+	return v
+}
 
+func (b *builder) expr0(fn *Function, e ast.Expr) Value {
 	switch e := e.(type) {
 	case *ast.BasicLit:
 		panic("non-constant BasicLit") // unreachable
@@ -827,6 +829,9 @@ func (b *builder) setCallFunc(fn *Function, e *ast.CallExpr, c *CallCommon) {
 				c.Method = obj
 			} else {
 				// "Call"-mode call.
+				// TODO(adonovan): fix: in -build=G
+				// mode, declaredFunc panics for
+				// cross-package calls.
 				c.Value = fn.Prog.declaredFunc(obj)
 				c.Args = append(c.Args, v)
 			}

ssa/source.go

@@ -217,8 +217,11 @@ func (prog *Program) ConstValue(obj *types.Const) *Const {
 // because its package was not built, the debug information was not
 // requested during SSA construction, or the value was optimized away.
 //
-// ref must be the path to an ast.Ident (e.g. from
+// ref is the path to an ast.Ident (e.g. from PathEnclosingInterval),
-// PathEnclosingInterval), and that ident must resolve to obj.
+// and that ident must resolve to obj.
+//
+// pkg is the package enclosing the reference.  (A reference to a var
+// may result in code, so we need to know where to find that code.)
 //
 // The Value of a defining (as opposed to referring) identifier is the
 // value assigned to it in its definition.
@@ -235,7 +238,7 @@ func (prog *Program) ConstValue(obj *types.Const) *Const {
 // and all package-level vars.  (This situation can be detected by
 // comparing the types of the Var and Value.)
 //
-func (prog *Program) VarValue(obj *types.Var, ref []ast.Node) Value {
+func (prog *Program) VarValue(obj *types.Var, pkg *Package, ref []ast.Node) Value {
 	id := ref[0].(*ast.Ident)
 
 	// Package-level variable?
@@ -243,15 +246,8 @@ func (prog *Program) VarValue(obj *types.Var, ref []ast.Node) Value {
 		return v.(*Global)
 	}
 
-	// It's a local variable (or param) of some function.
+	// Must be a function-local variable.
-
+	// (e.g. local, parameter, or field selection e.f)
-	// The reference may occur inside a lexically nested function,
-	// so find that first.
-	pkg := prog.packages[obj.Pkg()]
-	if pkg == nil {
-		panic("no package for " + obj.String())
-	}
-
 	fn := EnclosingFunction(pkg, ref)
 	if fn == nil {
 		return nil // e.g. SSA not built

ssa/source_test.go

@@ -96,7 +96,7 @@ func TestObjValueLookup(t *testing.T) {
 				wantAddr = true
 				exp = exp[1:]
 			}
-			checkVarValue(t, prog, ref, obj, exp, wantAddr)
+			checkVarValue(t, prog, mainPkg, ref, obj, exp, wantAddr)
 		}
 	}
 }
@@ -148,12 +148,12 @@ func checkConstValue(t *testing.T, prog *ssa.Program, obj *types.Const) {
 	}
 }
 
-func checkVarValue(t *testing.T, prog *ssa.Program, ref []ast.Node, obj *types.Var, expKind string, wantAddr bool) {
+func checkVarValue(t *testing.T, prog *ssa.Program, pkg *ssa.Package, ref []ast.Node, obj *types.Var, expKind string, wantAddr bool) {
 	// The prefix of all assertions messages.
 	prefix := fmt.Sprintf("VarValue(%s @ L%d)",
 		obj, prog.Fset.Position(ref[0].Pos()).Line)
 
-	v := prog.VarValue(obj, ref)
+	v := prog.VarValue(obj, pkg, ref)
 
 	// Kind is the concrete type of the ssa Value.
 	gotKind := "nil"

ssa/ssadump.go

@@ -120,7 +120,7 @@ func main() {
 	}
 	prog.BuildAll()
 
-	prog.Package(info.Pkg).CreateTestMainFunction() // FIXME
+	prog.Package(info.Pkg).CreateTestMainFunction() // TODO(adonovan): remove hack
 
 	// Run the interpreter.
 	if *runFlag {

ssa/testdata/objlookup.go

@@ -16,6 +16,7 @@ package main
 // declaration is enough.
 
 import "fmt"
+import "os"
 
 type J int
 
@@ -131,4 +132,9 @@ func main() {
 	select {
 	case x := <-ch: // x::UnOp (receive) ch::MakeChan
 	}
+
+	// .Op is an inter-package FieldVal-selection.
+	var err os.PathError // err::UnOp
+	_ = err.Op           // err::UnOp Op::Field
+	_ = &err.Op          // &err::Alloc &Op::FieldAddr
 }