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

go/types, types2: better error message for failing type inference #39742

Closed
arl opened this issue Jun 21, 2020 · 8 comments
Closed

go/types, types2: better error message for failing type inference #39742

arl opened this issue Jun 21, 2020 · 8 comments
Assignees
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@arl
Copy link
Contributor

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 ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 22, 2020
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jun 22, 2020
@ianlancetaylor
Copy link
Contributor

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

@griesemer
Copy link
Contributor

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 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

@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 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

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.

@griesemer
Copy link
Contributor

I believe the first example correctly reports an error now.
The [second example](type foo[int] of f does not match Foo[T] (cannot infer T)) may be related to #51593.

@griesemer griesemer changed the title cmd/go2go: better error message for failing type inference go/types, types2: better error message for failing type inference Mar 18, 2022
@griesemer griesemer modified the milestones: Unreleased, Go1.19 Mar 18, 2022
@griesemer
Copy link
Contributor

The remaining error here is justified:

package main

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

func forEach[T any](t iterable[T]) {}

func main() {
	v := binaryTree[int]{}
	forEach(v)      // error: type binaryTree[int] of v does not match iterable[T] (cannot infer T)
	
	// arbitrary types T make this work - therefore type inference cannot infer the "correct" one
	forEach[int](v)
	forEach[string](v)
	forEach[bool](v)
}

Since arbitrary types may be used to satisfy the constraint, type inference cannot infer the "correct" type.
Closing.

@golang golang locked and limited conversation to collaborators Jun 23, 2023
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.
Projects
Development

No branches or pull requests

5 participants