exp/types: fixed field/method lookup

also:

- composite literal checking close to complete
- cleaned up parameter, method, field checking
- don't let panics escape type checker
- more TODOs eliminated

R=rsc
CC=golang-dev
https://golang.org/cl/6816083

src/pkg/exp/types/check.go

@@ -36,7 +36,7 @@ type checker struct {
 //
 // TODO(gri) This is very similar to the declare function in go/parser; it
 // is only used to associate methods with their respective receiver base types.
-// In a future version, it might be simpler and cleaner do to all the resolution
+// In a future version, it might be simpler and cleaner to do all the resolution
 // in the type-checking phase. It would simplify the parser, AST, and also
 // reduce some amount of code duplication.
 //
@@ -188,11 +188,7 @@ func (check *checker) object(obj *ast.Object, cycleOk bool) {
 
 	case ast.Fun:
 		fdecl := obj.Decl.(*ast.FuncDecl)
-		if fdecl.Recv != nil {
-			// This will ensure that the method base type is
-			// type-checked
-			check.collectFields(token.FUNC, fdecl.Recv, true)
-		}
+		check.collectParams(fdecl.Recv) // ensure method base is type-checked
 		ftyp := check.typ(fdecl.Type, cycleOk).(*Signature)
 		obj.Type = ftyp
 		check.function(ftyp, fdecl.Body)
@@ -355,12 +351,19 @@ func check(fset *token.FileSet, pkg *ast.Package, errh func(token.Pos, string),
 	check.mapf = f
 	check.initexprs = make(map[*ast.ValueSpec][]ast.Expr)
 
-	// handle bailouts
+	// handle panics
 	defer func() {
-		if p := recover(); p != nil {
-			_ = p.(bailout) // re-panic if not a bailout
+		switch p := recover().(type) {
+		case nil:
+			// normal return - nothing to do
+		case bailout:
+			// early exit
+			err = check.firsterr
+		default:
+			// unexpected panic: don't crash clients
+			// panic(p) // enable for debugging
+			err = fmt.Errorf("types.check internal error: %v", p)
 		}
-		err = check.firsterr
 	}()
 
 	// determine missing constant initialization expressions

src/pkg/exp/types/check_test.go

@@ -48,6 +48,7 @@ var tests = []struct {
 	{"decls0", []string{"testdata/decls0.src"}},
 	{"decls1", []string{"testdata/decls1.src"}},
 	{"decls2", []string{"testdata/decls2a.src", "testdata/decls2b.src"}},
+	{"decls3", []string{"testdata/decls3.src"}},
 	{"const0", []string{"testdata/const0.src"}},
 	{"expr0", []string{"testdata/expr0.src"}},
 	{"expr1", []string{"testdata/expr1.src"}},

src/pkg/exp/types/operand.go

@@ -125,7 +125,16 @@ func (x *operand) implements(T *Interface) bool {
 		return true // avoid spurious errors
 	}
 
-	unimplemented()
+	// x implements T if it implements all methods of T.
+	// TODO(gri): distinguish pointer and non-pointer receivers
+	for _, m := range T.Methods {
+		mode, typ := lookupField(x.typ, m.Name)
+		if mode == invalid || !isIdentical(typ, m.Type.(Type)) {
+			// TODO(gri) should report which method is missing
+			return false
+		}
+	}
+
 	return true
 }
 
@@ -134,6 +143,10 @@ func (x *operand) isNil() bool {
 	return x.mode == constant && x.val == nilConst
 }
 
+// TODO(gri) The functions operand.isAssignable, checker.convertUntyped,
+//           checker.isRepresentable, and checker.assignOperand are
+//           overlapping in functionality. Need to simplify and clean up.
+
 // isAssignable reports whether x is assignable to a variable of type T.
 func (x *operand) isAssignable(T Type) bool {
 	if x.mode == invalid || T == Typ[Invalid] {
@@ -181,8 +194,18 @@ func (x *operand) isAssignable(T Type) bool {
 	}
 
 	// x is an untyped constant representable by a value of type T
-	// - this is taken care of in the assignment check
-	// TODO(gri) double-check - isAssignable is used elsewhere
+	// TODO(gri) This is borrowing from checker.convertUntyped and
+	//           checker.isRepresentable. Need to clean up.
+	if isUntyped(Vu) {
+		switch t := Tu.(type) {
+		case *Basic:
+			return x.mode == constant && isRepresentableConst(x.val, t.Kind)
+		case *Interface:
+			return x.isNil() || len(t.Methods) == 0
+		case *Pointer, *Signature, *Slice, *Map, *Chan:
+			return x.isNil()
+		}
+	}
 
 	return false
 }
@@ -199,35 +222,50 @@ type lookupResult struct {
 	typ  Type
 }
 
-// lookupFieldRecursive is similar to FieldByNameFunc in reflect/type.go
-// TODO(gri): FieldByNameFunc seems more complex - what are we missing?
-func lookupFieldRecursive(list []*NamedType, name string) (res lookupResult) {
-	// visited records the types that have been searched already
-	visited := make(map[Type]bool)
+type embeddedType struct {
+	typ       *NamedType
+	multiples bool // if set, typ is embedded multiple times at the same level
+}
+
+// lookupFieldBreadthFirst searches all types in list for a single entry (field
+// or method) of the given name. If such a field is found, the result describes
+// the field mode and type; otherwise the result mode is invalid.
+// (This function is similar in structure to FieldByNameFunc in reflect/type.go)
+//
+func lookupFieldBreadthFirst(list []embeddedType, name string) (res lookupResult) {
+	// visited records the types that have been searched already.
+	visited := make(map[*NamedType]bool)
 
 	// embedded types of the next lower level
-	var next []*NamedType
+	var next []embeddedType
 
-	potentialMatch := func(mode operandMode, typ Type) bool {
-		if res.mode != invalid {
-			// name appeared multiple times at this level - annihilate
+	// potentialMatch is invoked every time a match is found.
+	potentialMatch := func(multiples bool, mode operandMode, typ Type) bool {
+		if multiples || res.mode != invalid {
+			// name appeared already at this level - annihilate
 			res.mode = invalid
 			return false
 		}
+		// first appearance of name
 		res.mode = mode
 		res.typ = typ
 		return true
 	}
 
-	// look for name in all types of this level
+	// Search the current level if there is any work to do and collect
+	// embedded types of the next lower level in the next list.
 	for len(list) > 0 {
+		// The res.mode indicates whether we have found a match already
+		// on this level (mode != invalid), or not (mode == invalid).
 		assert(res.mode == invalid)
-		for _, typ := range list {
+
+		// start with empty next list (don't waste underlying array)
+		next = next[:0]
+
+		// look for name in all types at this level
+		for _, e := range list {
+			typ := e.typ
 			if visited[typ] {
-				// We have seen this type before, at a higher level.
-				// That higher level shadows the lower level we are
-				// at now, and either we would have found or not
-				// found the field before. Ignore this type now.
 				continue
 			}
 			visited[typ] = true
@@ -236,7 +274,7 @@ func lookupFieldRecursive(list []*NamedType, name string) (res lookupResult) {
 			if data := typ.Obj.Data; data != nil {
 				if obj := data.(*ast.Scope).Lookup(name); obj != nil {
 					assert(obj.Type != nil)
-					if !potentialMatch(value, obj.Type.(Type)) {
+					if !potentialMatch(e.multiples, value, obj.Type.(Type)) {
 						return // name collision
 					}
 				}
@@ -244,21 +282,26 @@ func lookupFieldRecursive(list []*NamedType, name string) (res lookupResult) {
 
 			switch typ := underlying(typ).(type) {
 			case *Struct:
-				// look for a matching fieldm and collect embedded types
+				// look for a matching field and collect embedded types
 				for _, f := range typ.Fields {
 					if f.Name == name {
 						assert(f.Type != nil)
-						if !potentialMatch(variable, f.Type) {
+						if !potentialMatch(e.multiples, variable, f.Type) {
 							return // name collision
 						}
 						continue
 					}
 					// Collect embedded struct fields for searching the next
-					// lower level, but only if we have not seen a match yet.
+					// lower level, but only if we have not seen a match yet
+					// (if we have a match it is either the desired field or
+					// we have a name collision on the same level; in either
+					// case we don't need to look further).
 					// Embedded fields are always of the form T or *T where
-					// T is a named type.
+					// T is a named type. If typ appeared multiple times at
+					// this level, f.Type appears multiple times at the next
+					// level.
 					if f.IsAnonymous && res.mode == invalid {
-						next = append(next, deref(f.Type).(*NamedType))
+						next = append(next, embeddedType{deref(f.Type).(*NamedType), e.multiples})
 					}
 				}
 
@@ -267,7 +310,7 @@ func lookupFieldRecursive(list []*NamedType, name string) (res lookupResult) {
 				for _, obj := range typ.Methods {
 					if obj.Name == name {
 						assert(obj.Type != nil)
-						if !potentialMatch(value, obj.Type.(Type)) {
+						if !potentialMatch(e.multiples, value, obj.Type.(Type)) {
 							return // name collision
 						}
 					}
@@ -276,17 +319,41 @@ func lookupFieldRecursive(list []*NamedType, name string) (res lookupResult) {
 		}
 
 		if res.mode != invalid {
-			// we found a match on this level
+			// we found a single match on this level
 			return
 		}
 
-		// search the next level
-		list = append(list[:0], next...) // don't waste underlying arrays
-		next = next[:0]
+		// No match and no collision so far.
+		// Compute the list to search for the next level.
+		list = list[:0] // don't waste underlying array
+		for _, e := range next {
+			// Instead of adding the same type multiple times, look for
+			// it in the list and mark it as multiple if it was added
+			// before.
+			// We use a sequential search (instead of a map for next)
+			// because the lists tend to be small, can easily be reused,
+			// and explicit search appears to be faster in this case.
+			if alt := findType(list, e.typ); alt != nil {
+				alt.multiples = true
+			} else {
+				list = append(list, e)
+			}
+		}
+
 	}
+
 	return
 }
 
+func findType(list []embeddedType, typ *NamedType) *embeddedType {
+	for i := range list {
+		if p := &list[i]; p.typ == typ {
+			return p
+		}
+	}
+	return nil
+}
+
 func lookupField(typ Type, name string) (operandMode, Type) {
 	typ = deref(typ)
 
@@ -301,17 +368,20 @@ func lookupField(typ Type, name string) (operandMode, Type) {
 
 	switch typ := underlying(typ).(type) {
 	case *Struct:
-		var list []*NamedType
+		var next []embeddedType
 		for _, f := range typ.Fields {
 			if f.Name == name {
 				return variable, f.Type
 			}
 			if f.IsAnonymous {
-				list = append(list, deref(f.Type).(*NamedType))
+				// Possible optimization: If the embedded type
+				// is a pointer to the current type we could
+				// ignore it.
+				next = append(next, embeddedType{typ: deref(f.Type).(*NamedType)})
 			}
 		}
-		if len(list) > 0 {
-			res := lookupFieldRecursive(list, name)
+		if len(next) > 0 {
+			res := lookupFieldBreadthFirst(next, name)
 			return res.mode, res.typ
 		}
 

src/pkg/exp/types/testdata/decls0.src

@@ -41,7 +41,7 @@ type (
 
 
 type (
-	p1 pi /* ERROR "no field or method foo" */ .foo
+	p1 pi /* ERROR "no single field or method foo" */ .foo
 	p2 unsafe.Pointer
 )
 
@@ -131,7 +131,7 @@ type (
 		m1(I5)
 	}
 	I6 interface {
-		S0 /* ERROR "non-interface" */
+		S0 /* ERROR "not an interface" */
 	}
 	I7 interface {
 		I1

src/pkg/exp/types/testdata/decls3.src

+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// embedded types
+
+package decls3
+
+// fields with the same name at the same level cancel each other out
+
+func _() {
+	type (
+		T1 struct { X int }
+		T2 struct { X int }
+		T3 struct { T1; T2 } // X is embedded twice at the same level via T1->X, T2->X
+	)
+
+	var t T3
+	_ = t /* ERROR "no single field or method" */ .X
+}
+
+func _() {
+	type (
+		T1 struct { X int }
+		T2 struct { T1 }
+		T3 struct { T1 }
+		T4 struct { T2; T3 } // X is embedded twice at the same level via T2->T1->X, T3->T1->X
+	)
+
+	var t T4
+	_ = t /* ERROR "no single field or method" */ .X
+}
+
+func issue4355() {
+	type (
+	    T1 struct {X int}
+	    T2 struct {T1}
+	    T3 struct {T2}
+	    T4 struct {T2}
+	    T5 struct {T3; T4} // X is embedded twice at the same level via T3->T2->T1->X, T4->T2->T1->X
+	)	
+
+	var t T5
+	_ = t /* ERROR "no single field or method" */ .X
+}
+
+// Borrowed from the FieldByName test cases in reflect/all_test.go.
+
+type D1 struct {
+	d int
+}
+type D2 struct {
+	d int
+}
+
+type S0 struct {
+	A, B, C int
+	D1
+	D2
+}
+
+type S1 struct {
+	B int
+	S0
+}
+
+type S2 struct {
+	A int
+	*S1
+}
+
+type S1x struct {
+	S1
+}
+
+type S1y struct {
+	S1
+}
+
+type S3 struct {
+	S1x
+	S2
+	D, E int
+	*S1y
+}
+
+type S4 struct {
+	*S4
+	A int
+}
+
+// The X in S6 and S7 annihilate, but they also block the X in S8.S9.
+type S5 struct {
+	S6
+	S7
+	S8
+}
+
+type S6 struct {
+	X int
+}
+
+type S7 S6
+
+type S8 struct {
+	S9
+}
+
+type S9 struct {
+	X int
+	Y int
+}
+
+// The X in S11.S6 and S12.S6 annihilate, but they also block the X in S13.S8.S9.
+type S10 struct {
+	S11
+	S12
+	S13
+}
+
+type S11 struct {
+	S6
+}
+
+type S12 struct {
+	S6
+}
+
+type S13 struct {
+	S8
+}
+
+func _() {
+	_ = struct /* ERROR "no single field or method" */ {}{}.Foo
+	_ = S0{}.A
+	_ = S0 /* ERROR "no single field or method" */ {}.D
+	_ = S1{}.A
+	_ = S1{}.B
+	_ = S1{}.S0
+	_ = S1{}.C
+	_ = S2{}.A
+	_ = S2{}.S1
+	_ = S2{}.B
+	_ = S2{}.C
+	_ = S2 /* ERROR "no single field or method" */ {}.D
+	_ = S3 /* ERROR "no single field or method" */ {}.S1
+	_ = S3{}.A
+	_ = S3 /* ERROR "no single field or method" */ {}.B
+	_ = S3{}.D
+	_ = S3{}.E
+	_ = S4{}.A
+	_ = S4 /* ERROR "no single field or method" */ {}.B
+	_ = S5 /* ERROR "no single field or method" */ {}.X
+	_ = S5{}.Y
+	_ = S10 /* ERROR "no single field or method" */ {}.X
+	_ = S10{}.Y
+}
+
+// Borrowed from the FieldByName benchmark in reflect/all_test.go.
+
+type R0 struct {
+	*R1
+	*R2
+	*R3
+	*R4
+}
+
+type R1 struct {
+	*R5
+	*R6
+	*R7
+	*R8
+}
+
+type R2 R1
+type R3 R1
+type R4 R1
+
+type R5 struct {
+	*R9
+	*R10
+	*R11
+	*R12
+}
+
+type R6 R5
+type R7 R5
+type R8 R5
+
+type R9 struct {
+	*R13
+	*R14
+	*R15
+	*R16
+}
+
+type R10 R9
+type R11 R9
+type R12 R9
+
+type R13 struct {
+	*R17
+	*R18
+	*R19
+	*R20
+}
+
+type R14 R13
+type R15 R13
+type R16 R13
+
+type R17 struct {
+	*R21
+	*R22
+	*R23
+	*R24
+}
+
+type R18 R17
+type R19 R17
+type R20 R17
+
+type R21 struct {
+	X int
+}
+
+type R22 R21
+type R23 R21
+type R24 R21
+
+var _ = R0 /* ERROR "no single field or method" */ {}.X
\ No newline at end of file

src/pkg/exp/types/testdata/expr3.src

@@ -126,9 +126,59 @@ type T struct {
 func (*T) m() {}
 
 func method_expressions() {
-	_ = T /* ERROR "no field or method" */ .a
+	_ = T /* ERROR "no single field or method" */ .a
 	_ = T /* ERROR "has no method" */ .x
 	_ = T.m
 	var f func(*T) = (*T).m
 	var g func(*T) = ( /* ERROR "cannot assign" */ T).m
+}
+
+func struct_literals() {
+	type T0 struct {
+		a, b, c int
+	}
+
+	type T1 struct {
+		T0
+		a, b int
+		u float64
+		s string
+	}
+
+	// keyed elements
+	_ = T1{}
+	_ = T1{a: 0, 1 /* ERROR "mixture of .* elements" */ }
+	_ = T1{aa /* ERROR "unknown field" */ : 0}
+	_ = T1{1 /* ERROR "invalid field name" */ : 0}
+	_ = T1{a: 0, s: "foo", u: 0, a /* ERROR "duplicate field" */: 10}
+	_ = T1{a: "foo" /* ERROR "cannot use" */ }
+	_ = T1{c /* ERROR "unknown field" */ : 0}
+	_ = T1{T0: { /* ERROR "missing type" */ }}
+	_ = T1{T0: T0{}}
+	_ = T1{T0 /* ERROR "invalid field name" */ .a: 0}
+
+	// unkeyed elements
+	_ = T0{1, 2, 3}
+	_ = T0{1, b /* ERROR "mixture" */ : 2, 3}
+	_ = T0{1, 2} /* ERROR "too few values" */
+	_ = T0{1, 2, 3, 4  /* ERROR "too many values" */ }
+	_ = T0{1, "foo" /* ERROR "cannot use" */, 3.4  /* ERROR "cannot use" */}
+}
+
+func array_literals() {
+	// TODO(gri)
+}
+
+func slice_literals() {
+	// TODO(gri)
+}
+
+func map_literals() {
+	type M0 map[string]int
+
+	_ = M0{}
+	_ = M0{1 /* ERROR "missing key" */ }
+	_ = M0{1 /* ERROR "cannot use .* as string key" */ : 2}
+	_ = M0{"foo": "bar" /* ERROR "cannot use .* as int value" */ }
+	_ = M0{"foo": 1, "bar": 2, "foo" /* ERROR "duplicate key" */ : 3 }
 }
\ No newline at end of file

src/pkg/exp/types/types.go

@@ -126,6 +126,15 @@ type Struct struct {
 	Fields []*StructField
 }
 
+func (typ *Struct) fieldIndex(name string) int {
+	for i, f := range typ.Fields {
+		if f.Name == name {
+			return i
+		}
+	}
+	return -1
+}
+
 // A Pointer represents a pointer type *Base.
 type Pointer struct {
 	implementsType