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

Fixes #1556.

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

diff --git a/src/cmd/gc/dcl.c b/src/cmd/gc/dcl.c
index 01457ecd4515cb5e7ebfd7cde7791ef8eaf18463..71afbf1e50093aee0b4aa11f362e0010620b024a 100644 (file)
--- a/src/cmd/gc/dcl.c
+++ b/src/cmd/gc/dcl.c
@@ -709,181 +709,264 @@ ok:
  * they don't belong here, but where do they belong?
  */
 
+static void
+checkembeddedtype(Type *t)
+{
+       if (t == T)
+               return;
 
-/*
- * turn a parsed struct into a type
- */
-static Type**
-stotype(NodeList *l, int et, Type **t, int funarg)
+       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, *t1, *t2, **t0;
-       Strlit *note;
+       Type *f;
        int lno;
-       Node *n, *left;
-       char *what;
 
-       t0 = t;
        lno = lineno;
-       what = "field";
-       if(et == TINTER)
-               what = "method";
+       lineno = n->lineno;
 
-       for(; l; l=l->next) {
-               n = l->n;
-               lineno = n->lineno;
+       if(n->op != ODCLFIELD)
+               fatal("structfield: oops %N\n", n);
 
-               if(n->op != ODCLFIELD)
-                       fatal("stotype: oops %N\n", n);
-               left = n->left;
-               if(funarg && isblank(left))
-                       left = N;
-               if(n->right != N) {
-                       if(et == TINTER && 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;
-                               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;
+       f = typ(TFIELD);
+       f->isddd = n->isddd;
+
+       if(n->right != 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;
                                }
-                       }
-               }
 
-               if(n->type == T) {
-                       // assume error already printed
-                       continue;
-               }
+       lineno = lno;
+}
 
-               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;
+/*
+ * 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;
                }
 
-               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;
+               // 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->left->type = n->type;
+                       queuemethod(n);
+
+                       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);
                        }
-                       continue;
-               }
 
-               f = typ(TFIELD);
-               f->type = n->type;
-               f->note = note;
-               f->width = BADWIDTH;
-               f->isddd = n->isddd;
+               } else {
+
+                       typecheck(&n->right, Etype);
+                       n->type = n->right->type;
 
-               // 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;
-                       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);
-                                               break;
-                                       }
+                       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;
        }
 
-       *t = T;
+       n->right = N;
+       
+       f->type = n->type;
+       if(f->type == T)
+               f->broke = 1;
+       
        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;
-       }
-       t = typ(et);
-       t->funarg = funarg;
-       stotype(l, et, &t->type, funarg);
-       if(t->type == T && l != nil) {
-               t->broke = 1;
-               return t;
+       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;
+                       }
+               } else {
+                       *tp = f;
+                       tp = &f->down;
+               }
        }
-       if(et == TINTER)
-               t = sortinter(t);
-       if(!funarg)
-               checkwidth(t);
+
+       for(f=t->type; f && !t->broke; f=f->down)
+               if(f->broke)
+                       t->broke = 1;
+
+       checkdupfields(t->type, "method");
+       t = sortinter(t);
+       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;
 }
 

diff --git a/src/cmd/gc/go.h b/src/cmd/gc/go.h
index 40ba7394e8bcd652173dfd141cd1a925b917745a..52344e75637edf3c3610c6376c0e315b3ddb5565 100644 (file)
--- a/src/cmd/gc/go.h
+++ b/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);

diff --git a/src/cmd/gc/go.y b/src/cmd/gc/go.y
index 31ffc6d5baa350b52bd3011e3b73bf3e9a6701c8..0ec1905b55805e5223f39180993cf9e3a41e82a9 100644 (file)
--- a/src/cmd/gc/go.y
+++ b/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
        {

diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c
index 11feb61a9738d773048ddf10266bb9003b080845..83486e576237faa52bf9d4b559456a869e3ee773 100644 (file)
--- a/src/cmd/gc/subr.c
+++ b/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);

diff --git a/src/cmd/gc/typecheck.c b/src/cmd/gc/typecheck.c
index 6ae4384e0b8e0a2a5c08368bd6a99ce4da69876e..f154db9eb4840dd65ae1d13b7bcdd01e575c1b63 100644 (file)
--- a/src/cmd/gc/typecheck.c
+++ b/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;

diff --git a/test/fixedbugs/bug251.go b/test/fixedbugs/bug251.go
index c94ad2abe2caca41beef727df0904c7352c14fcb..385f28dd4dacec5984607efaeae78bef2fd0f701 100644 (file)
--- a/test/fixedbugs/bug251.go
+++ b/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

diff --git a/test/fixedbugs/bug374.go b/test/fixedbugs/bug374.go
new file mode 100644 (file)
index 0000000..2e6f27a
--- /dev/null
+++ b/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
+}
\ No newline at end of file