Skip to content

Commit 8ae8576

Browse files
committed
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 <[email protected]>
1 parent bf92406 commit 8ae8576

File tree

5 files changed

+35
-17
lines changed

5 files changed

+35
-17
lines changed

src/go/types/check_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ var tests = [][]string{
9292
{"testdata/blank.src"},
9393
{"testdata/issue25008b.src", "testdata/issue25008a.src"}, // order (b before a) is crucial!
9494
{"testdata/issue26390.src"}, // stand-alone test to ensure case is triggered
95+
{"testdata/issue23203a.src"},
96+
{"testdata/issue23203b.src"},
9597
}
9698

9799
var fset = token.NewFileSet()

src/go/types/decl.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -515,11 +515,6 @@ func (check *Checker) typeDecl(obj *TypeName, typ ast.Expr, def *Named, alias bo
515515

516516
}
517517

518-
// check and add associated methods
519-
// TODO(gri) It's easy to create pathological cases where the
520-
// current approach is incorrect: In general we need to know
521-
// and add all methods _before_ type-checking the type.
522-
// See https://play.golang.org/p/WMpE0q2wK8
523518
check.addMethodDecls(obj)
524519
}
525520

@@ -567,7 +562,7 @@ func (check *Checker) addMethodDecls(obj *TypeName) {
567562
check.push(cutCycle)
568563
defer check.pop()
569564

570-
// type-check methods
565+
// add valid methods
571566
for _, m := range methods {
572567
// spec: "For a base type, the non-blank names of methods bound
573568
// to it must be unique."
@@ -585,13 +580,6 @@ func (check *Checker) addMethodDecls(obj *TypeName) {
585580
continue
586581
}
587582

588-
// type-check
589-
// TODO(gri): This call is not needed anymore because the code can handle
590-
// method signatures that have not yet been type-checked.
591-
// Remove in separate CL to make it easy to isolate issues
592-
// that might be introduced by this change.
593-
check.objDecl(m, nil)
594-
595583
if base != nil {
596584
base.methods = append(base.methods, m)
597585
}

src/go/types/testdata/decls0.src

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,10 +189,10 @@ func f2(x *f2 /* ERROR "not a type" */ ) {}
189189
func f3() (x f3 /* ERROR "not a type" */ ) { return }
190190
func f4() (x *f4 /* ERROR "not a type" */ ) { return }
191191

192-
func (S0) m1(x S0.m1 /* ERROR "field or method" */ ) {}
193-
func (S0) m2(x *S0.m2 /* ERROR "field or method" */ ) {}
194-
func (S0) m3() (x S0.m3 /* ERROR "field or method" */ ) { return }
195-
func (S0) m4() (x *S0.m4 /* ERROR "field or method" */ ) { return }
192+
func (S0) m1 /* ERROR illegal cycle */ (x S0 /* ERROR value .* is not a type */ .m1) {}
193+
func (S0) m2 /* ERROR illegal cycle */ (x *S0 /* ERROR value .* is not a type */ .m2) {}
194+
func (S0) m3 /* ERROR illegal cycle */ () (x S0 /* ERROR value .* is not a type */ .m3) { return }
195+
func (S0) m4 /* ERROR illegal cycle */ () (x *S0 /* ERROR value .* is not a type */ .m4) { return }
196196

197197
// interfaces may not have any blank methods
198198
type BlankI interface {

src/go/types/testdata/issue23203a.src

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2018 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
import "unsafe"
8+
9+
type T struct{}
10+
11+
func (T) m1() {}
12+
func (T) m2([unsafe.Sizeof(T.m1)]int) {}
13+
14+
func main() {}

src/go/types/testdata/issue23203b.src

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2018 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
import "unsafe"
8+
9+
type T struct{}
10+
11+
func (T) m2([unsafe.Sizeof(T.m1)]int) {}
12+
func (T) m1() {}
13+
14+
func main() {}

0 commit comments

Comments
 (0)