Skip to content

go/types: missing cycle errors #34333

Closed
Closed
@griesemer

Description

@griesemer
package p

type A interface {
	B
}

type B interface {
	A
}

produces: "illegal cycle in declaration of A" with go/types. But when we introduce a method m such as in:

package p

type A interface {
	m() B
	B
}

type B interface {
	A
}

the error disappears and the code typechecks. If m is declared after the embedded interface B, the error appears again.

This is an ordering issue in go/types.

Activity

added
NeedsFixThe path to resolution is known, but the work has not been done.
on Sep 16, 2019
added this to the Go1.14 milestone on Sep 16, 2019
self-assigned this
on Sep 16, 2019
griesemer

griesemer commented on Sep 16, 2019

@griesemer
ContributorAuthor

[edited]
PS: This bug was introduced exposed with a80c5f0 .

griesemer

griesemer commented on Sep 16, 2019

@griesemer
ContributorAuthor

This is a general cycle detection bug. Replacing interface with struct:

package p

type A struct {
	f func() B
	b B
}

type B struct {
	A
}

type-checks but it shouldn't.

added
SoonThis needs action soon. (recent regressions, service outages, unusual time-sensitive situations)
on Sep 16, 2019
changed the title [-]go/types: missing interface cycle error[/-] [+]go/types: missing cycle errors for types that permit embedding[/+] on Sep 16, 2019
av86743

av86743 commented on Sep 17, 2019

@av86743

Root cause:

Analysis of detailed design proposal (Jul-Sep 2019) is not based on formal model.

griesemer

griesemer commented on Sep 17, 2019

@griesemer
ContributorAuthor

@av86743 Thanks, but the design proposal you're mentioning is not the root cause at all - it is in fact unrelated. It just so happens that I changed the implementation of interface type-checking entirely when implementing overlapping interfaces, which in turn exposed an underlying bug. As I have mentioned here, this bug also appears for structs which have nothing to do with interfaces; in fact this bug has been around for an embarrassingly long time.

av86743

av86743 commented on Sep 17, 2019

@av86743

@griesemer

... the design proposal you're mentioning is not the root cause at all ...

This is not what I said. I said that "Analysis of ... proposal is [essentially absent]".

... in fact this bug has been around for an embarrassingly long time.

I certainly appreciate your recognition of the fact.

griesemer

griesemer commented on Sep 17, 2019

@griesemer
ContributorAuthor

@av86743 Hm, apologies for the misunderstanding, but I don't understand what you are saying. Since the specific proposal is unrelated to this bug, I don't see why you bring in that proposal in the first place.

av86743

av86743 commented on Sep 17, 2019

@av86743

@griesemer

... I don't see why you bring in that proposal in the first place.

This proposal is the first and only place where you specify (in section "Proposal") what you are intending to do. It also indicates a period when implementation has taken place.

changed the title [-]go/types: missing cycle errors for types that permit embedding[/-] [+]go/types: missing cycle errors[/+] on Sep 17, 2019
griesemer

griesemer commented on Sep 17, 2019

@griesemer
ContributorAuthor

@av86743 Please see the updated title of this issue. It is unrelated to the proposal.

av86743

av86743 commented on Sep 17, 2019

@av86743

@griesemer

Your example with structs does not type-check with current go1.12 (and likely with earlier go versions) - as it should. It started to fail only after "unrelated" changes per said proposal. Think a moment about it.

griesemer

griesemer commented on Sep 17, 2019

@griesemer
ContributorAuthor

@av86743 The latest released version is go1.13 (commit cc8838d), and on my machine the struct example is (incorrectly) accepted. This code failed before said proposal was proposed or implemented.

av86743

av86743 commented on Sep 17, 2019

@av86743

@griesemer

[edited]
PS: This bug was introduced exposed with a80c5f0 .

Then what about this?

griesemer

griesemer commented on Sep 17, 2019

@griesemer
ContributorAuthor

@av86743 Initial guess/misdiagnosis - which is why I corrected it once I realized that this was unrelated. The problem is due to how cycle detection was changed a while back. Anyway, I consider this discussed resolved at this point. Thanks.

av86743

av86743 commented on Sep 17, 2019

@av86743

"... bug was exposed with [commit]" is rather strange choice of wording if you admit that regression in cycle detection appeared before commit was applied (or even existed.) Clearly, the initial interface example is not part of the commit in question but was somehow arrived at, at a later time. Which in turn translates into "... bug was not exposed with [commit]". Hence the root cause.

gopherbot

gopherbot commented on Sep 18, 2019

@gopherbot
Contributor

Change https://golang.org/cl/196338 mentions this issue: go/types: fix cycle detection

locked and limited conversation to collaborators on Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.SoonThis needs action soon. (recent regressions, service outages, unusual time-sensitive situations)release-blocker

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @av86743@griesemer@gopherbot

        Issue actions

          go/types: missing cycle errors · Issue #34333 · golang/go