go/types: incorrect "no such field/method" error in cyclic interface #18395

alandonovan · 2016-12-21T00:20:57Z

The following excerpt minimized from a real program causes go/types to emit a spurious error:

$ cat a.go
package base

type A interface {
        B
}

type B interface {
        C
}

type C interface {
        D
        F() A
}

type D interface {
        G() B
}

var _ = A(nil).G // error: no such field or method
$ ssadump a.go
a.go:20:9: invalid operation: A(nil) (value of type A) has no field or method G
ssadump: couldn't load packages due to errors: base

The text was updated successfully, but these errors were encountered:

griesemer · 2017-06-23T01:00:41Z

Nice. It appears that the existing algorithm for type-checking interfaces is incorrect, and this subtle example exposes the problem. Type-checking proceeds as follows:

$ gotype y.go
y.go:16:6:	-- declaring D
y.go:16:8:	.  interface{G() B}
y.go:17:10:	.  .  func() B
y.go:17:13:	.  .  .  B
y.go:7:6:	.  .  .  .  -- declaring B
y.go:7:8:	.  .  .  .  .  interface{C}
y.go:8:9:	.  .  .  .  .  .  C
y.go:11:6:	.  .  .  .  .  .  .  -- declaring C
y.go:11:8:	.  .  .  .  .  .  .  .  interface{D; F() A}
y.go:12:9:	.  .  .  .  .  .  .  .  .  D
y.go:12:9:	.  .  .  .  .  .  .  .  .  => D
y.go:13:10:	.  .  .  .  .  .  .  .  .  func() A
y.go:13:13:	.  .  .  .  .  .  .  .  .  .  A
y.go:3:6:	.  .  .  .  .  .  .  .  .  .  .  -- declaring A
y.go:3:8:	.  .  .  .  .  .  .  .  .  .  .  .  interface{B}
y.go:4:9:	.  .  .  .  .  .  .  .  .  .  .  .  .  B
y.go:4:9:	.  .  .  .  .  .  .  .  .  .  .  .  .  => B
y.go:3:8:	.  .  .  .  .  .  .  .  .  .  .  .  => interface{B}
y.go:3:6:	.  .  .  .  .  .  .  .  .  .  .  => type A interface{B}
y.go:13:13:	.  .  .  .  .  .  .  .  .  .  => A
y.go:13:10:	.  .  .  .  .  .  .  .  .  => func() A
y.go:11:8:	.  .  .  .  .  .  .  .  => interface{F() A; D}
y.go:11:6:	.  .  .  .  .  .  .  => type C interface{F() A; D}
y.go:8:9:	.  .  .  .  .  .  => C
y.go:7:8:	.  .  .  .  .  => interface{C}
y.go:7:6:	.  .  .  .  => type B interface{C}
y.go:17:13:	.  .  .  => B
y.go:17:10:	.  .  => func() B
y.go:16:8:	.  => interface{G() B}
y.go:16:6:	=> type D interface{G() B}
y.go:20:5:	-- declaring _
y.go:20:9:	.  A(nil).F
y.go:20:9:	.  .  A(nil)
y.go:20:9:	.  .  .  A
y.go:20:9:	.  .  .  => A (type)
y.go:20:11:	.  .  .  nil
y.go:20:11:	.  .  .  => nil (untyped nil value)
y.go:20:9:	.  .  => A(nil) (value of type A)
y.go:20:9: invalid operation: A(nil) (value of type A) has no field or method F
y.go:20:9:	.  => A(nil).F (invalid operand)
y.go:20:5:	=> var _ invalid type

The problem is that the collection of methods for method A depends on B at a time when interface B is not yet finished collecting its methods.

A correct (?) approach may require the collection of all methods first, for all interfaces, before proceeding with type-checking their signatures. This may also eliminate the need for a topological sort of the interfaces before type-checking them (see ordering.go, Checker.resolveOrder). Either way, a correct solution requires some serious thinking.

This is too invasive a change for Go1.9. Postponing to 1.10.

gopherbot · 2017-11-21T06:05:48Z

Change https://golang.org/cl/78955 mentions this issue: go/types: add debugging code to detect use of incomplete interfaces

griesemer · 2017-11-21T06:21:36Z

