go/types: postpone interface method type comparison to the end

Introduce a new list of final actions that is executed at the
end of type checking and use it to collect method type comparisons
and also map key checks.

Fixes #33656.

Change-Id: Ia77a35a45a9d7eaa7fc3e9e19f41f32dcd6ef9d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/191418
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/go/types/check.go b/src/go/types/check.go
index fbf0f4a91184399ef4fce7eea7ca7c9909e2aeff..7d58183911c47815344c1cc58da953aee94e6f6f 100644 (file)
--- a/src/go/types/check.go
+++ b/src/go/types/check.go
@@ -89,7 +89,8 @@ type Checker struct {
        firstErr error                 // first error encountered
        methods  map[*TypeName][]*Func // maps package scope type names to associated non-blank (non-interface) methods
        untyped  map[ast.Expr]exprInfo // map of expressions without final type
-       delayed  []func()              // stack of delayed actions
+       delayed  []func()              // stack of delayed action segments; segments are processed in FIFO order
+       finals   []func()              // list of final actions; processed at the end of type-checking the current set of files
        objPath  []Object              // path of object dependencies during type inference (for cycle reporting)
 
        // context within which the current object is type-checked
@@ -145,6 +146,14 @@ func (check *Checker) later(f func()) {
        check.delayed = append(check.delayed, f)
 }
 
+// atEnd adds f to the list of actions processed at the end
+// of type-checking, before initialization order computation.
+// Actions added by atEnd are processed after any actions
+// added by later.
+func (check *Checker) atEnd(f func()) {
+       check.finals = append(check.finals, f)
+}
+
 // push pushes obj onto the object path and returns its index in the path.
 func (check *Checker) push(obj Object) int {
        check.objPath = append(check.objPath, obj)
@@ -195,6 +204,7 @@ func (check *Checker) initFiles(files []*ast.File) {
        check.methods = nil
        check.untyped = nil
        check.delayed = nil
+       check.finals = nil
 
        // determine package name and collect valid files
        pkg := check.pkg
@@ -245,6 +255,7 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
        check.packageObjects()
 
        check.processDelayed(0) // incl. all functions
+       check.processFinals()
 
        check.initOrder()
 
@@ -258,6 +269,16 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
        return
 }
 
+func (check *Checker) processFinals() {
+       n := len(check.finals)
+       for _, f := range check.finals {
+               f() // must not append to check.finals
+       }
+       if len(check.finals) != n {
+               panic("internal error: final action list grew")
+       }
+}
+
 func (check *Checker) recordUntyped() {
        if !debug && check.Types == nil {
                return // nothing to do

diff --git a/src/go/types/testdata/cycles2.src b/src/go/types/testdata/cycles2.src
index e95506a1089700f8c58cbf548d087f42c5ac95d8..98ca6f4e44bce3cbb0a6ce5cdb2ea31388be3dfc 100644 (file)
--- a/src/go/types/testdata/cycles2.src
+++ b/src/go/types/testdata/cycles2.src
@@ -45,13 +45,11 @@ type B interface {
 type AB interface {
        a() interface {
                A
-               // TODO(gri) there shouldn't be an error here. See issue #33656.
-               B // ERROR duplicate method a
+               B
        }
        b() interface {
                A
-               // TODO(gri) there shouldn't be an error here. See issue #33656.
-               B // ERROR duplicate method a
+               B
        }
 }
 

diff --git a/src/go/types/type.go b/src/go/types/type.go
index 23ae6e33b709bf9cecb720462e87ecdd581a7b1a..5c28a2e7ba364d79f123f6081591719b87707649 100644 (file)
--- a/src/go/types/type.go
+++ b/src/go/types/type.go
@@ -351,16 +351,18 @@ func (t *Interface) Complete() *Interface {
 
        t.allMethods = markComplete // avoid infinite recursion
 
+       var todo []*Func
        var methods []*Func
        var seen objset
        addMethod := func(m *Func, explicit bool) {
-               switch alt := seen.insert(m); {
-               case alt == nil:
+               switch other := seen.insert(m); {
+               case other == nil:
                        methods = append(methods, m)
-               case explicit || !Identical(m.Type(), alt.Type()):
+               case explicit:
                        panic("duplicate method " + m.name)
                default:
-                       // silently drop method m
+                       // check method signatures after all locally embedded interfaces are computed
+                       todo = append(todo, m, other.(*Func))
                }
        }
 
@@ -376,6 +378,14 @@ func (t *Interface) Complete() *Interface {
                }
        }
 
+       for i := 0; i < len(todo); i += 2 {
+               m := todo[i]
+               other := todo[i+1]
+               if !Identical(m.typ, other.typ) {
+                       panic("duplicate method " + m.name)
+               }
+       }
+
        if methods != nil {
                sort.Sort(byUniqueMethodName(methods))
                t.allMethods = methods

diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go
index c2d218f5c3fdb35c1a6c340c04e538bdea9e45ed..19bedae590a60bffd1fe72256925ae2d6d14cc8e 100644 (file)
--- a/src/go/types/typexpr.go
+++ b/src/go/types/typexpr.go
@@ -314,7 +314,7 @@ func (check *Checker) typInternal(e ast.Expr, def *Named) Type {
                //
                // Delay this check because it requires fully setup types;
                // it is safe to continue in any case (was issue 6667).
-               check.later(func() {
+               check.atEnd(func() {
                        if !Comparable(typ.key) {
                                check.errorf(e.Key.Pos(), "invalid map key type %s", typ.key)
                        }
@@ -560,17 +560,20 @@ func (check *Checker) completeInterface(ityp *Interface) {
        var methods []*Func
        var seen objset
        addMethod := func(m *Func, explicit bool) {
-               switch alt := seen.insert(m); {
-               case alt == nil:
+               switch other := seen.insert(m); {
+               case other == nil:
                        methods = append(methods, m)
-               case explicit || !Identical(m.Type(), alt.Type()):
+               case explicit:
                        check.errorf(m.pos, "duplicate method %s", m.name)
-                       // We use "other" rather than "previous" here because
-                       // the first declaration seen may not be textually
-                       // earlier in the source.
-                       check.errorf(alt.Pos(), "\tother declaration of %s", m) // secondary error, \t indented
+                       check.reportAltDecl(other)
                default:
-                       // silently drop method m
+                       // check method signatures after all types are computed (issue #33656)
+                       check.atEnd(func() {
+                               if !Identical(m.typ, other.Type()) {
+                                       check.errorf(m.pos, "duplicate method %s", m.name)
+                                       check.reportAltDecl(other)
+                               }
+                       })
                }
        }
 
@@ -581,7 +584,7 @@ func (check *Checker) completeInterface(ityp *Interface) {
        posList := check.posMap[ityp]
        for i, typ := range ityp.embeddeds {
                pos := posList[i] // embedding position
-               typ := typ.Underlying().(*Interface)
+               typ := underlying(typ).(*Interface)
                check.completeInterface(typ)
                for _, m := range typ.allMethods {
                        copy := *m