go/types: simplify tracking of side effects for len/cap

LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/67190043

go/types/builtins.go

@@ -27,6 +27,18 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 		return
 	}
 
+	// For len(x) and cap(x) we need to know if x contains any function calls or
+	// receive operations. Save/restore current setting and set hasCallOrRecv to
+	// false for the evaluation of x so that we can check it afterwards.
+	// Note: We must do this _before_ calling unpack because unpack evaluates the
+	//       first argument before we even call arg(x, 0)!
+	if id == _Len || id == _Cap {
+		defer func(b bool) {
+			check.hasCallOrRecv = b
+		}(check.hasCallOrRecv)
+		check.hasCallOrRecv = false
+	}
+
 	// determine actual arguments
 	var arg getter
 	nargs := len(call.Args)
@@ -142,7 +154,7 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 			// if the type of s is an array or pointer to an array and
 			// the expression s does not contain channel receives or
 			// function calls; in this case s is not evaluated."
-			if !check.containsCallsOrReceives(x.expr) {
+			if !check.hasCallOrRecv {
 				mode = constant
 				val = exact.MakeInt64(t.len)
 			}
@@ -586,27 +598,6 @@ func implicitArrayDeref(typ Type) Type {
 	return typ
 }
 
-// containsCallsOrReceives reports if x contains function calls or channel receives.
-// Expects that x was type-checked already.
-//
-func (check *checker) containsCallsOrReceives(x ast.Expr) (found bool) {
-	ast.Inspect(x, func(x ast.Node) bool {
-		switch x := x.(type) {
-		case *ast.CallExpr:
-			// calls and conversions look the same
-			if !check.conversions[x] {
-				found = true
-			}
-		case *ast.UnaryExpr:
-			if x.Op == token.ARROW {
-				found = true
-			}
-		}
-		return !found // no need to continue if found
-	})
-	return
-}
-
 // unparen removes any parentheses surrounding an expression and returns
 // the naked expression.
 //

go/types/call.go

@@ -32,9 +32,6 @@ func (check *checker) call(x *operand, e *ast.CallExpr) exprKind {
 			check.expr(x, e.Args[0])
 			if x.mode != invalid {
 				check.conversion(x, T)
-				if x.mode != invalid {
-					check.markAsConversion(e) // for cap/len checking
-				}
 			}
 		default:
 			check.errorf(e.Args[n-1].Pos(), "too many arguments in conversion to %s", T)
@@ -48,6 +45,9 @@ func (check *checker) call(x *operand, e *ast.CallExpr) exprKind {
 			x.mode = invalid
 		}
 		x.expr = e
+		// TODO(gri) Depending on the pending decision on the issue 7387,
+		// hasCallOrRecv may only need to be set if the result is not constant.
+		check.hasCallOrRecv = true
 		return predeclaredFuncs[id].kind
 
 	default:
@@ -75,6 +75,7 @@ func (check *checker) call(x *operand, e *ast.CallExpr) exprKind {
 			x.typ = sig.results
 		}
 		x.expr = e
+		check.hasCallOrRecv = true
 
 		return statement
 	}

go/types/check.go

@@ -28,11 +28,12 @@ type exprInfo struct {
 
 // A context represents the context within which an object is type-checked.
 type context struct {
-	decl     *declInfo   // package-level declaration whose init expression/function body is checked
-	scope    *Scope      // top-most scope for lookups
-	iota     exact.Value // value of iota in a constant declaration; nil otherwise
-	sig      *Signature  // function signature if inside a function; nil otherwise
-	hasLabel bool        // set if a function makes use of labels (only ~1% of functions); unused outside functions
+	decl          *declInfo   // package-level declaration whose init expression/function body is checked
+	scope         *Scope      // top-most scope for lookups
+	iota          exact.Value // value of iota in a constant declaration; nil otherwise
+	sig           *Signature  // function signature if inside a function; nil otherwise
+	hasLabel      bool        // set if a function makes use of labels (only ~1% of functions); unused outside functions
+	hasCallOrRecv bool        // set if an expression contains a function call or channel receive operation
 }
 
 // A checker maintains the state of the type checker.
@@ -47,12 +48,11 @@ type checker struct {
 
 	// information collected during type-checking of an entire package
 	// (maps are allocated lazily)
-	firstErr    error                  // first error encountered
-	methods     map[string][]*Func     // maps type names to associated methods
-	conversions map[*ast.CallExpr]bool // set of type-checked conversions (to distinguish from calls)
-	untyped     map[ast.Expr]exprInfo  // map of expressions without final type
-	funcs       []funcInfo             // list of functions/methods with correct signatures and non-empty bodies
-	delayed     []func()               // delayed checks that require fully setup types
+	firstErr error                 // first error encountered
+	methods  map[string][]*Func    // maps type names to associated methods
+	untyped  map[ast.Expr]exprInfo // map of expressions without final type
+	funcs    []funcInfo            // list of functions/methods with correct signatures and non-empty bodies
+	delayed  []func()              // delayed checks that require fully setup types
 
 	objMap  map[Object]*declInfo // if set we are in the package-level declaration phase (otherwise all objects seen must be declared)
 	initMap map[Object]*declInfo // map of variables/functions with init expressions/bodies
@@ -74,15 +74,6 @@ func (check *checker) assocMethod(tname string, meth *Func) {
 	m[tname] = append(m[tname], meth)
 }
 
-func (check *checker) markAsConversion(e *ast.CallExpr) {
-	m := check.conversions
-	if m == nil {
-		m = make(map[*ast.CallExpr]bool)
-		check.conversions = m
-	}
-	m[e] = true
-}
-
 func (check *checker) rememberUntyped(e ast.Expr, lhs bool, typ *Basic, val exact.Value) {
 	m := check.untyped
 	if m == nil {

go/types/expr.go

@@ -108,6 +108,7 @@ func (check *checker) unary(x *operand, op token.Token) {
 		}
 		x.mode = commaok
 		x.typ = typ.elem
+		check.hasCallOrRecv = true
 		return
 	}
 

go/types/testdata/builtins.src

@@ -112,6 +112,31 @@ func cap2() {
 	_ = cap(f2()) // ERROR too many arguments
 }
 
+// test cases for issue 7387
+func cap3() {
+	var f = func() int { return 0 }
+	var x = f()
+	const (
+		_ = cap([4]int{})
+		_ = cap([4]int{x})
+		_ = cap /* ERROR not constant */ ([4]int{f()})
+		_ = cap /* ERROR not constant */ ([4]int{cap([]int{})})
+		_ = cap /* ERROR not constant */ ([4]int{cap([4]int{})})
+	)
+	var y float64
+	var z complex128
+	const (
+		_ = cap([4]float64{})
+		_ = cap([4]float64{y})
+		_ = cap /* ERROR not constant */ ([4]float64{real(z)})
+	)
+	var ch chan [10]int
+	const (
+		_ = cap /* ERROR not constant */ (<-ch)
+		_ = cap /* ERROR not constant */ ([4]int{(<-ch)[0]})
+	)
+}
+
 func close1() {
 	var c chan int
 	var r <-chan int
@@ -356,6 +381,31 @@ func len2() {
 	_ = len(f2()) // ERROR too many arguments
 }
 
+// test cases for issue 7387
+func len3() {
+	var f = func() int { return 0 }
+	var x = f()
+	const (
+		_ = len([4]int{})
+		_ = len([4]int{x})
+		_ = len /* ERROR not constant */ ([4]int{f()})
+		_ = len /* ERROR not constant */ ([4]int{len([]int{})})
+		_ = len /* ERROR not constant */ ([4]int{len([4]int{})})
+	)
+	var y float64
+	var z complex128
+	const (
+		_ = len([4]float64{})
+		_ = len([4]float64{y})
+		_ = len /* ERROR not constant */ ([4]float64{real(z)})
+	)
+	var ch chan [10]int
+	const (
+		_ = len /* ERROR not constant */ (<-ch)
+		_ = len /* ERROR not constant */ ([4]int{(<-ch)[0]})
+	)
+}
+
 func make1() {
 	var n int
 	var m float32