-
Notifications
You must be signed in to change notification settings - Fork 18k
go/types, cmd/compile/internal/types2: disallow embedding type parameters #43621
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
Comments
The current design draft in effect chooses your option (1): https://go.googlesource.com/proposal/+/refs/heads/master/design/go2draft-type-parameters.md#embedded-type-parameter-methods . The program should print Please don't draw any conclusions from the behavior of the go2go tool. The support for embedding type parameters is a horrible bodge. We should add this to our list of test cases, though. |
I see. I read "any methods of the type parameter's constraint are promoted to be methods of the struct" as merely confirming that embedded type parameters participate in method promotion like any other embedded type, not as implying that type parameter methods effectively have depth 0 regardless of their depth on corresponding type arguments. I think the current wording is fine for most Go developers, but I think advanced Go programmers (particularly Go tools developers) would benefit from an extra sentence clarifying this. |
I think it's also worth calling this out since it's a case where generic functions intentionally behave differently than a simple textual expansion of type arguments. For example, I think most Go programmers would intuitively expect Granted I don't think they'd intentionally write code exactly like that, but I can imagine someone doing a refactoring where they either introduce or remove type parameters, and then they're bit by some subtle change in semantics like in #41685. |
Change https://golang.org/cl/283113 mentions this issue: |
…thods For golang/go#43621 Change-Id: Ice63bffb753a1c429ee3537cb3093f2903d499d6 Reviewed-on: https://go-review.googlesource.com/c/proposal/+/283113 Trust: Ian Lance Taylor <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
I was thinking about this some more last night / this morning, and I think for type identity we need to distinguish: (1) normal embedding of a type into a struct and (2) embedding a type into a struct via a type parameter. In particular, I think |
I think we can still allow conversions between those two cases, similar to how type identity rules for structs are relaxed for converting structs that have tagged fields. But I think they do need to be different types. |
That sounds like an argument that we should not permit embedding type parameters. |
Disallowing embedding of type parameters would nicely avoid the issue as well. |
Disallowing embedding for now seems like a good idea. That would reduce the potential for mistakes in the initial spec, and we can always add support in a later version when we can dedicate more time to getting the (subtle) details right. |
Relatedly, I think we should disallow
|
Yes, we should clearly reject methods with receiver type (That seems like a separate bug.) |
I think the common issue here is that different types have different rules for how they contribute methods/fields when embedded / used as an underlying type, and when they're allowed to be used as the underlying type of a defined type with declared methods. Note that the issue with Another issue I realized this morning is |
I agree with what appears to be the consensus here: we should at least initially reject these ambiguous cases. We have #45639, which is similar to the case of |
@findleyr SGTM. |
Change https://golang.org/cl/339651 mentions this issue: |
This program prints
"A B"
, which I think is wrong: https://go2goplay.golang.org/p/QJiCMNb0YS3The type parameters draft mentions that type parameters can be embedded and their methods will be promoted, but it doesn't explain how depth of methods on the original types influences this promotion, if at all, and the test case above demonstrates it's inconsistently handled by cmd/go2go.
I think two reasonable options here are either (1) to specify that for the purposes of method promotion, methods on type parameters are always considered to have depth 0, regardless of their depth on the actual type arguments; or (2) side-step the issue by disallowing embedding type parameters if method promotion could vary depending on the type parameter's methods' depths.
/cc @griesemer @ianlancetaylor
The text was updated successfully, but these errors were encountered: