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: "missing method" diagnostic can be confusing when methods aren't promoted due to ambiguity #57352

Closed
maxatome opened this issue Dec 16, 2022 · 6 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@maxatome
Copy link

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

$ go version
go version go1.19.4 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

playground

What did you do?

package main

type A interface {
	a()
}

type AB interface {
	A
	b()
}

type Foo struct {
	A
	AB
}

func main() {
	var _ AB = Foo{}
}

https://go.dev/play/p/_SQyOJ_n2qv

What did you expect to see?

Either no error or an error saying that Foo has 2 a methods.

What did you see instead?

./prog.go:18:13: cannot use Foo{} (value of type Foo) as type AB in variable declaration:
	Foo does not implement AB (missing a method)

I didn't find this case in spec, but the message seems wrong.

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

thanm commented Dec 16, 2022

@griesemer

@zephyrtronium
Copy link
Contributor

Foo doesn't have two a methods; it is missing method a because a is not promoted due to being present in two embedded fields at the same depth. The rule comes from https://go.dev/ref/spec#Selectors:

For a value x of type T or *T where T is not a pointer or interface type, x.f denotes the field or method at the shallowest depth in T where there is such an f. If there is not exactly one f with shallowest depth, the selector expression is illegal.

And per https://go.dev/ref/spec#Struct_types, a field or method f of a type embedded in a struct x is only promoted when x.f is a legal selector.

@maxatome
Copy link
Author

@zephyrtronium you are right, thank you for the pointer I missed :)

That said, the message does not help to diagnose such cases.

Thanks again!

@mdempsky mdempsky changed the title cmd/compile: weird diagnostic, missing method instead of duplicate cmd/compile: "missing method" diagnostic can be confusing when methods aren't promoted due to ambiguity Dec 20, 2022
@mdempsky
Copy link
Member

It seems reasonable in principle for the error message hint to be "ambiguous selector" instead of "missing method".

/cc @findleyr

@griesemer
Copy link
Contributor

griesemer commented Dec 20, 2022

Agreed. This should be fairly straight-forward. I'll have a look.

Edit: lookupFieldOrMethod provides the necessary information via the index result but it needs to be returned through missingMethod etc. Not too difficult but requires some careful adjustments to internal APIs. For 1.21.

@griesemer griesemer self-assigned this Dec 20, 2022
@griesemer griesemer added this to the Go1.21 milestone Dec 20, 2022
@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 20, 2022
@gopherbot
Copy link

Change https://go.dev/cl/473658 mentions this issue: go/types, types2: better error when method is missing due to ambiguity

@golang golang locked and limited conversation to collaborators Mar 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants