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: improve error messages when types don't satisfy constraints #41125

Open
urandom opened this issue Aug 29, 2020 · 6 comments
Open

cmd/go2go: improve error messages when types don't satisfy constraints #41125

urandom opened this issue Aug 29, 2020 · 6 comments
Assignees
Milestone

Comments

@urandom
Copy link

@urandom urandom commented Aug 29, 2020

The following program fails to compile: https://go2goplay.golang.org/p/-2Qmhtax9VX
The following error is observed:
prog.go2:238:9: *mapIt(string, int, reverse(string, ReverseIterator(string))) does not satisfy Iterator(T) (missing method Next)

Note that on line 224, the same parametric type is being instantiated, and used in 228 without a problem. Also note line 238, invoking the method the type checker complains about.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 29, 2020

If I'm reading the code correctly, there should be an error here. But it would be nice if the error were better.

The line is

	err := ForEach(m2, func(i string) {

Here m2 has type mapIt[string, int, reverse[string, ReverseIterator[string]]].

The ForEach function is

func ForEach[T any, I Iterator[T]](it I, consumer func(T)) error {

So I must be the type of m2, shown above. And T must be string. So we require that the type of m2 must satisfy the constraint Iterator[string].

We have

type Iterator[T any] interface {
	Next() (T, bool)
}

So the type of m2 must have a method Next() (string, bool). The mapIt Next method is

func (i *mapIt[T, U, I]) Next() (U, bool) {

so in this case the type of the Next method is actually Next() (int, bool). But we need Next() (string, bool).

So I believe the type checker is correct to report an error. But the error shouldn't be missing method Next. It should be something more like "method Next with type Next() (int, bool) does not satisfy constraint which requires Next() (string, bool).

That would be a start, anyhow. It's still pretty hard to understand what is going on here. It would be good to see if there is something we can do to make it clearer.

@urandom
Copy link
Author

@urandom urandom commented Aug 30, 2020

Yes, that is indeed the issue, though I also didn't realize it at first. I've seen the type checker be able to tell the difference between methods with different instantiated parameters, so it's definitely able to report the error correctly in such cases

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 30, 2020

Change https://golang.org/cl/251717 mentions this issue: [dev.go2go] go/types: improve error when a type argument doesn't satisfy constraint

@griesemer
Copy link
Contributor

@griesemer griesemer commented Aug 30, 2020

The new error message is now:

tmp.go2:245:9: *mapIt[string, int, reverse[string, ReverseIterator[string]]] does not satisfy Iterator[T]: wrong method signature
                found func (*mapIt[T, U, I]).Next() (U, bool)
                want  func (Iterator[T any]).Next() (string, bool)

This can be improved further (the T, U, I parameters should be substituted), but this is already better than what we had.

Leaving this issue open for now for future improvements.

@urandom, please simplify test cases when reporting an issue; it makes it significantly easier to figure out what's going on.

gopherbot pushed a commit that referenced this issue Aug 31, 2020
…sfy constraint

If the cause is a method with incorrect signature, report the
expected and presented signature.

Updates #41125.

Change-Id: Ibd0fa23547b77df924ec5421cc8e657d41c5a5a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/251717
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@griesemer griesemer changed the title cmd/go2go: type stops satisfying constraint cmd/go2go: improve error messages when types don't satisfy constraints Sep 3, 2020
@urandom
Copy link
Author

@urandom urandom commented Sep 10, 2020

Perhaps the following example illustrates a similar message problem:
https://go2goplay.golang.org/p/O1Un2Vm9Dh0

The compiler outputs:
prog.go2:33:30: interface contains type constraints (T)

I initially through that parametric interfaces weren't allowed in type assertion expressions.

As pointed out in golang-nuts however, it appears that the problem is the fact that the interface has a type list. Perhaps the message should say so instead.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Sep 10, 2020

@urandom Well, a type list is a type constraint. But agreed, this should probably say "type list" instead (or whatever we use as the official terminology once we have a spec). Not urgent.

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.