gc: Clean up dostruct/stotype, detect broken fields and propagate up to structs and functions to supress spurious errors.

Fixes #1556.

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

src/cmd/gc/dcl.c

@@ -709,181 +709,264 @@ ok:
  * they don't belong here, but where do they belong?
  */
 
-
-/*
- * turn a parsed struct into a type
- */
-static Type**
-stotype(NodeList *l, int et, Type **t, int funarg)
+static void
+checkembeddedtype(Type *t)
 {
-	Type *f, *t1, *t2, **t0;
-	Strlit *note;
+	if (t == T)
+		return;
+
+	if(t->sym == S && isptr[t->etype]) {
+		t = t->type;
+		if(t->etype == TINTER)
+			yyerror("embedded type cannot be a pointer to interface");
+	}
+	if(isptr[t->etype])
+		yyerror("embedded type cannot be a pointer");
+	else if(t->etype == TFORW && t->embedlineno == 0)
+		t->embedlineno = lineno;
+}
+
+static Type*
+structfield(Node *n)
+{
+	Type *f;
 	int lno;
-	Node *n, *left;
-	char *what;
 
-	t0 = t;
 	lno = lineno;
-	what = "field";
-	if(et == TINTER)
-		what = "method";
-
-	for(; l; l=l->next) {
-		n = l->n;
 	lineno = n->lineno;
 
 	if(n->op != ODCLFIELD)
-			fatal("stotype: oops %N\n", n);
-		left = n->left;
-		if(funarg && isblank(left))
-			left = N;
+		fatal("structfield: oops %N\n", n);
+
+	f = typ(TFIELD);
+	f->isddd = n->isddd;
+
 	if(n->right != N) {
-			if(et == TINTER && left != N) {
+		typecheck(&n->right, Etype);
+		n->type = n->right->type;
+		if(n->left != N)
+			n->left->type = n->type;
+		if(n->embedded)
+			checkembeddedtype(n->type);
+	}
+	n->right = N;
+		
+	f->type = n->type;
+	if(f->type == T)
+		f->broke = 1;
+
+	switch(n->val.ctype) {
+	case CTSTR:
+		f->note = n->val.u.sval;
+		break;
+	default:
+		yyerror("field annotation must be string");
+		// fallthrough
+	case CTxxx:
+		f->note = nil;
+		break;
+	}
+
+	// tofunarg will undo this for _ arguments
+	if(n->left && n->left->op == ONAME) {
+		f->nname = n->left;
+		f->embedded = n->embedded;
+		f->sym = f->nname->sym;
+		if(importpkg && !exportname(f->sym->name))
+			f->sym = pkglookup(f->sym->name, structpkg);
+	}
+
+	lineno = lno;
+	return f;
+}
+
+static void
+checkdupfields(Type *t, char* what)
+{
+	Type* t1;
+	int lno;
+
+	lno = lineno;
+
+	for( ; t; t=t->down)
+		if(t->sym && t->nname && !isblank(t->nname))
+			for(t1=t->down; t1; t1=t1->down)
+				if(t1->sym == t->sym) {
+					lineno = t->nname->lineno;
+					yyerror("duplicate %s %s", what, t->sym->name);
+					break;
+				}
+
+	lineno = lno;
+}
+
+/*
+ * convert a parsed id/type list into
+ * a type for struct/interface/arglist
+ */
+Type*
+tostruct(NodeList *l)
+{
+	Type *t, *f, **tp;
+	t = typ(TSTRUCT);
+
+	for(tp = &t->type; l; l=l->next,tp = &(*tp)->down)
+		*tp = structfield(l->n);
+
+	for(f=t->type; f && !t->broke; f=f->down)
+		if(f->broke)
+			t->broke = 1;
+
+	checkdupfields(t->type, "field");
+
+	if (!t->broke)
+		checkwidth(t);
+
+	return t;
+}
+
+static Type*
+tofunargs(NodeList *l)
+{
+	Type *t, *f, **tp;
+
+	t = typ(TSTRUCT);
+	t->funarg = 1;
+
+	for(tp = &t->type; l; l=l->next) {
+		f = structfield(l->n);
+
+		// Unlink the name for _ arguments.
+		if(l->n->left && l->n->left->op == ONAME && isblank(l->n->left)) {
+			f->nname = nil;
+			f->sym = nil;
+			f->embedded = 0;
+		}
+
+		// esc.c needs to find f given a PPARAM to add the tag.
+		if(l->n->left && l->n->left->class == PPARAM)
+			l->n->left->paramfld = f;
+
+		*tp = f;
+		tp = &f->down;
+	}
+
+	for(f=t->type; f && !t->broke; f=f->down)
+		if(f->broke)
+			t->broke = 1;
+
+	checkdupfields(t->type, "argument");
+	return t;
+}
+
+static Type*
+interfacefield(Node *n)
+{
+	Type *f;
+	int lno;
+
+	lno = lineno;
+	lineno = n->lineno;
+
+	if(n->op != ODCLFIELD)
+		fatal("interfacefield: oops %N\n", n);
+
+	if (n->val.ctype != CTxxx)
+		yyerror("interface method cannot have annotation");
+
+	f = typ(TFIELD);
+	f->isddd = n->isddd;
+	
+	if(n->right != N) {
+		if(n->left != N) {
 			// queue resolution of method type for later.
 			// right now all we need is the name list.
 			// avoids cycles for recursive interface types.
 			n->type = typ(TINTERMETH);
 			n->type->nname = n->right;
-				n->right = N;
-				left->type = n->type;
+			n->left->type = n->type;
 			queuemethod(n);
-			} else {
-				typecheck(&n->right, Etype);
-				n->type = n->right->type;
-				if(n->type == T)
-					continue;
-				if(left != N)
-					left->type = n->type;
-				n->right = N;
-				if(n->embedded && n->type != T) {
-					t1 = n->type;
-					if(t1->sym == S && isptr[t1->etype]) {
-						t1 = t1->type;
-						if(t1->etype == TINTER)
-							yyerror("embedded type cannot be a pointer to interface");
-					}
-					if(isptr[t1->etype])
-						yyerror("embedded type cannot be a pointer");
-					else if(t1->etype == TFORW && t1->embedlineno == 0)
-						t1->embedlineno = lineno;
-				}
-			}
-		}
 
-		if(n->type == T) {
-			// assume error already printed
-			continue;
-		}
-
-		switch(n->val.ctype) {
-		case CTSTR:
-			if(et != TSTRUCT)
-				yyerror("interface method cannot have annotation");
-			note = n->val.u.sval;
-			break;
-		default:
-			if(et != TSTRUCT)
-				yyerror("interface method cannot have annotation");
-			else
-				yyerror("field annotation must be string");
-		case CTxxx:
-			note = nil;
-			break;
-		}
-
-		if(et == TINTER && left == N) {
-			// embedded interface - inline the methods
-			if(n->type->etype != TINTER) {
-				if(n->type->etype == TFORW)
-					yyerror("interface type loop involving %T", n->type);
-				else
-					yyerror("interface contains embedded non-interface %T", n->type);
-				continue;
-			}
-			for(t1=n->type->type; t1!=T; t1=t1->down) {
-				f = typ(TFIELD);
-				f->type = t1->type;
-				f->width = BADWIDTH;
-				f->nname = newname(t1->sym);
-				f->sym = t1->sym;
-				for(t2=*t0; t2!=T; t2=t2->down) {
-					if(t2->sym == f->sym) {
-						yyerror("duplicate method %s", t2->sym->name);
-						break;
-					}
-				}
-				*t = f;
-				t = &f->down;
-			}
-			continue;
-		}
-
-		f = typ(TFIELD);
-		f->type = n->type;
-		f->note = note;
-		f->width = BADWIDTH;
-		f->isddd = n->isddd;
-
-		// esc.c needs to find f given a PPARAM to add the tag.
-		if(funarg && n->left && n->left->class == PPARAM)
-			n->left->paramfld = f;
-
-		if(left != N && left->op == ONAME) {
-			f->nname = left;
+			if(n->left->op == ONAME) {
+				f->nname = n->left;
 				f->embedded = n->embedded;
 				f->sym = f->nname->sym;
 				if(importpkg && !exportname(f->sym->name))
 					f->sym = pkglookup(f->sym->name, structpkg);
-			if(f->sym && !isblank(f->nname)) {
-				for(t1=*t0; t1!=T; t1=t1->down) {
-					if(t1->sym == f->sym) {
-						yyerror("duplicate %s %s", what, t1->sym->name);
+			}
+
+		} else {
+
+			typecheck(&n->right, Etype);
+			n->type = n->right->type;
+
+			if(n->embedded)
+				checkembeddedtype(n->type);
+
+			if(n->type)
+				switch(n->type->etype) {
+				case TINTER:
+					break;
+				case TFORW:
+					yyerror("interface type loop involving %T", n->type);
+					f->broke = 1;
+					break;
+				default:
+					yyerror("interface contains embedded non-interface %T", n->type);
+					f->broke = 1;
 					break;
 				}
 		}
 	}
-		}
 
-		*t = f;
-		t = &f->down;
-	}
+	n->right = N;
+	
+	f->type = n->type;
+	if(f->type == T)
+		f->broke = 1;
 	
-	*t = T;
 	lineno = lno;
-	return t;
+	return f;
 }
 
 Type*
-dostruct(NodeList *l, int et)
+tointerface(NodeList *l)
 {
-	Type *t;
-	int funarg;
+	Type *t, *f, **tp, *t1;
 
-	/*
-	 * convert a parsed id/type list into
-	 * a type for struct/interface/arglist
-	 */
+	t = typ(TINTER);
 
-	funarg = 0;
-	if(et == TFUNC) {
-		funarg = 1;
-		et = TSTRUCT;
+	for(tp = &t->type; l; l=l->next) {
+		f = interfacefield(l->n);
+		if (l->n->left == N && f->type->etype == TINTER) {
+			// embedded interface, inline methods
+			for(t1=f->type->type; t1; t1=t1->down) {
+				f = typ(TFIELD);
+				f->type = t1->type;
+				f->broke = t1->broke;
+				f->sym = t1->sym;
+				if(f->sym)
+					f->nname = newname(f->sym);
+				*tp = f;
+				tp = &f->down;
 			}
-	t = typ(et);
-	t->funarg = funarg;
-	stotype(l, et, &t->type, funarg);
-	if(t->type == T && l != nil) {
+		} else {
+			*tp = f;
+			tp = &f->down;
+		}
+	}
+
+	for(f=t->type; f && !t->broke; f=f->down)
+		if(f->broke)
 			t->broke = 1;
-		return t;
-	}
-	if(et == TINTER)
+
+	checkdupfields(t->type, "method");
 	t = sortinter(t);
-	if(!funarg)
 	checkwidth(t);
+
 	return t;
 }
 
-
 Node*
 embedded(Sym *s)
 {
@@ -1038,9 +1121,12 @@ functype(Node *this, NodeList *in, NodeList *out)
 	rcvr = nil;
 	if(this)
 		rcvr = list1(this);
-	t->type = dostruct(rcvr, TFUNC);
-	t->type->down = dostruct(out, TFUNC);
-	t->type->down->down = dostruct(in, TFUNC);
+	t->type = tofunargs(rcvr);
+	t->type->down = tofunargs(out);
+	t->type->down->down = tofunargs(in);
+
+	if (t->type->broke || t->type->down->broke || t->type->down->down->broke)
+		t->broke = 1;
 
 	if(this)
 		t->thistuple = 1;
@@ -1212,9 +1298,9 @@ addmethod(Sym *sf, Type *t, int local)
 	}
 
 	if(d == T)
-		stotype(list1(n), 0, &pa->method, 0);
+		pa->method = structfield(n);
 	else
-		stotype(list1(n), 0, &d->down, 0);
+		d->down = structfield(n);
 	return;
 }
 

src/cmd/gc/go.h

@@ -935,7 +935,6 @@ void	colasdefn(NodeList *left, Node *defn);
 NodeList*	constiter(NodeList *vl, Node *t, NodeList *cl);
 Node*	dclname(Sym *s);
 void	declare(Node *n, int ctxt);
-Type*	dostruct(NodeList *l, int et);
 void	dumpdcl(char *st);
 Node*	embedded(Sym *s);
 Node*	fakethis(void);
@@ -956,6 +955,8 @@ void	popdcl(void);
 void	poptodcl(void);
 void	redeclare(Sym *s, char *where);
 void	testdclstack(void);
+Type*	tointerface(NodeList *l);
+Type*	tostruct(NodeList *l);
 Node*	typedcl0(Sym *s);
 Node*	typedcl1(Node *n, Node *t, int local);
 void	typedcl2(Type *pt, Type *t);

src/cmd/gc/go.y

@@ -1792,11 +1792,11 @@ hidden_type_misc:
 	}
 |	LSTRUCT '{' ohidden_structdcl_list '}'
 	{
-		$$ = dostruct($3, TSTRUCT);
+		$$ = tostruct($3);
 	}
 |	LINTERFACE '{' ohidden_interfacedcl_list '}'
 	{
-		$$ = dostruct($3, TINTER);
+		$$ = tointerface($3);
 	}
 |	'*' hidden_type
 	{

src/cmd/gc/subr.c

@@ -1052,6 +1052,11 @@ assignop(Type *src, Type *dst, char **why)
 	if(dst->etype == TINTER && src->etype != TNIL) {
 		if(implements(src, dst, &missing, &have, &ptr))
 			return OCONVIFACE;
+
+		// we'll have complained about this method anyway, supress spurious messages.
+		if(have && have->sym == missing->sym && (have->type->broke || missing->type->broke))
+			return OCONVIFACE;
+
 		if(why != nil) {
 			if(isptrto(src, TINTER))
 				*why = smprint(":\n\t%T is pointer to interface, not interface", src);

src/cmd/gc/typecheck.c

@@ -311,7 +311,7 @@ reswitch:
 	case OTSTRUCT:
 		ok |= Etype;
 		n->op = OTYPE;
-		n->type = dostruct(n->list, TSTRUCT);
+		n->type = tostruct(n->list);
 		if(n->type == T)
 			goto error;
 		n->list = nil;
@@ -320,7 +320,7 @@ reswitch:
 	case OTINTER:
 		ok |= Etype;
 		n->op = OTYPE;
-		n->type = dostruct(n->list, TINTER);
+		n->type = tointerface(n->list);
 		if(n->type == T)
 			goto error;
 		break;

test/fixedbugs/bug251.go

@@ -16,6 +16,6 @@ type I2 interface {
 }
 
 
-var i1 I1 = i2 // GC_ERROR "missing m method|need type assertion"
+var i1 I1 = i2
 var i2 I2
 var i2a I2 = i1

test/fixedbugs/bug374.go

@@ -0,0 +1,20 @@
+// errchk $G $D/$F.go
+
+// Copyright 2011 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.
+
+// issue 1556
+package foo
+
+type I interface {
+	m() int
+}
+
+type T int
+
+var _ I = T(0)
+
+func (T) m(buf []byte) (a int, b xxxx) {  // ERROR "xxxx"
+	return 0, nil
+}