Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: compiler runs "forever" for test case #49068

Closed
griesemer opened this issue Oct 19, 2021 · 6 comments
Closed

cmd/compile: compiler runs "forever" for test case #49068

griesemer opened this issue Oct 19, 2021 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@griesemer
Copy link
Contributor

This is from #48974. The following program

package p

type Fooer interface {
	Foo()
}

type Fooable[F Fooer] struct {
	ptr F
}

func (f *Fooable[F]) Adapter() *Fooable[*FooerImpl[F]] {
	return &Fooable[*FooerImpl[F]]{&FooerImpl[F]{}}
}

//
// By removing the 'F Fooer' type param, program compiles sucessfully
//            |||||||||
//            vvvvvvvvv
type FooerImpl[F Fooer] struct {
}

func (fi *FooerImpl[F]) Foo() {}

type-checks successfully now (after fixing #48974) but the compiler appears to hang.

cc: @danscales

@griesemer griesemer added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Oct 19, 2021
@griesemer griesemer added this to the Go1.18 milestone Oct 19, 2021
@griesemer
Copy link
Contributor Author

Slightly simplified reproducer:

package p

type Fooer interface {
	Foo()
}

type Fooable[_ Fooer] struct {}

func (_ *Fooable[F]) Adapter() (res *Fooable[*FooerImpl[F]]) {
	return
}

type FooerImpl[_ Fooer] struct {}

func (*FooerImpl[_]) Foo() {}

@danscales
Copy link
Contributor

I think this another case where things are not monomorphisable, right? In #48018, it was more focused on a generic function that can create arbitrary levels of the generic type Box[]. In this case, we have a method/type combination, where a type's own method creates another level of instantiation of the same type.

If we reference/create Fooable[MyInt], where MyInt satisfies Fooer, then when we instantiate the type and methods, we must create Fooable[*FooerImpl[MyInt]]. But then when we instantiate that type, we must create Fooable[Fooable[*FooerImpl[MyInt]]], etc. So, if we do stenciling/monomorphisation, we need to create instantiations for an infinite number of types. (I realize in this case that Adapter is not actually called and its result used, so maybe we could special-case and not created those methods in this case, but the general problem remains.)

In the current implementation, we actually get in an infinite loop creating types during the translation to types1. If we had some kind of lazy representation of types, we might be able to compile, but we would need to create run-time types or dictionaries on the fly at run-time, especially if actually created an instance of Fooable[] and Adapter were actually called and used for the recursively created types.

I don't think we actually want to handle this case, do we?

@mdempsky Will your non-mono pass catch this case? Or can it be adapted to catch this case?

@mdempsky
Copy link
Member

@danscales Yes, the nomono check will reject this.

@griesemer
Copy link
Contributor Author

@jkmpariab Please discuss design of your code or generics features using the appropriate channels. This is for tracking issues. Thanks.

@jkmpariab
Copy link

@jkmpariab Please discuss design of your code or generics features using the appropriate channels. This is for tracking issues. Thanks.

BTW i just wanted to say that this program works in go1.17 with -gcflags=-G=3 !!!
Isn't this a backward incompatible change to reject this case in go1.18? (with or without -gcflags=-G=3)

@ianlancetaylor
Copy link
Contributor

Using -gcflags=-G=3 is unsupported and does not carry any compatibility guarantees.

@golang golang locked and limited conversation to collaborators Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants