go/types: don't type-check method signatures eagerly anymore

As a side-effect we also get slightly clearer errors for some
pathological cyclic method declarations.

Fixes #23203.
Updates #26854.

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

diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go
index 2bdfc150f4191d4d0bf1056dc1c33d4395d09c1e..e8ba1a037c45a2c7dfadf4ca64f1a90a7e876d91 100644 (file)
--- a/src/go/types/check_test.go
+++ b/src/go/types/check_test.go
@@ -92,6 +92,8 @@ var tests = [][]string{
        {"testdata/blank.src"},
        {"testdata/issue25008b.src", "testdata/issue25008a.src"}, // order (b before a) is crucial!
        {"testdata/issue26390.src"},                              // stand-alone test to ensure case is triggered
+       {"testdata/issue23203a.src"},
+       {"testdata/issue23203b.src"},
 }
 
 var fset = token.NewFileSet()

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index b4a1eec1acd928e711c65345963aca1399992654..5a6eda8ee4903cc733f507762b6810cf4e33f98a 100644 (file)
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -515,11 +515,6 @@ func (check *Checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, alias bo
 
        }
 
-       // check and add associated methods
-       // TODO(gri) It's easy to create pathological cases where the
-       // current approach is incorrect: In general we need to know
-       // and add all methods _before_ type-checking the type.
-       // See https://play.golang.org/p/WMpE0q2wK8
        check.addMethodDecls(obj)
 }
 
@@ -567,7 +562,7 @@ func (check *Checker) addMethodDecls(obj *TypeName) {
        check.push(cutCycle)
        defer check.pop()
 
-       // type-check methods
+       // add valid methods
        for _, m := range methods {
                // spec: "For a base type, the non-blank names of methods bound
                // to it must be unique."
@@ -585,13 +580,6 @@ func (check *Checker) addMethodDecls(obj *TypeName) {
                        continue
                }
 
-               // type-check
-               // TODO(gri): This call is not needed anymore because the code can handle
-               //            method signatures that have not yet been type-checked.
-               //            Remove in separate CL to make it easy to isolate issues
-               //            that might be introduced by this change.
-               check.objDecl(m, nil)
-
                if base != nil {
                        base.methods = append(base.methods, m)
                }

diff --git a/src/go/types/testdata/decls0.src b/src/go/types/testdata/decls0.src
index e75216172b83b507001cfdb4368f91ec89c3c216..56adbbfaae671c8dd3cb575e93ff63108fa08925 100644 (file)
--- a/src/go/types/testdata/decls0.src
+++ b/src/go/types/testdata/decls0.src
@@ -189,10 +189,10 @@ func f2(x *f2 /* ERROR "not a type" */ ) {}
 func f3() (x f3 /* ERROR "not a type" */ ) { return }
 func f4() (x *f4 /* ERROR "not a type" */ ) { return }
 
-func (S0) m1(x S0.m1 /* ERROR "field or method" */ ) {}
-func (S0) m2(x *S0.m2 /* ERROR "field or method" */ ) {}
-func (S0) m3() (x S0.m3 /* ERROR "field or method" */ ) { return }
-func (S0) m4() (x *S0.m4 /* ERROR "field or method" */ ) { return }
+func (S0) m1 /* ERROR illegal cycle */ (x S0 /* ERROR value .* is not a type */ .m1) {}
+func (S0) m2 /* ERROR illegal cycle */ (x *S0 /* ERROR value .* is not a type */ .m2) {}
+func (S0) m3 /* ERROR illegal cycle */ () (x S0 /* ERROR value .* is not a type */ .m3) { return }
+func (S0) m4 /* ERROR illegal cycle */ () (x *S0 /* ERROR value .* is not a type */ .m4) { return }
 
 // interfaces may not have any blank methods
 type BlankI interface {

diff --git a/src/go/types/testdata/issue23203a.src b/src/go/types/testdata/issue23203a.src
new file mode 100644 (file)
index 0000000..48cb588
--- /dev/null
+++ b/src/go/types/testdata/issue23203a.src
@@ -0,0 +1,14 @@
+// Copyright 2018 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.
+
+package main
+
+import "unsafe"
+
+type T struct{}
+
+func (T) m1()                         {}
+func (T) m2([unsafe.Sizeof(T.m1)]int) {}
+
+func main() {}

diff --git a/src/go/types/testdata/issue23203b.src b/src/go/types/testdata/issue23203b.src
new file mode 100644 (file)
index 0000000..638ec6c
--- /dev/null
+++ b/src/go/types/testdata/issue23203b.src
@@ -0,0 +1,14 @@
+// Copyright 2018 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.
+
+package main
+
+import "unsafe"
+
+type T struct{}
+
+func (T) m2([unsafe.Sizeof(T.m1)]int) {}
+func (T) m1()                         {}
+
+func main() {}