]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile/internal/gc: handle recursive interfaces better
authorMatthew Dempsky <mdempsky@google.com>
Mon, 20 Mar 2017 18:56:15 +0000 (11:56 -0700)
committerMatthew Dempsky <mdempsky@google.com>
Tue, 21 Mar 2017 01:56:25 +0000 (01:56 +0000)
Previously, we handled recursive interfaces by deferring typechecking
of interface methods, while eagerly expanding interface embeddings.

This CL switches to eagerly evaluating interface methods, and
deferring expanding interface embeddings to dowidth. This allows us to
detect recursive interface embeddings with the same mechanism used for
detecting recursive struct embeddings.

Updates #16369.

Change-Id: If4c0320058047f8a2d9b52b9a79de47eb9887f95
Reviewed-on: https://go-review.googlesource.com/38391
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
src/cmd/compile/internal/gc/align.go
src/cmd/compile/internal/gc/bimport.go
src/cmd/compile/internal/gc/dcl.go
src/cmd/compile/internal/gc/fmt.go
src/cmd/compile/internal/gc/type.go
src/cmd/compile/internal/gc/typecheck.go
test/fixedbugs/bug195.go
test/fixedbugs/bug251.go
test/fixedbugs/issue18392.go

index ee3c7aec7eb6cd27a1bc1a02a301e6df96ef5f54..db5edee4510997cc7c3a2c7c74d4a47ca4858fa6 100644 (file)
@@ -4,6 +4,10 @@
 
 package gc
 
+import (
+       "sort"
+)
+
 // machine size and rounding alignment is dictated around
 // the size of a pointer, set in betypeinit (see ../amd64/galign.go).
 var defercalc int
@@ -15,6 +19,47 @@ func Rnd(o int64, r int64) int64 {
        return (o + r - 1) &^ (r - 1)
 }
 
+// expandiface computes the method set for interface type t by
+// expanding embedded interfaces.
+func expandiface(t *Type) {
+       var fields []*Field
+       for _, m := range t.Methods().Slice() {
+               if m.Sym != nil {
+                       fields = append(fields, m)
+                       continue
+               }
+
+               if !m.Type.IsInterface() {
+                       yyerrorl(m.Nname.Pos, "interface contains embedded non-interface %v", m.Type)
+                       m.SetBroke(true)
+                       t.SetBroke(true)
+                       // Add to fields so that error messages
+                       // include the broken embedded type when
+                       // printing t.
+                       // TODO(mdempsky): Revisit this.
+                       fields = append(fields, m)
+                       continue
+               }
+
+               // Embedded interface: duplicate all methods
+               // (including broken ones, if any) and add to t's
+               // method set.
+               for _, t1 := range m.Type.Fields().Slice() {
+                       f := newField()
+                       f.Type = t1.Type
+                       f.SetBroke(t1.Broke())
+                       f.Sym = t1.Sym
+                       f.Nname = m.Nname // preserve embedding position
+                       fields = append(fields, f)
+               }
+       }
+       sort.Sort(methcmp(fields))
+
+       // Access fields directly to avoid recursively calling dowidth
+       // within Type.Fields().
+       t.Extra.(*InterType).fields.Set(fields)
+}
+
 func offmod(t *Type) {
        o := int32(0)
        for _, f := range t.Fields().Slice() {
@@ -203,9 +248,8 @@ func dowidth(t *Type) {
 
        case TINTER: // implemented as 2 pointers
                w = 2 * int64(Widthptr)
-
                t.Align = uint8(Widthptr)
-               offmod(t)
+               expandiface(t)
 
        case TCHAN: // implemented as pointer
                w = int64(Widthptr)
@@ -316,6 +360,14 @@ func dowidth(t *Type) {
                t.Align = uint8(w)
        }
 
+       if t.Etype == TINTER {
+               // We defer calling these functions until after
+               // setting t.Width and t.Align so the recursive calls
+               // to dowidth within t.Fields() will succeed.
+               checkdupfields("method", t)
+               offmod(t)
+       }
+
        lineno = lno
 
        if defercalc == 1 {
index e7308df2a87070efb6849b38281675353803f2c0..e417536fc82f77c8a1a0869901be2674ce847b3c 100644 (file)
@@ -535,7 +535,6 @@ func (p *importer) typ() *Type {
                        t = p.newtyp(TINTER)
                        t.SetInterface(ml)
                }
-               checkwidth(t)
 
        case mapTag:
                t = p.newtyp(TMAP)
index 6fca2062d07f352782ec930e17384c7ee7402712..9bc7f84a5ef1a359657b667d22154ae27b0ae341 100644 (file)
@@ -7,7 +7,6 @@ package gc
 import (
        "cmd/internal/src"
        "fmt"
-       "sort"
        "strings"
 )
 
@@ -705,24 +704,19 @@ func structfield(n *Node) *Field {
 // checkdupfields emits errors for duplicately named fields or methods in
 // a list of struct or interface types.
 func checkdupfields(what string, ts ...*Type) {
-       lno := lineno
-
        seen := make(map[*Sym]bool)
        for _, t := range ts {
                for _, f := range t.Fields().Slice() {
-                       if f.Sym == nil || f.Nname == nil || isblank(f.Nname) {
+                       if f.Sym == nil || isblanksym(f.Sym) || f.Nname == nil {
                                continue
                        }
                        if seen[f.Sym] {
-                               lineno = f.Nname.Pos
-                               yyerror("duplicate %s %s", what, f.Sym.Name)
+                               yyerrorl(f.Nname.Pos, "duplicate %s %s", what, f.Sym.Name)
                                continue
                        }
                        seen[f.Sym] = true
                }
        }
-
-       lineno = lno
 }
 
 // convert a parsed id/type list into
@@ -805,50 +799,26 @@ func interfacefield(n *Node) *Field {
                yyerror("interface method cannot have annotation")
        }
 
-       f := newField()
-       f.SetIsddd(n.Isddd())
+       // MethodSpec = MethodName Signature | InterfaceTypeName .
+       //
+       // If Left != nil, then Left is MethodName and Right is Signature.
+       // Otherwise, Right is InterfaceTypeName.
 
        if n.Right != nil {
-               if n.Left != nil {
-                       // 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.SetNname(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
-                       }
-               } else {
-                       n.Right = typecheck(n.Right, Etype)
-                       n.Type = n.Right.Type
-
-                       if n.Embedded != 0 {
-                               checkembeddedtype(n.Type)
-                       }
-
-                       if n.Type != nil {
-                               switch n.Type.Etype {
-                               case TINTER:
-                                       break
-
-                               case TFORW:
-                                       yyerror("interface type loop involving %v", n.Type)
-                                       f.SetBroke(true)
-
-                               default:
-                                       yyerror("interface contains embedded non-interface %v", n.Type)
-                                       f.SetBroke(true)
-                               }
-                       }
-               }
+               n.Right = typecheck(n.Right, Etype)
+               n.Type = n.Right.Type
+               n.Right = nil
        }
 
-       n.Right = nil
+       f := newField()
+       if n.Left != nil {
+               f.Nname = n.Left
+               f.Sym = f.Nname.Sym
+       } else {
+               // Placeholder ONAME just to hold Pos.
+               // TODO(mdempsky): Add Pos directly to Field instead.
+               f.Nname = newname(nblank.Sym)
+       }
 
        f.Type = n.Type
        if f.Type == nil {
@@ -876,32 +846,13 @@ func tointerface0(t *Type, l []*Node) *Type {
        var fields []*Field
        for _, n := range l {
                f := interfacefield(n)
-
-               if n.Left == nil && f.Type.IsInterface() {
-                       // embedded interface, inline methods
-                       for _, t1 := range f.Type.Fields().Slice() {
-                               f = newField()
-                               f.Type = t1.Type
-                               f.SetBroke(t1.Broke())
-                               f.Sym = t1.Sym
-                               if f.Sym != nil {
-                                       f.Nname = newname(f.Sym)
-                               }
-                               fields = append(fields, f)
-                       }
-               } else {
-                       fields = append(fields, f)
-               }
                if f.Broke() {
                        t.SetBroke(true)
                }
+               fields = append(fields, f)
        }
-       sort.Sort(methcmp(fields))
        t.SetInterface(fields)
 
-       checkdupfields("method", t)
-       checkwidth(t)
-
        return t
 }
 
index d09c9808ea641f86b363472401292fcf5d8d4020..6932f6de2c2a8a37eed8e7135b116f71fb117b2d 100644 (file)
@@ -642,7 +642,6 @@ var etnames = []string{
        TBLANK:      "TBLANK",
        TFUNCARGS:   "TFUNCARGS",
        TCHANARGS:   "TCHANARGS",
-       TINTERMETH:  "TINTERMETH",
        TDDDFIELD:   "TDDDFIELD",
 }
 
index e4841708f6e736e0f0d67de02c7f7219c560d43e..8beba292f6ca53a5428020592cfdb7b1d3dac8f5 100644 (file)
@@ -65,7 +65,6 @@ const (
        // pseudo-types for frame layout
        TFUNCARGS
        TCHANARGS
-       TINTERMETH
 
        // pseudo-types for import/export
        TDDDFIELD // wrapper: contained type is a ... field
@@ -420,8 +419,6 @@ func typ(et EType) *Type {
                t.Extra = new(ForwardType)
        case TFUNC:
                t.Extra = new(FuncType)
-       case TINTERMETH:
-               t.Extra = InterMethType{}
        case TSTRUCT:
                t.Extra = new(StructType)
        case TINTER:
@@ -807,8 +804,6 @@ func (t *Type) Nname() *Node {
        switch t.Etype {
        case TFUNC:
                return t.Extra.(*FuncType).Nname
-       case TINTERMETH:
-               return t.Extra.(InterMethType).Nname
        }
        Fatalf("Type.Nname %v %v", t.Etype, t)
        return nil
@@ -819,8 +814,6 @@ func (t *Type) SetNname(n *Node) {
        switch t.Etype {
        case TFUNC:
                t.Extra.(*FuncType).Nname = n
-       case TINTERMETH:
-               t.Extra = InterMethType{Nname: n}
        default:
                Fatalf("Type.SetNname %v %v", t.Etype, t)
        }
@@ -846,6 +839,7 @@ func (t *Type) Fields() *Fields {
        case TSTRUCT:
                return &t.Extra.(*StructType).fields
        case TINTER:
+               dowidth(t)
                return &t.Extra.(*InterType).fields
        }
        Fatalf("Fields: type %v does not have fields", t)
@@ -882,7 +876,7 @@ func (t *Type) SetFields(fields []*Field) {
 
 func (t *Type) SetInterface(methods []*Field) {
        t.wantEtype(TINTER)
-       t.Fields().Set(methods)
+       t.Methods().Set(methods)
 }
 
 func (t *Type) isDDDArray() bool {
index 27b9bb7b5e9a0dea6e9ad7b6d571afec6c9cfc55..353380a0d9d12f29fd5a32fb37e086d994a0dda3 100644 (file)
@@ -3571,8 +3571,15 @@ func copytype(n *Node, t *Type) {
        if n.Name != nil {
                t.Vargen = n.Name.Vargen
        }
-       t.methods = Fields{}
-       t.allMethods = Fields{}
+
+       // spec: "The declared type does not inherit any methods bound
+       // to the existing type, but the method set of an interface
+       // type [...] remains unchanged."
+       if !t.IsInterface() {
+               t.methods = Fields{}
+               t.allMethods = Fields{}
+       }
+
        t.nod = n
        t.SetDeferwidth(false)
        t.ptrTo = ptrTo
index 85367cb8882939edc758352b073620cb4b1d523d..8d392bda715053541b64a8de59e9158aa0195c0c 100644 (file)
@@ -14,14 +14,14 @@ type I3 interface { int }   // ERROR "interface"
 type S struct {
        x interface{ S }        // ERROR "interface"
 }
-type I4 interface {
-       I4      // ERROR "interface"
+type I4 interface { // GC_ERROR "invalid recursive type"
+       I4      // GCCGO_ERROR "interface"
 }
 
 type I5 interface {
        I6      // GCCGO_ERROR "interface"
 }
 
-type I6 interface {
-       I5      // ERROR "interface"
+type I6 interface { // GC_ERROR "invalid recursive type"
+       I5      // GCCGO_ERROR "interface"
 }
index f061723eda531d468994af818d55bb53c4519997..05e111a61f810648a6408227fa25a3957b036608 100644 (file)
@@ -6,13 +6,17 @@
 
 package main
 
-type I1 interface {
+type I1 interface { // GC_ERROR "invalid recursive type"
        m() I2
-       I2 // GCCGO_ERROR "loop|interface"
+       // TODO(mdempsky): The duplicate method error is silly
+       // and redundant, but tricky to prevent as it's actually
+       // being emitted against the underlying interface type
+       // literal, not I1 itself.
+       I2 // ERROR "loop|interface|duplicate method m"
 }
 
 type I2 interface {
-       I1 // ERROR "loop|interface"
+       I1 // GCCGO_ERROR "loop|interface"
 }
 
 
index ad6423898307b37b50bf5bdfe11e9fa8bd718ce9..053a337867997cd9df9c03183c2e9925b20f6de4 100644 (file)
@@ -7,5 +7,8 @@
 package p
 
 type A interface {
-       Fn(A.Fn) // ERROR "type A has no method A.Fn"
+       // TODO(mdempsky): This should be an error, but this error is
+       // nonsense. The error should actually mention that there's a
+       // type loop.
+       Fn(A.Fn) // ERROR "type A has no method Fn"
 }