go/types: fix type-checking of shift expressions

Completely rethought shift expression type checking.
Instead of attempting to type-check them eagerly, now
delay the checking of untyped constant lhs in non-
constant shifts until the final expression type
becomes clear. Once it is clear, update the respective
expression tree with the final (not untyped) type and
check respective shift lhs' where necessary.

This also cleans up another conundrum: How to report
the type of untyped constants as it changes from
untyped to typed. Now, Context.Expr is only called
for an expresion x once x has received its final
(not untyped) type (for constant initializers, the
final type may still be untyped).

With this CL all remaining std lib packages that
did not typecheck due to shift errors pass now.

TODO: There's a lot of residual stuff that needs
to be cleaned up but with this CL all tests pass
now.

R=adonovan, axwalk
CC=golang-dev
https://golang.org/cl/7381052

src/pkg/exp/gotype/gotype_test.go

@@ -16,6 +16,7 @@ func runTest(t *testing.T, path string) {
 	errorCount = 0
 
 	*recursive = false
+	*allErrors = true
 	if suffix := ".go"; strings.HasSuffix(path, suffix) {
 		// single file
 		path = filepath.Join(runtime.GOROOT(), "src/pkg", path)
@@ -64,7 +65,7 @@ var tests = []string{
 	"compress/bzip2",
 	"compress/flate",
 	"compress/gzip",
-	// "compress/lzw",
+	"compress/lzw",
 	"compress/zlib",
 
 	"container/heap",
@@ -94,14 +95,14 @@ var tests = []string{
 	"database/sql",
 	"database/sql/driver",
 
-	// "debug/dwarf",
+	"debug/dwarf",
 	"debug/elf",
 	"debug/gosym",
 	"debug/macho",
 	"debug/pe",
 
 	"encoding/ascii85",
-	// "encoding/asn1",
+	"encoding/asn1",
 	"encoding/base32",
 	"encoding/base64",
 	"encoding/binary",
@@ -150,14 +151,14 @@ var tests = []string{
 	"log/syslog",
 
 	"math",
-	//"math/big",
+	"math/big",
 	"math/cmplx",
 	"math/rand",
 
 	"mime",
 	"mime/multipart",
 
-	// "net",
+	"net",
 	"net/http",
 	"net/http/cgi",
 	"net/http/fcgi",
@@ -179,25 +180,25 @@ var tests = []string{
 	"regexp",
 	"regexp/syntax",
 
-	// "runtime",
+	"runtime",
 	"runtime/cgo",
 	"runtime/debug",
 	"runtime/pprof",
 
 	"sort",
-	// "strconv",
+	"strconv",
 	"strings",
 
 	"sync",
 	"sync/atomic",
 
-	// "syscall",
+	"syscall",
 
 	"testing",
 	"testing/iotest",
 	"testing/quick",
 
-	// "text/scanner",
+	"text/scanner",
 	"text/tabwriter",
 	"text/template",
 	"text/template/parse",

src/pkg/go/types/api.go

@@ -33,9 +33,13 @@ type Context struct {
 	// Objects - than we could lift this restriction.
 	Ident func(id *ast.Ident, obj Object)
 
-	// If Expr != nil, it is called 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.
+	// 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.
+	//
+	// 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
+	// static type (possibly an interface type).
 	//
 	// Constants are represented as follows:
 	//

src/pkg/go/types/builtins.go

@@ -338,7 +338,7 @@ func (check *checker) builtin(x *operand, call *ast.CallExpr, bin *builtin, iota
 			check.invalidArg(x.pos(), "%s has no single field %s", x, sel)
 			goto Error
 		}
-		offs := check.ctxt.offsetof(x.typ, res.index)
+		offs := check.ctxt.offsetof(deref(x.typ), res.index)
 		if offs < 0 {
 			check.invalidArg(x.pos(), "field %s is embedded via a pointer in %s", sel, x)
 			goto Error

src/pkg/go/types/check.go

@@ -12,8 +12,11 @@ import (
 	"go/token"
 )
 
-// enable for debugging
-const trace = false
+// debugging support
+const (
+	debug = true  // leave on during development
+	trace = false // turn on for detailed type resolution traces
+)
 
 type checker struct {
 	ctxt  *Context
@@ -28,6 +31,16 @@ type checker struct {
 	initspecs   map[*ast.ValueSpec]*ast.ValueSpec // "inherited" type and initialization expressions for constant declarations
 	methods     map[*TypeName]*Scope              // maps type names to associated methods
 	conversions map[*ast.CallExpr]bool            // set of type-checked conversions (to distinguish from calls)
+
+	// untyped expressions
+	// TODO(gri): Consider merging the untyped and constants map. Should measure
+	// the ratio between untyped non-constant and untyped constant expressions
+	// to make an informed decision.
+	untyped   map[ast.Expr]*Basic      // map of expressions of untyped type
+	constants map[ast.Expr]interface{} // map of untyped constant expressions; each key also appears in untyped
+	shiftOps  map[ast.Expr]bool        // map of lhs shift operands with delayed type-checking
+
+	// 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
@@ -413,6 +426,9 @@ func check(ctxt *Context, fset *token.FileSet, files []*ast.File) (pkg *Package,
 		initspecs:   make(map[*ast.ValueSpec]*ast.ValueSpec),
 		methods:     make(map[*TypeName]*Scope),
 		conversions: make(map[*ast.CallExpr]bool),
+		untyped:     make(map[ast.Expr]*Basic),
+		constants:   make(map[ast.Expr]interface{}),
+		shiftOps:    make(map[ast.Expr]bool),
 	}
 
 	// set results and handle panics
@@ -424,7 +440,6 @@ func check(ctxt *Context, fset *token.FileSet, files []*ast.File) (pkg *Package,
 			err = check.firsterr
 		default:
 			// unexpected panic: don't crash clients
-			const debug = true
 			if debug {
 				check.dump("INTERNAL PANIC: %v", p)
 				panic(p)
@@ -468,5 +483,25 @@ func check(ctxt *Context, fset *token.FileSet, files []*ast.File) (pkg *Package,
 		check.stmtList(f.body.List)
 	}
 
+	// remaining untyped expressions must indeed be untyped
+	if debug {
+		for x, typ := range check.untyped {
+			if !isUntyped(typ) {
+				check.dump("%s: %s (type %s) is not untyped", x.Pos(), x, typ)
+				panic(0)
+			}
+		}
+	}
+
+	// notify client of any untyped types left
+	// TODO(gri) Consider doing this before and
+	// after function body checking for smaller
+	// map size and more immediate feedback.
+	if ctxt.Expr != nil {
+		for x, typ := range check.untyped {
+			ctxt.Expr(x, typ, check.constants[x])
+		}
+	}
+
 	return
 }

src/pkg/go/types/stmt.go

@@ -11,20 +11,23 @@ import (
 	"go/token"
 )
 
-func (check *checker) assignOperand(z, x *operand) {
+// assigment reports whether x can be assigned to a variable of type 'to',
+// if necessary by attempting to convert untyped values to the appropriate
+// type. If x.mode == invalid upon return, then assignment has already
+// issued an error message and the caller doesn't have to report another.
+// TODO(gri) This latter behavior is for historic reasons and complicates
+// callers. Needs to be cleaned up.
+func (check *checker) assignment(x *operand, to Type) bool {
 	if t, ok := x.typ.(*Result); ok {
 		// TODO(gri) elsewhere we use "assignment count mismatch" (consolidate)
 		check.errorf(x.pos(), "%d-valued expression %s used as single value", len(t.Values), x)
 		x.mode = invalid
-		return
+		return false
 	}
 
-	check.convertUntyped(x, z.typ)
+	check.convertUntyped(x, to)
 
-	if !x.isAssignable(check.ctxt, z.typ) {
-		check.errorf(x.pos(), "cannot assign %s to %s", x, z)
-		x.mode = invalid
-	}
+	return x.mode != invalid && x.isAssignable(check.ctxt, to)
 }
 
 // assign1to1 typechecks a single assignment of the form lhs = rhs (if rhs != nil),
@@ -49,6 +52,7 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota
 
 	if !decl {
 		// regular assignment - start with lhs to obtain a type hint
+		// TODO(gri) clean this up - we don't need type hints anymore
 		var z operand
 		check.expr(&z, lhs, nil, -1)
 		if z.mode == invalid {
@@ -66,8 +70,13 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota
 			return
 		}
 
-		check.assignOperand(&z, x)
-		if x.mode != invalid && z.mode == constant {
+		if !check.assignment(x, z.typ) {
+			if x.mode != invalid {
+				check.errorf(x.pos(), "cannot assign %s to %s", x, &z)
+			}
+			return
+		}
+		if z.mode == constant {
 			check.errorf(x.pos(), "cannot assign %s to %s", x, &z)
 		}
 		return
@@ -118,18 +127,19 @@ func (check *checker) assign1to1(lhs, rhs ast.Expr, x *operand, decl bool, iota
 	}
 
 	if x.mode != invalid {
-		var z operand
+		if !check.assignment(x, typ) {
+			if x.mode != invalid {
 				switch obj.(type) {
 				case *Const:
-			z.mode = constant
+					check.errorf(x.pos(), "cannot assign %s to variable of type %s", x, typ)
 				case *Var:
-			z.mode = variable
+					check.errorf(x.pos(), "cannot initialize constant of type %s with %s", typ, x)
 				default:
 					unreachable()
 				}
-		z.expr = ident
-		z.typ = typ
-		check.assignOperand(&z, x)
+				x.mode = invalid
+			}
+		}
 	}
 
 	// for constants, set their value
@@ -345,9 +355,11 @@ func (check *checker) stmt(s ast.Stmt) {
 		if ch.mode == invalid || x.mode == invalid {
 			return
 		}
-		if tch, ok := underlying(ch.typ).(*Chan); !ok || tch.Dir&ast.SEND == 0 || !x.isAssignable(check.ctxt, tch.Elt) {
+		if tch, ok := underlying(ch.typ).(*Chan); !ok || tch.Dir&ast.SEND == 0 || !check.assignment(&x, tch.Elt) {
+			if x.mode != invalid {
 				check.invalidOp(ch.pos(), "cannot send %s to channel %s", &x, &ch)
 			}
+		}
 
 	case *ast.IncDecStmt:
 		var op token.Token
@@ -360,10 +372,12 @@ func (check *checker) stmt(s ast.Stmt) {
 			check.invalidAST(s.TokPos, "unknown inc/dec operation %s", s.Tok)
 			return
 		}
-		var x, y operand
-		check.expr(&x, s.X, nil, -1)
-		check.expr(&y, &ast.BasicLit{ValuePos: x.pos(), Kind: token.INT, Value: "1"}, nil, -1) // use x's position
-		check.binary(&x, &y, op, nil)
+		var x operand
+		Y := &ast.BasicLit{ValuePos: s.X.Pos(), Kind: token.INT, Value: "1"} // use x's position
+		check.binary(&x, s.X, Y, op, -1)
+		if x.mode == invalid {
+			return
+		}
 		check.assign1to1(s.X, nil, &x, false, -1)
 
 	case *ast.AssignStmt:
@@ -409,18 +423,11 @@ func (check *checker) stmt(s ast.Stmt) {
 				check.invalidAST(s.TokPos, "unknown assignment operation %s", s.Tok)
 				return
 			}
-			var x, y operand
-			// The lhs operand's type doesn't need a hint (from the rhs operand),
-			// because it must be a fully typed variable in this case.
-			check.expr(&x, s.Lhs[0], nil, -1)
+			var x operand
+			check.binary(&x, s.Lhs[0], s.Rhs[0], op, -1)
 			if x.mode == invalid {
 				return
 			}
-			check.expr(&y, s.Rhs[0], x.typ, -1)
-			if y.mode == invalid {
-				return
-			}
-			check.binary(&x, &y, op, x.typ)
 			check.assign1to1(s.Lhs[0], nil, &x, false, -1)
 		}
 
@@ -464,7 +471,7 @@ func (check *checker) stmt(s ast.Stmt) {
 		check.optionalStmt(s.Init)
 		var x operand
 		check.expr(&x, s.Cond, nil, -1)
-		if !isBoolean(x.typ) {
+		if x.mode != invalid && !isBoolean(x.typ) {
 			check.errorf(s.Cond.Pos(), "non-boolean condition in if statement")
 		}
 		check.stmt(s.Body)
@@ -641,7 +648,7 @@ func (check *checker) stmt(s ast.Stmt) {
 		if s.Cond != nil {
 			var x operand
 			check.expr(&x, s.Cond, nil, -1)
-			if !isBoolean(x.typ) {
+			if x.mode != invalid && !isBoolean(x.typ) {
 				check.errorf(s.Cond.Pos(), "non-boolean condition in for statement")
 			}
 		}

src/pkg/go/types/testdata/expr3.src

@@ -28,22 +28,103 @@ func shifts1() {
 }
 
 func shifts2() {
-	// TODO(gri) enable commented out tests below.
+	// from the spec
 	var (
 		s uint = 33
 		i = 1<<s           // 1 has type int
 		j int32 = 1<<s     // 1 has type int32; j == 0
 		k = uint64(1<<s)   // 1 has type uint64; k == 1<<33
 		m int = 1.0<<s     // 1.0 has type int
-	//	n = 1.0<<s != 0    // 1.0 has type int; n == false if ints are 32bits in size
+		n = 1.0<<s != 0    // 1.0 has type int; n == false if ints are 32bits in size
 		o = 1<<s == 2<<s   // 1 and 2 have type int; o == true if ints are 32bits in size
-	//	p = 1<<s == 1 /* ERROR "overflows" */ <<33  // illegal if ints are 32bits in size: 1 has type int, but 1<<33 overflows int
+		p = 1<<s == 1<<33  // illegal if ints are 32bits in size: 1 has type int, but 1<<33 overflows int
 		u = 1.0 /* ERROR "must be integer" */ <<s         // illegal: 1.0 has type float64, cannot shift
 		v float32 = 1 /* ERROR "must be integer" */ <<s   // illegal: 1 has type float32, cannot shift
 		w int64 = 1.0<<33  // 1.0<<33 is a constant shift expression
 	)
 }
 
+func shifts3(a int16, b float32) {
+	var (
+		s uint = 11
+		u = 1 /* ERROR "must be integer" */ <<s + 1.0
+		v complex128 = 1 /* ERROR "must be integer" */ << s + 1.0 /* ERROR "must be integer" */ << s + 1
+	)
+	x := 1.0 /* ERROR "must be integer" */ <<s + 1
+	shifts3(1.0 << s, 1 /* ERROR "must be integer" */ >> s)
+	// TODO(gri) add more tests (systematically)
+}
+
+func shifts4() {
+	// from src/pkg/compress/lzw/reader.go:90
+	{
+		var d struct {
+			bits     uint32
+			width    uint
+		}
+		_ = uint16(d.bits & (1<<d.width - 1))
+	}
+
+	// from src/pkg/debug/dwarf/buf.go:116
+	{
+		var ux uint64
+		var bits uint
+		x := int64(ux)
+		if x&(1<<(bits-1)) != 0 {}
+	}
+
+	// from src/pkg/encoding/asn1/asn1.go:160
+	{
+		var bytes []byte
+		if bytes[len(bytes)-1]&((1<<bytes[0])-1) != 0 {}
+	}
+
+	// from src/pkg/math/big/rat.go:140
+	{
+		var exp int
+		var mantissa uint64
+		shift := uint64(-1022 - (exp - 1)) // [1..53)
+		_ = mantissa & (1<<shift - 1)
+	}
+
+	// from src/pkg/net/interface.go:51
+	{
+		type Flags uint
+		var f Flags
+		var i int
+		if f&(1<<uint(i)) != 0 {}
+	}
+
+	// from src/pkg/runtime/softfloat64.go:234
+	{
+		var gm uint64
+		var shift uint
+		_ = gm & (1<<shift - 1)
+	}
+
+	// from src/pkg/strconv/atof.go:326
+	{
+		var mant uint64
+		var mantbits uint
+		if mant == 2<<mantbits {}
+	}
+
+	// from src/pkg/syscall/route_bsd.go:82
+	{
+		var Addrs int32
+		const rtaRtMask = 1
+		var i uint
+		if Addrs&rtaRtMask&(1<<i) == 0 {}
+	}
+
+	// from src/pkg/text/scanner/scanner.go:540
+	{
+		var s struct { Whitespace uint64 }
+		var ch rune
+		for s.Whitespace&(1<<uint(ch)) != 0 {}
+	}
+}
+
 // TODO(gri) The error messages below depond on adjusting the spec
 //           to reflect what gc is doing at the moment (the spec
 //           asks for run-time errors at the moment - see issue 4231).

src/pkg/go/types/testdata/stmt0.src

@@ -36,7 +36,7 @@ func _() {
 	undeclared /* ERROR "undeclared" */ = 991
 }
 
-func _incdecs() {
+func incdecs() {
 	const c = 3.14
 	c /* ERROR "cannot assign" */ ++
 	s := "foo"
@@ -52,17 +52,17 @@ func _incdecs() {
 	z++
 }
 
-func _sends() {
+func sends() {
 	var ch chan int
 	var rch <-chan int
 	var x int
 	x /* ERROR "cannot send" */ <- x
 	rch /* ERROR "cannot send" */ <- x
-	ch /* ERROR "cannot send" */ <- "foo"
+	ch <- "foo" /* ERROR "cannot convert" */
 	ch <- x
 }
 
-func _selects() {
+func selects() {
 	select {}
 	var (
 		ch chan int
@@ -82,23 +82,23 @@ func _selects() {
 	}
 }
 
-func _gos() {
+func gos() {
 	go 1 /* ERROR "expected function/method call" */
-	go _gos()
+	go gos()
 	var c chan int
 	go close(c)
 	go len(c) // TODO(gri) this should not be legal
 }
 
-func _defers() {
+func defers() {
 	defer 1 /* ERROR "expected function/method call" */
-	defer _defers()
+	defer defers()
 	var c chan int
 	defer close(c)
 	defer len(c) // TODO(gri) this should not be legal
 }
 
-func _switches() {
+func switches() {
 	var x int
 
 	switch x {
@@ -148,7 +148,7 @@ type T2 struct{}
 func (T) m() {}
 func (T2) m(int) {}
 
-func _typeswitches() {
+func typeswitches() {
 	var i int
 	var x interface{}
 
@@ -189,7 +189,16 @@ func _typeswitches() {
 	}
 }
 
-func _rangeloops() {
+func typeswitch0() {
+	switch y := interface{}(nil).(type) {
+	case int:
+		// TODO(gri) y has the wrong type here (type-checking
+		// of captured variable is delayed)
+		// func() int { return y + 0 }()
+	}
+}
+
+func rangeloops() {
 	var (
 		x int
 		a [10]float32