[release-branch.go1.20] go/types, types2: don't panic during interface completion

It should be possible for the importer to construct an invalid
interface, as would have been produced by type checking.

Updates #61737
Fixes #61744

Change-Id: I72e063f4f1a6205d273a623acce2ec08c34c3cc2
Reviewed-on: https://go-review.googlesource.com/c/go/+/515555
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Olif Oftimis <oftimisolif@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit d2ee7821d357a4e4948b9a6251e82b4ced9a1eae)
Reviewed-on: https://go-review.googlesource.com/c/go/+/515638
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/cmd/compile/internal/types2/api_test.go b/src/cmd/compile/internal/types2/api_test.go
index fe84720052eecc91635ec681d1ea328fe8414904..79b38c68a38b0fd48c2f2999acd4861671c29492 100644 (file)
--- a/src/cmd/compile/internal/types2/api_test.go
+++ b/src/cmd/compile/internal/types2/api_test.go
@@ -1982,6 +1982,29 @@ func TestIdenticalUnions(t *testing.T) {
        }
 }
 
+func TestIssue61737(t *testing.T) {
+       // This test verifies that it is possible to construct invalid interfaces
+       // containing duplicate methods using the go/types API.
+       //
+       // It must be possible for importers to construct such invalid interfaces.
+       // Previously, this panicked.
+
+       sig1 := NewSignatureType(nil, nil, nil, NewTuple(NewParam(nopos, nil, "", Typ[Int])), nil, false)
+       sig2 := NewSignatureType(nil, nil, nil, NewTuple(NewParam(nopos, nil, "", Typ[String])), nil, false)
+
+       methods := []*Func{
+               NewFunc(nopos, nil, "M", sig1),
+               NewFunc(nopos, nil, "M", sig2),
+       }
+
+       embeddedMethods := []*Func{
+               NewFunc(nopos, nil, "M", sig2),
+       }
+       embedded := NewInterfaceType(embeddedMethods, nil)
+       iface := NewInterfaceType(methods, []Type{embedded})
+       iface.NumMethods() // unlike go/types, there is no Complete() method, so we complete implicitly
+}
+
 func TestIssue15305(t *testing.T) {
        const src = "package p; func f() int16; var _ = f(undef)"
        f := mustParse("issue15305.go", src)

diff --git a/src/cmd/compile/internal/types2/typeset.go b/src/cmd/compile/internal/types2/typeset.go
index 673cadca902058625a517b87e11617ea39f805c3..e240866c5f7530bd625e7ca4624af1d0accdf9a3 100644 (file)
--- a/src/cmd/compile/internal/types2/typeset.go
+++ b/src/cmd/compile/internal/types2/typeset.go
@@ -6,7 +6,6 @@ package types2
 
 import (
        "cmd/compile/internal/syntax"
-       "fmt"
        . "internal/types/errors"
        "sort"
        "strings"
@@ -212,7 +211,6 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
        // we can get rid of the mpos map below and simply use the cloned method's
        // position.
 
-       var todo []*Func
        var seen objset
        var allMethods []*Func
        mpos := make(map[*Func]syntax.Pos) // method specification or method embedding position, for good error messages
@@ -222,36 +220,30 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
                        allMethods = append(allMethods, m)
                        mpos[m] = pos
                case explicit:
-                       if check == nil {
-                               panic(fmt.Sprintf("%s: duplicate method %s", m.pos, m.name))
+                       if check != nil {
+                               var err error_
+                               err.code = DuplicateDecl
+                               err.errorf(pos, "duplicate method %s", m.name)
+                               err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name)
+                               check.report(&err)
                        }
-                       // check != nil
-                       var err error_
-                       err.code = DuplicateDecl
-                       err.errorf(pos, "duplicate method %s", m.name)
-                       err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name)
-                       check.report(&err)
                default:
                        // We have a duplicate method name in an embedded (not explicitly declared) method.
                        // Check method signatures after all types are computed (issue #33656).
                        // If we're pre-go1.14 (overlapping embeddings are not permitted), report that
                        // error here as well (even though we could do it eagerly) because it's the same
                        // error message.
-                       if check == nil {
-                               // check method signatures after all locally embedded interfaces are computed
-                               todo = append(todo, m, other.(*Func))
-                               break
+                       if check != nil {
+                               check.later(func() {
+                                       if !check.allowVersion(m.pkg, 1, 14) || !Identical(m.typ, other.Type()) {
+                                               var err error_
+                                               err.code = DuplicateDecl
+                                               err.errorf(pos, "duplicate method %s", m.name)
+                                               err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name)
+                                               check.report(&err)
+                                       }
+                               }).describef(pos, "duplicate method check for %s", m.name)
                        }
-                       // check != nil
-                       check.later(func() {
-                               if !check.allowVersion(m.pkg, 1, 14) || !Identical(m.typ, other.Type()) {
-                                       var err error_
-                                       err.code = DuplicateDecl
-                                       err.errorf(pos, "duplicate method %s", m.name)
-                                       err.errorf(mpos[other.(*Func)], "other declaration of %s", m.name)
-                                       check.report(&err)
-                               }
-                       }).describef(pos, "duplicate method check for %s", m.name)
                }
        }
 
@@ -317,15 +309,6 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
        }
        ityp.embedPos = nil // not needed anymore (errors have been reported)
 
-       // process todo's (this only happens if check == nil)
-       for i := 0; i < len(todo); i += 2 {
-               m := todo[i]
-               other := todo[i+1]
-               if !Identical(m.typ, other.typ) {
-                       panic(fmt.Sprintf("%s: duplicate method %s", m.pos, m.name))
-               }
-       }
-
        ityp.tset.comparable = allComparable
        if len(allMethods) != 0 {
                sortMethods(allMethods)

diff --git a/src/go/types/api_test.go b/src/go/types/api_test.go
index 98ef6c423f5c4913a49d552bba0e63bf371e89a7..2b72607d4e3de823e2ed4ed6c4dfbe732f451280 100644 (file)
--- a/src/go/types/api_test.go
+++ b/src/go/types/api_test.go
@@ -1973,6 +1973,29 @@ func TestIdenticalUnions(t *testing.T) {
        }
 }
 
+func TestIssue61737(t *testing.T) {
+       // This test verifies that it is possible to construct invalid interfaces
+       // containing duplicate methods using the go/types API.
+       //
+       // It must be possible for importers to construct such invalid interfaces.
+       // Previously, this panicked.
+
+       sig1 := NewSignatureType(nil, nil, nil, NewTuple(NewParam(token.NoPos, nil, "", Typ[Int])), nil, false)
+       sig2 := NewSignatureType(nil, nil, nil, NewTuple(NewParam(token.NoPos, nil, "", Typ[String])), nil, false)
+
+       methods := []*Func{
+               NewFunc(token.NoPos, nil, "M", sig1),
+               NewFunc(token.NoPos, nil, "M", sig2),
+       }
+
+       embeddedMethods := []*Func{
+               NewFunc(token.NoPos, nil, "M", sig2),
+       }
+       embedded := NewInterfaceType(embeddedMethods, nil)
+       iface := NewInterfaceType(methods, []Type{embedded})
+       iface.Complete()
+}
+
 func TestIssue15305(t *testing.T) {
        const src = "package p; func f() int16; var _ = f(undef)"
        fset := token.NewFileSet()

diff --git a/src/go/types/typeset.go b/src/go/types/typeset.go
index d68446df66d7f49f44d97e7e9b0099dcdf715fe3..4facce0a33951d455bc4292a02ed60df03f83313 100644 (file)
--- a/src/go/types/typeset.go
+++ b/src/go/types/typeset.go
@@ -5,7 +5,6 @@
 package types
 
 import (
-       "fmt"
        "go/token"
        . "internal/types/errors"
        "sort"
@@ -216,7 +215,6 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
        // we can get rid of the mpos map below and simply use the cloned method's
        // position.
 
-       var todo []*Func
        var seen objset
        var allMethods []*Func
        mpos := make(map[*Func]token.Pos) // method specification or method embedding position, for good error messages
@@ -226,30 +224,24 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
                        allMethods = append(allMethods, m)
                        mpos[m] = pos
                case explicit:
-                       if check == nil {
-                               panic(fmt.Sprintf("%v: duplicate method %s", m.pos, m.name))
+                       if check != nil {
+                               check.errorf(atPos(pos), DuplicateDecl, "duplicate method %s", m.name)
+                               check.errorf(atPos(mpos[other.(*Func)]), DuplicateDecl, "\tother declaration of %s", m.name) // secondary error, \t indented
                        }
-                       // check != nil
-                       check.errorf(atPos(pos), DuplicateDecl, "duplicate method %s", m.name)
-                       check.errorf(atPos(mpos[other.(*Func)]), DuplicateDecl, "\tother declaration of %s", m.name) // secondary error, \t indented
                default:
                        // We have a duplicate method name in an embedded (not explicitly declared) method.
                        // Check method signatures after all types are computed (issue #33656).
                        // If we're pre-go1.14 (overlapping embeddings are not permitted), report that
                        // error here as well (even though we could do it eagerly) because it's the same
                        // error message.
-                       if check == nil {
-                               // check method signatures after all locally embedded interfaces are computed
-                               todo = append(todo, m, other.(*Func))
-                               break
+                       if check != nil {
+                               check.later(func() {
+                                       if !check.allowVersion(m.pkg, 1, 14) || !Identical(m.typ, other.Type()) {
+                                               check.errorf(atPos(pos), DuplicateDecl, "duplicate method %s", m.name)
+                                               check.errorf(atPos(mpos[other.(*Func)]), DuplicateDecl, "\tother declaration of %s", m.name) // secondary error, \t indented
+                                       }
+                               }).describef(atPos(pos), "duplicate method check for %s", m.name)
                        }
-                       // check != nil
-                       check.later(func() {
-                               if !check.allowVersion(m.pkg, 1, 14) || !Identical(m.typ, other.Type()) {
-                                       check.errorf(atPos(pos), DuplicateDecl, "duplicate method %s", m.name)
-                                       check.errorf(atPos(mpos[other.(*Func)]), DuplicateDecl, "\tother declaration of %s", m.name) // secondary error, \t indented
-                               }
-                       }).describef(atPos(pos), "duplicate method check for %s", m.name)
                }
        }
 
@@ -315,15 +307,6 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
        }
        ityp.embedPos = nil // not needed anymore (errors have been reported)
 
-       // process todo's (this only happens if check == nil)
-       for i := 0; i < len(todo); i += 2 {
-               m := todo[i]
-               other := todo[i+1]
-               if !Identical(m.typ, other.typ) {
-                       panic(fmt.Sprintf("%v: duplicate method %s", m.pos, m.name))
-               }
-       }
-
        ityp.tset.comparable = allComparable
        if len(allMethods) != 0 {
                sortMethods(allMethods)