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: pointer method example fails type interference when defined as a struct's method #63708

Closed
mitar opened this issue Oct 24, 2023 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@mitar
Copy link
Contributor

mitar commented Oct 24, 2023

I am looking at the pointer method example from generics doc. There it states:

This approach works as expected, but it is awkward to have to repeat Settable in the type arguments. Fortunately, constraint type inference makes it less awkward.

But if I convert that example into methods on a struct, the type interference does not make it less awkward. Demo

package main

import "strconv"

type Settable int

func (p *Settable) Set(s string) {
	i, _ := strconv.Atoi(s)
	*p = Settable(i)
}

type Setter[B any] interface {
	Set(string)
	*B // non-interface type constraint element
}

type F[T any, PT Setter[T]] struct{}

func (_ F[T, PT]) FromStrings(s []string) []T {
	result := make([]T, len(s))
	for i, v := range s {
		p := PT(&result[i])
		p.Set(v)
	}
	return result
}

func main() {
	_ = F[Settable, *Settable]{}.FromStrings([]string{"1", "2"})
	_ = F[Settable]{}.FromStrings([]string{"1", "2"})
}

The last line fails compiling. Are there plans to improve this?

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 24, 2023
@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 24, 2023
@dr2chase
Copy link
Contributor

@griesemer @findleyr

@griesemer
Copy link
Contributor

The problem here is not related to pointer methods in any way. For instance, this simplified version of the code:

package main

type F[T any, PT *T] struct{}

func main() {
	_ = F[int, *int]{}
	_ = F[int]{}
}

has the same problem. The issue is that type inference is not implemented for generic types because its possible to create cycles that our existing type inference cannot handle.

The plan forward is to enable type inference when certain cycles don't exist, but it's not a high priority at this point because you can simply use a factory function if needed:

// factory function for F type
func f[T any, PT *T]() struct{} {
	return F[T, PT]{}
}

Type inference works with the factory function (playground) and this way the code looks pretty nice. Here is the original example using a factory function.

I'm going to close this as working as intended.

@mitar
Copy link
Contributor Author

mitar commented Oct 25, 2023

@griesemer, thank you for your detailed explanation.

To me the issue here is that I am making an exported struct type from a package. I hoped that my users could just create a struct with one type variable, but now it seems just for the typing reasons I would have to provide a New function, even if it does not do anything else. While such a factory function seems nice, it feels to me as unnecessary?

On a side note, the only reason why I need two type variables seems like a more basic limitation to me. I would prefer if I could just do (play link):

type Setter interface {
	Set(string)
}

type F[T Setter] struct{}

func (_ F[T]) Make(s string) T {
	x := new(deref T)
	x.Set(s)
	return x
}

So the only reason I need two type variables is because there is not "deref" type operator, which would convert pointer to type to type and pass it to new. Doing x := *new(T) also does not work because you end up with nil.

Currently I decided to go with reflect (play link):

func newT[T Setter]() T {
	typ := reflect.TypeOf((*T)(nil)).Elem().Elem()
	return reflect.New(typ).Interface().(T)
}

But it feels to me like this should be unnecessary. Are there any plans to introduce such a type operator?

I'm going to close this as working as intended.

Why not leave it open until such (low priority) type inference is introduced?

@griesemer
Copy link
Contributor

@mitar Currently there's no plan to introduce a "deref" type operator - the mechanism we have in place is through additional (in this case a 2nd ) type parameters in the type parameter list with respective constraints. Using a "deref" operator is an interesting idea; it would be a type operator that allows you to "take apart" a type rather than construct a new type, which is what Go's type system usually does. Violating this "invariant" may have subtle repercussions that we may rely on elsewhere (need to think about it some more).

I closed this issue because a) there is no "bug" here, and b) we are aware of the missing type inference for generic types - it's something we hope to fix eventually. But it's not blocking anyone, and providing factory functions for exported types is not really a hurdle and may even be recommended.

Re: the "deref" idea: you could file a proposal if you are so inclined. Just know that may open the door to other "type deconstruction" operator requests. Not sure we want to go there. Thanks.

@mitar
Copy link
Contributor Author

mitar commented Oct 25, 2023

@griesemer Thank you again for the time for the explanation. I think my reflect approach above will work out for me just fine. Based on what you said I did try to write a generic newSiteT function with two type parameters where one would be inferred (because my understanding was that should work), but I just couldn't get it to work. I will leave to somebody else to propose the deref idea. Thanks.

@griesemer
Copy link
Contributor

@mitar I don't understand why the example I sent cannot be adjusted - it was based on your code. What are you intending to do? Why does that need reflection?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants