go/types: add Alias.Rhs method #66559
Comments
|
Change https://go.dev/cl/574716 mentions this issue: |
|
Seems fine but we already spell it Rhs in other places, so we should do the same here. |
rsc
changed the title
|
This proposal has been added to the active column of the proposals project |
I'm not convinced: Rhs is not idiomatic spelling, and ast.AssignStmt.Rhs is essentially unrelated to types.Alias.RHS (you wouldn't plumb one through to the other, for example) so I don't see any downside to using the correct capitalization. |
|
Note that That is, it seems like we shouldn't use two different spellings in the same package. |
|
The downside is that I am a gopher of very little brain, and I will never remember when to use Rhs and when to use RHS. |
|
I'm not opposed, but I don't really follow the argument that Aliases are fundamentally different from Named types here. If we want to be able to print Historically, we've intentionally avoided preserving inessential syntax in types, with Interface embeddings being the one notable exception. In fact, I think this philosophy is why we didn't have an Alias type in the first place. Can we clarify our philosophical approach? Have we decided that it's generally a good idea to preserve the syntax of type definitions in the type system? If so, should we add Is this proposal also suggesting that we change the |
|
OK, Rhs is is.
The main motive is that transiting export data should not make material changes to types, as this would cause tools like gopls to display different behavior depending on whether it obtained the types from a cache of export data or from type-annotated syntax. In the case of By contrast, Named types do not reveal their middlemen in their API or their printed representation. (You raise an interesting question as to whether we would be better off if they did, but I don't think we need to address it now. IMHO the more troublesome difficulty with named types is not being able to distinguish A.f from B.f in
No. |
|
Got it. So the problem is that there is an observable behavior of Aliases (their current type string) that can't be preserved through import. I agree that this needs to be fixed. I also agree that the behavior of showing the RHS in the type string is correct. We can worry about philosophy with respect to Named types later. |
As we migrate towards materialized Alias types, the ObjectString
for a type A such as
type A = B
type B = int
should be "type A = B", removing exactly one Alias constructor
from the type of A. (The previous behavior was "type A = int".)
I suspect the existing Alias.{Unalias,Underlying} API is
inadequate and that we will need an Alias.RHS accessor that
removes exactly one Alias. Other clients such as the import/
export packages will need it, because aliases are not
isomorphic to defined types, in which, given
type A B
type B int
the Underlying of A is indeed int. See #66559.
Change-Id: I11a4aacbe6dbeeafc3aee31b3c096296b5970cd8
Reviewed-on: https://go-review.googlesource.com/c/go/+/574716
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
|
Have all remaining concerns about this proposal been addressed? The proposal is to add in go/types |
|
Based on the discussion above, this proposal seems like a likely accept. The proposal is to add in go/types |
|
No change in consensus, so accepted. 🎉 The proposal is to add in go/types |
rsc
changed the title
|
Change https://go.dev/cl/581615 mentions this issue: |
|
Change https://go.dev/cl/581635 mentions this issue: |
Unfortunately we need an aliases.Rhs shim to handle the three cases (before, at, after go1.22). (Alias.Rhs was recently added in CL CL 581615.) Updates golang/go#66559 Change-Id: I25a35c14f3ef5ddb77712afcce17f960dd181b5c Reviewed-on: https://go-review.googlesource.com/c/tools/+/581635 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com>
Proposal Details
Background: Go 1.22's go/types defined new API for materialized alias types, as a step towards generic alias types. Alias has two operations:
Underlying, which removes all Alias and Named constructors, recursively, returning the representation type; andUnalias, which removes only Alias constructors, again recursively, returning the first non-alias type. (A judicious sprinkling ofUnaliasoperations throughout existing code is the minimal change to keep most existing go/types clients working.)However, the API provides no way to remove a single
Aliasconstructor. That means, for example, that there is no way for a client of go/types, given the types for:to write a function that prints "type A = B" (without calling the existing
ObjectStringmethod). More importantly, import/export tools are unable to preserve the structure of the original types.(
Unaliasis nonetheless useful, andUnderlyingis of course a requirement of theTypeinterface, but Alias types are not isomorphic to defined types in the way that, giventype A B; type B int, the spec defines the underlying type of A as int, and otherwise stipulates no relationship between A and B.)Proposal: We add an
Alias.RHSaccessor method, which removes exactly oneAliasconstructor. So,Alias.RHS(A)would return B.@gri @findleyr @mdempsky
The text was updated successfully, but these errors were encountered: