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/go2go: better error message for failing type inference #39742

Open
arl opened this issue Jun 21, 2020 · 6 comments
Open

cmd/go2go: better error message for failing type inference #39742

arl opened this issue Jun 21, 2020 · 6 comments
Assignees
Milestone

Comments

@arl
Copy link
Contributor

@arl arl commented Jun 21, 2020

What version of Go are you using (go version)?

$ go version
go version devel +0a030888da Sat Jun 20 04:46:40 2020 +0000 linux/amd64

What did you do?

I felt upon this while toying with go2go.
This is the minimal reproducible example illustrating what I think is a bug/problem.

type (
	iterable(type T)   interface{} // empty constraint
	binaryTree(type T) struct{}
)

func forEach(type T)(t iterable(T)) {}

func main() {
	v := binaryTree(int){}

	forEach(v) // error: type binaryTree(int) of v does not match iterable(T)
	// forEach(int)(v) // ok
}

https://go2goplay.golang.org/p/TBApH4WbSxk

What did you expect to see?

First, I think int should not have to be provided explicitly since it can be inferred from v.
Also v does match iterable(T) since iterable is the empty constraint.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 22, 2020

Thanks. This does seem like a problem with the type checker.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Jun 22, 2020

Since no explicit type argument is passed to forEach, it must be inferred. But the (underlying) type of v is struct{} and the underlying type of the t argument type of forEach is interface{}. They don't match in any way and so type inference fails. (Note that for inference to have a chance to succeed, the types must have identical structure at a minimum.) Now, in this case, even if they had the same structure, since the T type parameters are never used, they cannot be inferred anyway, and it would still fail.

If an explicit type argument is provided, it works.

Thus this is working as intended. Leaving open as a reminder that the error message could perhaps be improved. Not urgent.

@griesemer griesemer changed the title cmd/go2go: type does not match empty constraint cmd/go2go: better error message for failing type inference Jun 22, 2020
@CAFxX
Copy link
Contributor

@CAFxX CAFxX commented Jul 6, 2020

I am running into the same issue, but IIUC the explanation above does not seem to apply cleanly:

https://go2goplay.golang.org/p/dVyEtkQ6oGD

package main

type Foo(type T) interface {
	Get() T
}

type foo(type T) struct {
}

func (_ foo(T)) Get() T {
	var zero T
	return zero
}

func Bar(type T)(_ Foo(T)) {
}

func main() {
	{
		var F Foo(int)
		Bar(F) // allowed
	}
	{
		var f foo(int)
		F := Foo(int)(f)
		Bar(F) // allowed
	}
	{
		var f foo(int)
		Bar(int)(f) // allowed
	}
	{	
		var f foo(int)
		Bar(f) // not allowed: "type foo(int) of f does not match Foo(T)"
	}
}

(as a side note, if it somehow were to apply then it's extremely unfortunate that the simplest and most intuitive way of performing the Bar call is actually the only one that is not accepted; also, it's definitely surprising since in current go it is AFAIK always allowed to pass a concrete type in a interface argument - as long as the concrete type satisfies the interface)

Note that the same error is returned even if foo is not an empty struct (e.g. if it contains a T member).

If it turns out this is not the same issue I can open a new one.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 6, 2020

@CAFxX I don't think that's the same issue. But I also don't think it is expected to work. The type inference in your final example is more or less the one described at #39049 (comment); the current algorithm doesn't support that.

@CAFxX
Copy link
Contributor

@CAFxX CAFxX commented Jul 7, 2020

I suppose you meant to link a different issue. Regardless, if really the current design doesn't support it then please consider the bit above after the example as general user feedback on the design rather than on the implementation. If appropriate I can add this to #15292 (or file a new one?).

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 8, 2020

Sorry about that, I meant to link to https://go.googlesource.com/proposal/+/refs/heads/master/design/go2draft-type-parameters.md#type-inference-for-generic-function-arguments

Thanks, the feedback is noted. I don't think it's time yet to file issues against the design draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.