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/139897Reviewed-by: Alan Donovan <adonovan@google.com>

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()

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)
 		}

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 {

src/go/types/testdata/issue23203a.src

+// 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() {}

src/go/types/testdata/issue23203b.src

+// 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() {}