Status update: Started working on this. Have a basic solution based on proposed solution above (collecting methods first, before creating the respective types). Working on cleaning up the approach (it's a lot of code).

rsc · 2017-11-22T03:35:44Z

If it's a lot of code maybe this should wait for Go 1.11?

griesemer · 2017-11-22T06:48:46Z

@rsc: Maybe. The issue is that not having this will prevent vet from working in some cases. They are rare but so far three have been filed. I want to work on this just a tad longer to see what the final outcome is.

gopherbot · 2017-11-23T00:36:47Z

Change https://golang.org/cl/79575 mentions this issue: go/types: correctly compute method set of some recusive interfaces

The comment for phase 2 of checker.interfaceType (typexpr.go:517) requires that embedded interfaces be complete for correctness of the algorithm. Yet, the very next comment (typexpr.go:530) states that underlying embedded interfaces may in fact be incomplete. This is in fact the case and the underlying bug in issue #18395. This change makes sure that new interface types are marked complete when finished (per the implicit definition in Interface.Complete, type.go:302). It also adds a check, enabled in debug mode only, to detect the use of incomplete embedded interfaces during construction of a new interface. In debug mode, this check fails for the testcase in the issue (and several others). This change has no noticeable impact with debug mode disabled. For #18395. Change-Id: Ibb81e47257651282fb3755a80a36ab5d392e636d Reviewed-on: https://go-review.googlesource.com/78955 Reviewed-by: Alan Donovan <adonovan@google.com>

griesemer · 2017-11-27T21:54:09Z

Decided to postpone submit of fix to 1.11 in discussion with @rsc, @ianlancetaylor . See also related #22890.

gopherbot · 2017-11-29T01:30:59Z

Change https://golang.org/cl/80455 mentions this issue: go/types: panic when recognizing issue #18395.

The fix (CL 79575) for #18395 is too risky at this stage of the Go 1.10 release process. Since issue #18395 is easily recognized (but not easily fixed), report an error instead of silently continuing. This avoids inscrutable follow on errors. Also, make sure all empty interfaces are "completed", and adjust printing code to report incomplete interfaces. For #18395. Change-Id: I7fa5f97ff31ac9775c9a6d318fce9f526b0350cd Reviewed-on: https://go-review.googlesource.com/80455 Reviewed-by: Alan Donovan <adonovan@google.com>

gopherbot · 2017-12-19T23:57:37Z

Change https://golang.org/cl/84898 mentions this issue: go/types: perform delayed tests even for types.Eval

R=go1.11 types.Eval historically never evaluated any delayed tests, which included verification of validity of map keys, but also function literal bodies. Now, embedded interfaces are also type-checked in a delayed fashion, so it becomes imperative to do all delayed checks for eval (otherwise obviously incorrect type expressions are silently accepted). Enabling the delayed tests also removes the restriction that function literals were not type-checked. Also fixed a bug where eval wouldn't return a type-checking error because check.handleBailout was using the wrong err variable. Added tests that verify that method set computation is using the right types when evaluating interfaces with embedded types. For #18395. For #22992. Change-Id: I574fa84568b5158bca4b4ccd4ef5abb616fbf896 Reviewed-on: https://go-review.googlesource.com/84898 Reviewed-by: Alan Donovan <adonovan@google.com>

gopherbot · 2018-04-20T20:22:54Z

Change https://golang.org/cl/108555 mentions this issue: cmd/go: respect vet typecheck failures again

Now that #18395 is fixed, let's see if we can insist on vet during go test being able to type-check packages again. Change-Id: Iaa55a4d9c582ba743df2347d28c24f130e16e406 Reviewed-on: https://go-review.googlesource.com/108555 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>

alandonovan assigned griesemer Dec 21, 2016

dsnet mentioned this issue Dec 21, 2016

go/types: spurious "no such field/methodmissing #18394

Closed

griesemer added this to the Go1.9 milestone Dec 21, 2016

griesemer added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 21, 2016

griesemer modified the milestones: Go1.10, Go1.9 Jun 23, 2017

griesemer changed the title ~~go/types: spurious "no such field/method" error in cyclic interface~~ go/types: incorrect "no such field/method" error in cyclic interface Jun 23, 2017

griesemer mentioned this issue Oct 23, 2017

go/types: rejects valid program involving interfaces and cycles #21804

Closed

griesemer mentioned this issue Nov 14, 2017

go/types: fails to fully collect methods from embedded interfaces #22701

Closed

griesemer added the release-blocker label Nov 14, 2017

griesemer mentioned this issue Nov 27, 2017

cmd/go: re-enable go/types requirement during vet during go test #22890

Closed

griesemer modified the milestones: Go1.10, Go1.11 Nov 29, 2017

griesemer added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Nov 29, 2017

ianlancetaylor mentioned this issue Dec 5, 2017

x/tools/go/loader: program with nested interfaces works in Go but not with the loader #22866

Closed

gopherbot closed this as completed in dd44895 Feb 12, 2018

golang locked and limited conversation to collaborators Apr 20, 2019

gopherbot added the FrozenDueToAge label Apr 20, 2019

rsc unassigned griesemer Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go/types: incorrect "no such field/method" error in cyclic interface #18395

go/types: incorrect "no such field/method" error in cyclic interface #18395

alandonovan commented Dec 21, 2016 •

edited

griesemer commented Jun 23, 2017

gopherbot commented Nov 21, 2017

griesemer commented Nov 21, 2017

rsc commented Nov 22, 2017

griesemer commented Nov 22, 2017

gopherbot commented Nov 23, 2017

griesemer commented Nov 27, 2017

gopherbot commented Nov 29, 2017

gopherbot commented Dec 19, 2017

gopherbot commented Apr 20, 2018

Navigation Menu

go/types: incorrect "no such field/method" error in cyclic interface #18395

go/types: incorrect "no such field/method" error in cyclic interface #18395

Comments

alandonovan commented Dec 21, 2016 • edited

griesemer commented Jun 23, 2017

gopherbot commented Nov 21, 2017

griesemer commented Nov 21, 2017

rsc commented Nov 22, 2017

griesemer commented Nov 22, 2017

gopherbot commented Nov 23, 2017

griesemer commented Nov 27, 2017

gopherbot commented Nov 29, 2017

gopherbot commented Dec 19, 2017

gopherbot commented Apr 20, 2018

alandonovan commented Dec 21, 2016 •

edited