-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
Proposal: Alias declarations for Go #16339
Comments
Pending design doc: https://go-review.googlesource.com/24867 |
Personally, just off the top of my head 👎 , I think I'm not interested in language features specifically targeting refactoring. I vendor libraries (using submodules or subtrees) for precisely this reason. That is, all my vendors can change their implementations at will without breaking my builds and my repos can uptake more current libraries as time permits. In fact, i'm inclined to think this is a very bad idea because you can end up with aliases to libraries that further change their interfaces and break the aliases. For example, let's just say I ended up with 50 libraries that alias types in my library. How do I know about those dependencies? And if i choose to make a drastic change and privatize or rename the type, let's say i even gave a year notice of its deprecation. Do the aliases get that notification? How? And after the deprecation deadline, all those libraries that weren't changed now break. |
CL https://golang.org/cl/24867 mentions this issue. |
What is the significance of forbidding aliasing |
I am confident this would have made a painful LSC refactoring I conducted several months ago more palatable. I appreciate your attention to these shortcomings. One issue: how much is it worth to use (edited for clarity) |
@matttproud Go does not already have If I may jump on the syntax bikeshed bandwagon, though, I think that |
I'm inclined in favor of a more limited form of the proposal. Here's a first round of comments, in no particular order:
|
I can't speak yet if it's a good thing or bad (I lean towards bad but I need more time to understand the ramifications). Meanwhile, I would see this as |
I don't like how closely the
Rather than reading the alias as "this points to", I see it as Beyond that, I really don't like the scope that this proposal allows. Refactoring is a difficult problem, but it should really be driven by the package owner(s). To that point, I'd add these restrictions:
|
I definitely prefer this to import shenanigans. Something like
seems like it could be confusing. Though obviously one should not do such a thing. Vet check? What would the effect of
be? That seems like it would have to be explicitly illegal. As far as the syntax, I do like @ better than ->, very mnemonic. Totally fine with -> though—it'd stop feeling weird after a couple uses. |
I think @Kunde21's point is an important one: since this is presented as a method to facilitate long refactorings, that implies it's addressing a temporary need. Yet at GopherCon one of the immediate questions was "can this be used to expose details of internal packages?" (I'd add "in a controlled manner") and that implies a more permanent situation. I'm not saying for or against, I'm saying that perhaps the motivation for this needs some refinement. |
I presume that the following is illegal, but at first glance I don't see what in the proposal forbids it: package foo
type T -> bar.T
func (t T) SomeMethod() { /* ... */ } |
|
I agree with @Kunde21 @mem: while the motivation is to add this feature for refactoring I think it will likely be used more often in permanent code. I think @Kunde21's suggestions to have Alternatives that come to mind that I did not see listed in the design document:
(edited only to remove a duplicate word which made a sentence read poorly) |
@leighmcculloch Versioning doesn't work. You need to actually alias, to keep type signatures compatible. Imagine if module |
An observation: |
How about:
This would, at least conceptually, lead to things like:
(does this go towards pros or cons?) |
If a thing is not exported and you want to export it, just change it's name to uppercase. Aliases seem neither helpful nor sensible to me, to "export unexported things". Same goes for aliasing into internal packages - by definition you have full control over both the internal subtree and everything that's using it, so if you want to export something from an internal package, don't put it into internal/. For those reasons I think it's not a good idea to allow exporting unexported things; either giving a compilation error in the package that defines the alias (in the case of unexported identifiers) or in the package that uses it (maybe? In the case of internal packages. Because "using an alias into an internal package is the same as using the aliased identifier", so it should give an import error?), or also the package that defines the alias (probably less confusing). |
Just a few quick comments before I try to think it over a little more:
|
The discovery that there are uses for aliases other than the original motivating examples speaks in favor of the proposal, not against it. It means the proposed mechanism is general and powerful. I find this second application appealing. Note that we already have a similar mechanism for 'go test' that provides a similar feature, namely exporting a variable/function only when testing. (See any of the files named "export_test.go" in the standard library.) It is very useful. |
And that observation points out an existing way to accomplish the refactoring goals (I think) without changing the language: build tags. Type T could live in package L or L1, depending on the presence of a build tag. When clients migrate, they flip their build tags. Once all clients have migrated, all build tags get removed. This adds several burdens.
import "pkg/path/L" // +build v2 There are obviously wrinkles here. For example, the build tags for an import must appear only once (or must not conflict), but that kind of situation already exists for the custom import path restriction comments. There are other build tag persistence mechanisms available; this is just to illustrate. (Please don't bikeshed with new ideas for build tag persistence here. That would be a separate proposal. The important point is that there are options.)
I'm not sure where I stand on aliases vs build tags, but it seems worth exploring a little. |
I see two drawbacks to this feature: 1.) It breaks the "different names means different types" invariant that has existed since Go 1.0. type foo struct { ... }
type bar => foo bar is foo. It's not just the same shape in memory as foo, it's not just compatible with foo's methods. They are the same type. Something that takes a 2.) When this is used in practice, the aliases will almost certainly live on forever. The whole reason you'd use an alias is because the refactor would be too cumbersome without it and/or you just don't control the other code referencing these types. This means that large swaths of the codebase will be referencing these aliases, and then any new code has to make a choice - use the new "real" location of the types, or use the aliases to conform to the way the rest of the codebase looks. In practice, I think this feature will lead to confusion, since this is mostly useful for large codebases with lots of developers, and developers will constantly trip over two pieces of code that look like they're using different types, but really are not. For an example, consider how many people have asked how to convert int32 to a rune, or a string into a []byte (the latter is not a perfect example, but close enough for illustration).. and those are built into the language. However, I'm not totally against it. This would have been hugely useful in one specific case I had in Juju. I had a to do a massive refactor to move a very commonly used package outside of the main Juju repo for use in a separate repo. The work was delayed by months because I had to touch so many files that I inevitably had a million conflicts every time I updated from master. If all I had to do was make an alias in the original place, all those conflicts would not have happened, and in theory, we could have done the conversion incrementally (though, see number 2 above, I don't think we actually ever would have put forth the effort). |
TLDR I'm a -1. So I have some thoughts we discussed in person but I thought I would put them here to make sure they are taken into account by all members: 1.) I really think with the current proposed implementation this could be easily abused. cc @kelseyhightower who also had some thoughts I can't express as eloquently as he can |
For sufficiently large codebases, type aliases aren't just a convenience. It's effectively impossible to refactor types without them. It's not uncommon for us to make changes to packages which are imported by hundreds or thousands of files spread across many different projects. The usual process when making an incompatible API change is to initially support both the old and new interfaces, gradually update all the uses of the old interface to the new one, and finally drop the old interface. We've got tools which automate parts of this process. It is impossible to move a type from one package to another using this approach, however, since it is not possible to have a transitional period where the type exists in both places.. Build tags as suggested by @josharian are unfortunately not a solution, since they would require that all the components of a program agree on a single location for the type. The inability to move types in widely-used packages is causing us ongoing pain, and I don't see any way to resolve it without some form of type aliasing. |
Thanks to everybody for providing this extremely valuable feedback. This is exactly what we (the Go team) were hoping for. Please keep it coming; especially if you have specific comments that have not been voiced so far. Please use the +1/-1 emoticons to express general support or disagreement. Thanks. Here is some feedback regarding the comments so far. @Merovius: The unsafe functions are actually built-in functions and we also disallow aliasing for built-in functions. Some of them (in fact all the unsafe built-in functions) have a "generic" signature. If it were possible to alias them, they could potentially be exported, and that would require special export/import machinery since they don't have a signature that can be expressed in Go. It does not seem worthwhile adding complexity for a mechanism that is questionable at best (aliasing unsafe functions). The reason we allow aliasing for unsafe.Pointer is that it's already possible to define a type that has unsafe.Pointer as underlying type. This is all outlined in the design doc. @matttproud, @cespare, @josharian, and others: A lot of people have raised concerns about using @josharian: It is probably a good idea to restrict alias declarations, at least to start with. We can always remove restrictions down the road if necessary. I am inclined to say we should allow alias declarations for imported objects only (so no local aliases). Restricting them to be placed at the top-level only would be a secondary restriction in my mind. I'm not sure it's needed if we have the former restriction. @josharian: I believe you're right that it's going to be a bit tricky to get the compiler to give good error messages in the presence of aliases. But that's mostly an implementation detail. There are probably also a lot of issues that go doc and friends need to handle. That said, I believe this is something that can be refined over time w/o impacting the fundamentals of the proposal. @dlsniper: Your suggestion of writing @Kunde21: We have spent several hours playing with different notations, and @Kunde21, @mem: Regarding chains of aliases: That is a very valid concern. We should perhaps start with disallowing aliases to aliases. It's a simple, easy to understand restriction, doesn't limit the proposal much, and can be trivially lifted if need be down the road. @Kunde21: Allowing aliasing only internally defeats the purpose of the proposal (but perhaps I misunderstood your comment). @Kunde21: There's a lot we can do with go vet/lint, goimports, and friends. Let's discuss this as a separate issue. @jimmyfrasche: If we restrict aliasing to imported objects only, I believe your concerns are addressed. That is, an alias can only export something that was exported before (since it can only alias something that was imported). @mem: Using aliases to export objects of internal objects seems like a very important alternative use case. This would still be possible to do if we only permit aliases to exported objects. @josharian: Adding methods to a type via its alias name in the receiver specification seems like it should be possible if the alias refers to a locally declared type since the alias really just means that type. It should not be legal if the type is imported (it's not legal now). But it we restrict aliases to imported objects only, this question becomes moot. @zellyn: I think @zellyn: Several people have expressed concerns abut exposing private things via exported aliases. Let's start small. @leighmcculloch: I believe your questions have been answered by others. I'm not sure versioning is helping here (see @zellyn's feedback). Renaming of packages may work in some cases but not others. @stephens2424: You are correct that one has to be very careful about not creating circular dependencies. I have hinted at that when discussing the work-around for variables. I think aliasing will make it possible to avoid circles which might occur if only other work-arounds were available. @perses: We did think about a special notation for variables only; including the possibility of making @Merovius: Agreed. There is strong sentiment in favor of not allowing aliases to export unexported objects. @atdiar: Thanks for your feedback. I believe many of your concerns have been brought up by others as well. Perhaps you can elaborate on the relationship to the "functor" mechanism of OCaml (with which I and probably many others are not familiar with). @natefinch, @jfrazelle: Your concerns have been echoed by many people. I think we do need to start with a smaller and restricted proposal and take it from there. I will update the proposal to take this feedback into account. |
That restriction fixes my second concern, but, even with that restriction, you could create an unexported alias of an exported type at top level. If you have
and you import B from your package A, the rules available for construction of a value of type unexported change based on whether you also import C, which gives you direct access to C.Exported, but unexported won't show up in godoc so it changes silently. A lot of bad decision dominoes need to fall to get into that situation, but it seems like a recipe for confusion and edge cases. |
@griesemer To clarify my suggestion about restricting internally, this would only be for defining an alias. If we're moving // Within the same repository
import "vcs/user1/library/subpkg2"
type OldType => subpkg2.NewType
// Or within the user's tree
import "vcs/user1/newlib"
type OldType => newlib.NewType Consumers of the library would still see: // Unchanged from before the alias definition
import "vcs/user1/library/subpkg"
var myvar subpkg.OldType What I'm looking to avoid is allowing anyone to create aliases into someone else's code. This would still require users to fork the base library, in order to refactor a type out of it: // package vcs/user1/mylib
import "vcs/user2/YourLib"
type MyType => YourLib.YourType // Error: Alias can't be created. |
Reason: Decision to back out current alias implementation. Leaving import/export related code in place for now. For #16339. TBR=mdempsky Change-Id: Ib0897cab2c1c3dc8a541f2efb9893271b0b0efe2 Reviewed-on: https://go-review.googlesource.com/32757 Reviewed-by: Robert Griesemer <gri@golang.org>
CL https://golang.org/cl/32827 mentions this issue. |
This reverts commit aa8c8e7. Reason: Decision to back out current alias implementation. For #16339. Change-Id: I4db9a8d6b3625c794be9d2f1ff0e9c047f383d28 Reviewed-on: https://go-review.googlesource.com/32827 Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Manghane <cmang@golang.org>
This reverts commit 59c63c7. Reason: Decision to back out current alias implementation. For #16339. Change-Id: Idd135fe84b7ce4814cb3632f717736fc6985634c Reviewed-on: https://go-review.googlesource.com/32822 Reviewed-by: Chris Manghane <cmang@golang.org>
This reverts commit 57ae833. Reason: Decision to back out current alias implementation. For #16339. Change-Id: I7bcc04ac87ea3590999e58ff65a7f2e1e6c6bc77 Reviewed-on: https://go-review.googlesource.com/32823 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit 776a901. Reason: Decision to back out current alias implementation. For #16339. Change-Id: Icb451a122c661ded05d9293356b466fa72b965f3 Reviewed-on: https://go-review.googlesource.com/32824 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit 32db3f2. Reason: Decision to back out current alias implementation. For #16339. Change-Id: Ib05e3d96041d8347e49cae292f66bec791a1fdc8 Reviewed-on: https://go-review.googlesource.com/32825 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reason: Decision to back out current alias implementation. For golang/go#16339 (comment). Change-Id: Id2a394d78a8661c767bcc05370b81f79d9bfb714 Reviewed-on: https://go-review.googlesource.com/32756 Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Chris Manghane <cmang@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
As I said on a thread in golang-nuts, I'm in favour of type-only aliases with the "type S = T" notation, as consts can easily be done anyway, mutable global variables are a smell and that notation is by far the most obvious. One other issue occurs to me: There's another issue that comes to mind though - which value should be used for Type.PkgPath? It wouldn't be surprising if someone were checking for specific type by looking at the combination of that field and the type name. For example:
A quick grep through the packages in my GOPATH found me a couple of places |
Off the top of my head, that shouldn't be an issue since this field is the empty string unless exported. I'd like to emphasize, again, that I don't see the context move as the best use case to present for this proposal. The original use case @gri presented seems more befitting. The only thing is that, strictly speaking, an API is refactored; not so much the code/text. That's if we care about compatibility (backward and forward). As seen in the context case, build tags are required which has repercussions on ABI compatibility (for people who do not publish their source code, that could have implications). |
If type T = otherpkg.S is really to assist package refactoring, then I suggest that we add the following two requirements:
|
That kind of local aliasing isn't just for "lazy typists". If you're dealing with a generated API (e.g. a protobuf package), you can easily end up with a name like |
@trebe Refactorings are not the only use case mentioned in this proposal for aliases (which is a strength of the proposal. It means it solves several different problems). Two other use cases mentioned (so far) where a) implementing protocol buffer public imports and b) allowing drop-in augmentations of existing packages (like x/image/draw). There might be others that I'm missing right now. |
@Merovius Aliases were also proposed as a way to design a new API that is split into sub-packages for organizational purposes, while the clients need not care about those sub-packages. See #16339 (comment) (not something I've personally run into yet) |
CL https://golang.org/cl/33019 mentions this issue. |
They interfere with gofmt -w across this directory. Follow-up on https://go-review.googlesource.com/32819. For #16339 (comment). Change-Id: I4298b6117d89517d4fe6addce3942d950d821817 Reviewed-on: https://go-review.googlesource.com/33019 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
I was talking about the value returned by the Type.PkgPath, not the value in the StructField. This is always non-empty for named non-built-in types. |
@rogpeppe You're right, confusion is mine ( was thinking of Method.PkgPath) In any case, since this is a compile time mechanism only, I think that the PkgPath should be the one corresponding to the package in which the object definition was written. |
CL https://golang.org/cl/33365 mentions this issue. |
(Revert of https://go-review.googlesource.com/#/c/32310/) For #16339. Fixes #17975. Change-Id: I36062703c423a81ea1c5b00f4429a4faf00b3782 Reviewed-on: https://go-review.googlesource.com/33365 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Locking because aliases are no longer proposed. |
Although aliases were not adopted for Go 1.8, we still want to solve the underlying problem for Go 1.9. I’ve filed a new issue #18130 to start a discussion about our options. Please see that issue and the corresponding background article “Codebase Refactoring (with help from Go)” for more information. Thanks. |
Abstract
We propose to add alias declarations to the Go language. An alias declaration introduces an alternative name for an object (type, function, etc.) declared elsewhere. Aliases simplify splitting packages because clients can be updated incrementally, which is crucial for large-scale refactoring.
Motivation
During refactoring, it is often desirable to split an existing package into multiple packages. Exported objects such as types, functions, etc. that move from one package to another will require clients to be updated accordingly. In a continuous build environment, build breakages are the consequence if not all clients can be updated simultaneously.
This is a real issue in large-scale systems such as we find at Google and other companies because the number of dependencies can grow into the hundreds if not thousands. Client packages may be under control of different teams and evolve at different speeds. Updating a large number of client packages simultaneously may be close to impossible. This is an effective barrier to system evolution and maintenance.
Go trivially permits constants to refer to other constants, possibly from another package. Similarly, if a function moves from one package to another, a “forwarder” function that simply calls the moved function may be left behind so clients can continue to refer to the old package. These techniques enable incremental update of clients without breaking a continuous build system.
No such work-around exists for types and variables. To address the issue, we propose the notion of a general alias declaration. An alias declaration introduces an alternative name for an object (constant, type, variable, or function) declared elsewhere, possibly in another package. An alias may be exported, so clients can refer to on object via the package that declares the object, or any package exporting an alias to an object, and get the same object.
Alias declarations are designed such that they fit seamlessly in the existing language spec without invalidating backward-compatibility or any other aspect of the Go 1 guarantees. Tools that process Go code which will require changes to support alias declarations.
Alias declarations are a compile-time mechanism and don’t incur any runtime costs.
The design document (forthcoming) describes alias declarations in detail.
Added July 25, 2016: Link to design document: https://github.com/golang/proposal/blob/master/design/16339-alias-decls.md
The text was updated successfully, but these errors were encountered: