From 4ca3d7e9da201f61ed3c9ccef3309de968013309 Mon Sep 17 00:00:00 2001
From: Robert Griesemer <gri@golang.org>
Date: Wed, 3 Jul 2013 20:51:39 -0700
Subject: [PATCH] go.tools/go/types: remove defers from critical paths

Various minor cleanups.

R=adonovan
CC=golang-dev
https://golang.org/cl/10925043
---
 go/types/api.go           |   4 +-
 go/types/check.go         |   8 +--
 go/types/errors.go        |  35 ++++--------
 go/types/expr.go          | 108 +++++++++++++++++++-------------------
 go/types/operand.go       |   2 +
 go/types/resolver_test.go |   5 +-
 go/types/stmt.go          |   1 +
 go/types/typexpr.go       |  28 +++++++---
 8 files changed, 96 insertions(+), 95 deletions(-)

diff --git a/go/types/api.go b/go/types/api.go
index eced819f..be7f8abc 100644
--- a/go/types/api.go
+++ b/go/types/api.go
@@ -33,7 +33,6 @@ package types
 // pass without errors. Please do not file issues against these for now
 // since they are known already:
 //
-// BUG(gri): Method expressions and method values work accidentally and may not be fully checked.
 // BUG(gri): Conversions of constants only change the type, not the value (e.g., int(1.1) is wrong).
 // BUG(gri): Some built-ins don't check parameters fully, yet (e.g. append).
 // BUG(gri): Use of labels is not checked.
@@ -89,7 +88,8 @@ type Context struct {
 
 	// If Expr != nil, it is called exactly once for each expression x
 	// that is type-checked: typ is the expression type, and val is the
-	// value if x is constant, val is nil otherwise.
+	// value if x is constant, val is nil otherwise. Expr is not called
+	// for identifiers appearing on the lhs of declarations.
 	//
 	// If x is a literal value (constant, composite literal), typ is always
 	// the dynamic type of x (never an interface type). Otherwise, typ is x's
diff --git a/go/types/check.go b/go/types/check.go
index 027c06c2..fe154ae5 100644
--- a/go/types/check.go
+++ b/go/types/check.go
@@ -51,9 +51,11 @@ type checker struct {
 	iota     exact.Value      // value of iota in a constant declaration; nil otherwise
 
 	// functions
-	funclist []function  // list of functions/methods with correct signatures and non-empty bodies
-	funcsig  *Signature  // signature of currently typechecked function
-	pos      []token.Pos // stack of expr positions; debugging support, used if trace is set
+	funclist []function // list of functions/methods with correct signatures and non-empty bodies
+	funcsig  *Signature // signature of currently typechecked function
+
+	// debugging
+	indent int // indentation for tracing
 }
 
 func (check *checker) callIdent(id *ast.Ident, obj Object) {
diff --git a/go/types/errors.go b/go/types/errors.go
index f354a2ef..c11a3f82 100644
--- a/go/types/errors.go
+++ b/go/types/errors.go
@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
+	"strings"
 )
 
 // TODO(gri) eventually assert should disappear.
@@ -24,32 +25,6 @@ func unreachable() {
 	panic("unreachable")
 }
 
-func (check *checker) printTrace(format string, args []interface{}) {
-	const dots = ".  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  "
-	n := len(check.pos) - 1
-	i := 3 * n
-	for i > len(dots) {
-		fmt.Print(dots)
-		i -= len(dots)
-	}
-	// i <= len(dots)
-	fmt.Printf("%s:\t", check.fset.Position(check.pos[n]))
-	fmt.Print(dots[0:i])
-	fmt.Println(check.formatMsg(format, args))
-}
-
-func (check *checker) trace(pos token.Pos, format string, args ...interface{}) {
-	check.pos = append(check.pos, pos)
-	check.printTrace(format, args)
-}
-
-func (check *checker) untrace(format string, args ...interface{}) {
-	if len(format) > 0 {
-		check.printTrace(format, args)
-	}
-	check.pos = check.pos[:len(check.pos)-1]
-}
-
 func (check *checker) formatMsg(format string, args []interface{}) string {
 	for i, arg := range args {
 		switch a := arg.(type) {
@@ -72,6 +47,14 @@ func (check *checker) formatMsg(format string, args []interface{}) string {
 	return fmt.Sprintf(format, args...)
 }
 
+func (check *checker) trace(pos token.Pos, format string, args ...interface{}) {
+	fmt.Printf("%s:\t%s%s\n",
+		check.fset.Position(pos),
+		strings.Repeat(".  ", check.indent),
+		check.formatMsg(format, args),
+	)
+}
+
 // dump is only needed for debugging
 func (check *checker) dump(format string, args ...interface{}) {
 	fmt.Println(check.formatMsg(format, args))
diff --git a/go/types/expr.go b/go/types/expr.go
index 6f34b71c..6b416720 100644
--- a/go/types/expr.go
+++ b/go/types/expr.go
@@ -764,63 +764,67 @@ func (check *checker) indexedElts(elts []ast.Expr, typ Type, length int64) int64
 	return max
 }
 
-func (check *checker) callExpr(x *operand, ignore *bool) {
-	if *ignore {
-		return
-	}
-
-	// convert x into a user-friendly set of values
-	var typ Type
-	var val exact.Value
-	switch x.mode {
-	case invalid:
-		return // nothing to do
-	case novalue:
-		typ = (*Tuple)(nil)
-	case constant:
-		typ = x.typ
-		val = x.val
-	default:
-		typ = x.typ
-	}
-
-	// if the operand is untyped, delay notification
-	// until it becomes typed or until the end of
-	// type checking
-	if isUntyped(typ) {
-		check.untyped[x.expr] = exprInfo{false, typ.(*Basic), val}
-		return
-	}
-
-	// TODO(gri) ensure that literals always report
-	// their dynamic (never interface) type.
-	// This is not the case yet.
-
-	if check.ctxt.Expr != nil {
-		assert(x.expr != nil && typ != nil)
-		check.ctxt.Expr(x.expr, typ, val)
-	}
-}
-
 // rawExpr typechecks expression e and initializes x with the expression
 // value or type. If an error occurred, x.mode is set to invalid.
 // If hint != nil, it is the type of a composite literal element.
 //
 func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type) {
-	// make sure x has a valid state for deferred functions in case of bailout
+	if trace {
+		check.trace(e.Pos(), "%s", e)
+		check.indent++
+	}
+
+	check.expr0(x, e, hint)
+
+	// convert x into a user-friendly set of values
+	notify := check.ctxt.Expr
+	var typ Type
+	var val exact.Value
+	switch x.mode {
+	case invalid:
+		typ = Typ[Invalid]
+		notify = nil // nothing to do
+	case novalue:
+		typ = (*Tuple)(nil)
+	case constant:
+		typ = x.typ
+		val = x.val
+	case typexprn:
+		x.mode = typexpr
+		typ = x.typ
+		notify = nil // clients were already notified
+	default:
+		typ = x.typ
+	}
+	assert(x.expr != nil && typ != nil)
+
+	if isUntyped(typ) {
+		// delay notification until it becomes typed
+		// or until the end of type checking
+		check.untyped[x.expr] = exprInfo{false, typ.(*Basic), val}
+	} else if notify != nil {
+		// notify clients
+		// TODO(gri) ensure that literals always report
+		// their dynamic (never interface) type.
+		// This is not the case yet.
+		notify(x.expr, typ, val)
+	}
+
+	if trace {
+		check.indent--
+		check.trace(e.Pos(), "=> %s", x)
+	}
+}
+
+// expr0 contains the core of type checking of expressions.
+// Must only be called by rawExpr.
+//
+func (check *checker) expr0(x *operand, e ast.Expr, hint Type) {
+	// make sure x has a valid state in case of bailout
 	// (was issue 5770)
 	x.mode = invalid
 	x.typ = Typ[Invalid]
 
-	if trace {
-		check.trace(e.Pos(), "%s", e)
-		defer check.untrace("=> %s", x)
-	}
-
-	// record final type of x if untyped, notify clients of type otherwise
-	ignore := false // if set, don't do anything in the deferred call
-	defer check.callExpr(x, &ignore)
-
 	switch e := e.(type) {
 	case *ast.BadExpr:
 		goto Error // error was reported before
@@ -1065,7 +1069,7 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type) {
 
 		if e.Index == nil {
 			check.invalidAST(e.Pos(), "missing index expression for %s", x)
-			return
+			goto Error
 		}
 
 		check.index(e.Index, length)
@@ -1182,7 +1186,6 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type) {
 			// ok to continue
 		}
 		x.mode = valueok
-		x.expr = e
 		x.typ = typ
 
 	case *ast.CallExpr:
@@ -1228,11 +1231,8 @@ func (check *checker) rawExpr(x *operand, e ast.Expr, hint Type) {
 
 	case *ast.ArrayType, *ast.StructType, *ast.FuncType,
 		*ast.InterfaceType, *ast.MapType, *ast.ChanType:
-		x.mode = typexpr
+		x.mode = typexprn // not typexpr; Context.Expr callback was invoked by check.typ
 		x.typ = check.typ(e, nil, false)
-		// check.typ is already notifying clients
-		// of e's type; don't do it a 2nd time
-		ignore = true
 
 	default:
 		if debug {
diff --git a/go/types/operand.go b/go/types/operand.go
index b12ba569..cc74be64 100644
--- a/go/types/operand.go
+++ b/go/types/operand.go
@@ -22,6 +22,7 @@ const (
 	invalid  operandMode = iota // operand is invalid (due to an earlier error) - ignore
 	novalue                     // operand represents no value (result of a function call w/o result)
 	typexpr                     // operand is a type
+	typexprn                    // like typexpr; only used to communicate between checker.expr0 and checker.rawExpr
 	constant                    // operand is a constant; the operand's typ is a Basic type
 	variable                    // operand is an addressable variable
 	value                       // operand is a computed value
@@ -32,6 +33,7 @@ var operandModeString = [...]string{
 	invalid:  "invalid",
 	novalue:  "no value",
 	typexpr:  "type",
+	typexprn: "type/n",
 	constant: "constant",
 	variable: "variable",
 	value:    "value",
diff --git a/go/types/resolver_test.go b/go/types/resolver_test.go
index 7996014d..2960bd07 100644
--- a/go/types/resolver_test.go
+++ b/go/types/resolver_test.go
@@ -45,6 +45,7 @@ var sources = []string{
 	func (_ T) m() {}
 	var i I
 	var _ = i.m
+	func _(s []int) { for i, x := range s {} } 
 	`,
 }
 
@@ -53,7 +54,7 @@ var pkgnames = []string{
 	"math",
 }
 
-func TestResolveQualifiedIdents(t *testing.T) {
+func TestResolveIdents(t *testing.T) {
 	// parse package files
 	fset := token.NewFileSet()
 	var files []*ast.File
@@ -74,7 +75,7 @@ func TestResolveQualifiedIdents(t *testing.T) {
 		}
 		idents[id] = obj
 	}
-	pkg, err := ctxt.Check("testResolveQualifiedIdents", fset, files...)
+	pkg, err := ctxt.Check("testResolveIdents", fset, files...)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/go/types/stmt.go b/go/types/stmt.go
index 3e749af7..c362aea5 100644
--- a/go/types/stmt.go
+++ b/go/types/stmt.go
@@ -738,6 +738,7 @@ func (check *checker) stmt(s ast.Stmt) {
 		check.expr(&x, s.X)
 		if x.mode == invalid {
 			// if we don't have a declaration, we can still check the loop's body
+			// (otherwise we can't because we are missing the declared variables)
 			if !decl {
 				check.stmt(s.Body)
 			}
diff --git a/go/types/typexpr.go b/go/types/typexpr.go
index f0dda03f..d6c501bf 100644
--- a/go/types/typexpr.go
+++ b/go/types/typexpr.go
@@ -98,20 +98,32 @@ func (check *checker) ident(x *operand, e *ast.Ident, def *Named, cycleOk bool)
 // If cycleOk is set, e (or elements of e) may refer to a named type that is not
 // yet completely set up.
 //
-func (check *checker) typ(e ast.Expr, def *Named, cycleOk bool) (res Type) {
+func (check *checker) typ(e ast.Expr, def *Named, cycleOk bool) Type {
 	if trace {
 		check.trace(e.Pos(), "%s", e)
-		defer check.untrace("=> %s", res)
+		check.indent++
 	}
 
-	// notify clients of type
-	if f := check.ctxt.Expr; f != nil {
-		defer func() {
-			assert(e != nil && res != nil && !isUntyped(res))
-			f(e, res, nil)
-		}()
+	t := check.typ0(e, def, cycleOk)
+	assert(e != nil && t != nil && !isUntyped(t))
+
+	// notify clients
+	if notify := check.ctxt.Expr; notify != nil {
+		notify(e, t, nil)
 	}
 
+	if trace {
+		check.indent--
+		check.trace(e.Pos(), "=> %s", t)
+	}
+
+	return t
+}
+
+// typ0 contains the core of type checking of types.
+// Must only be called by typ.
+//
+func (check *checker) typ0(e ast.Expr, def *Named, cycleOk bool) Type {
 	switch e := e.(type) {
 	case *ast.Ident:
 		var x operand