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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: surprising behavior for doubly declared methods #66285

Closed
griesemer opened this issue Mar 13, 2024 · 4 comments
Closed

cmd/compile: surprising behavior for doubly declared methods #66285

griesemer opened this issue Mar 13, 2024 · 4 comments
Assignees
@griesemer
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.23

Comments

@griesemer
Copy link
Contributor

Before Go 1.13, it was an error for a method to appear more than once (via embedding) in an interface. For instance, this code produced an error ("duplicate method Read"). With 1.14, such code was permitted.

Similarly, this code also produces the same error under 1.13 as one would expect:

type T interface {
	io.Reader
	Reader
}

type Reader interface {
	Read(p []byte) (n int, err error)
}

However, this code does not:

type T interface {
	io.Reader
	io.Reader
}

The reason is that here the method Read both times is imported (through io.Reader): because the io package is "correct", the imported Read method is "exempt" from the 1.13 version check. This seems odd: even though io is valid, the actual embedding of Read happens in the current package (or file) and thus the 1.13 rules should apply irrespective of where the method came from.

A method's origin package should not matter for the version test, only the (file) version of the source where the embedding happens.
@griesemer griesemer added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 13, 2024
@griesemer griesemer added this to the Go1.23 milestone Mar 13, 2024
@griesemer griesemer self-assigned this Mar 13, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 13, 2024
@griesemer
Copy link
Contributor Author

cc: @findleyr @rsc for visibility

@rsc
Copy link
Contributor

rsc commented Mar 13, 2024

I agree that the error should be present even when io.Reader is imported from a newer package. If we don't understand why this is happening, then maybe it is worth understanding better, just in case it is symptomatic of some larger bug in the way we think about or handle versions.

That said, if we do understand it and it's just annoying to fix, I think it would be fine to take the check out entirely. The only reason for the check would be to help users keep their code compatible with Go 1.13, but Go 1.13 has been unsupported for many years now, so that's not a reason anymore.

@griesemer
Copy link
Contributor Author

We do understand where it's coming from and the fix is trivial (actually simplifies the code). Thanks for the comment.

@gopherbot
Copy link

Change https://go.dev/cl/571058 mentions this issue: go/types, types2: consistently report "duplicate method" error in go1.13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@griesemer griesemer

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Go Compiler / Runtime
Status: Done
Milestone
Go1.23
Development

No branches or pull requests
3 participants
@rsc @griesemer @gopherbot