-
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: Go 2: permit types to say they may only be created by containing package #43123
Comments
For language change proposals, please fill out the template at https://go.googlesource.com/proposal/+/refs/heads/master/go2-language-changes.md . When you are done, please reply to the issue with Thanks! |
@gopherbot please remove label WaitingForInfo |
@timruffles You haven't filled out the template. |
@ianlancetaylor yes I have, I should have said explicitly. It's near the top of the proposal: |
Ah, sorry, I did miss that. |
I really like this proposal and think that it solves the "use my constructor" problem in a very good manner. However, here is some criticism: Zero valuesZero values for PCTs should always be valid. Forcing the zero value to be valid for a PCT resolves a lot of the "strange" parts about this proposal (ie map/slice/new/etc not working). This also helps keep maintainers consistent with the idea that every type's zero value should be useful. SyntaxThe example given introduces a new keyword and (in my opinion) doesn't work well.
The reason that I don't think it works well is that anything to do with a type should be defined alongside the type itself, for instance something like UnmarshallingI think you were a bit strict with your sentiment that PCTs should never be unmarshalled. PCTs which specify their own unmarshallers (ie by implementing OperatorsProbably good to specify that many operators are invalid on PCTs (ie |
Here's a restatement of the problem you're trying to address: How can we define a user-defined type and ensure that any value of such a type satisfies certain user-defined invariants (outside the package defining the type)? As you have already shown, existing Go mechanisms do not fully address this problem. @deanveloper pointed out additional issues. Here's another one: Once I have a variable Given the restrictions you already mention for channel and map types, why aren't there any restrictions for the use of "PCT"s in other composite types? What about the zero value of PCTs? |
@deanveloper thanks for taking the time to read the proposal! Operators - thanks for pointing out they should be clarified. I've taken your syntax suggesting on board, but reversed the order to Unmarshaling - the proposal doesn't suggest PCTs are unmarshalable, it suggests that unmarshaling code can be made PCT aware by using:
Zero-values: your suggestion seems impossible to me. This proposal is about language level guarantees. The language can't know if your application invariants can be supported by Go's zero-value semantics. |
@griesemer thanks for reviewing!
Agreed 👍
I think it's fine to allow copies. You're right that explicit at-most-N-values-of-type constraints can't be enforced, but I think keeping the proposal simple is preferable.
I had examples of struct and channel behaviour, but I can see my examples were ginormous and hard to parse. I've split them up to be clearer - channels, structs.
Not sure I understand you here, but it's an explicit goal of this proposal to make it impossible to create zero-values of a PCT outside of its defining package. That's behind the restrictions on initialization, and on maps, slices and channel element types; structs; and operators (thanks @deanveloper ). Without these restriction I can't see any way to achieve the desired benefits. |
It's not about parsing, it's about readability and consistency. The first word of a top-level declaration is always the thing being declared.
What I am suggesting is that the language forces the zero-value of a type to always be valid. The package maintainer should make sure that the zero value of the type is always useful. The zero value is predictable, so the maintainer knows what needs to be done in order to assure that it still always works. Forcing zero-values of PCTs to always be valid makes many parts of this proposal more appealing. A type that can't be used in maps, lists, struct members, etc. isn't appealing to me, but all of these issues are solved if the zero value is forced to be valid. |
As discussed above, this proposal does not support zero values, and does not provide any special mechanism for copying a value (or avoiding copying a value). Without those features this functionality is not a good fit for the language and would be quite difficult to use in practice. Therefore, this is a likely decline. Leaving open for four weeks for final comments. |
All that's required is to remember:
I interact with multiple codebases where an attempt is made to follow those rules already, for a subset of types.
This is symmetrical with the statement, 'this proposal enforces use of constructors outside the package'. Zero values are fine within the package. |
I'll have a think about this. But to me if copying is a problem, mutability would surely be a more pressing concern (i.e there's nothing to stop someone zeroing out all the fields of any type, or making them invalid)? Given that this proposal already accepts that mutability is a fact of Go, I think it's a reasonable omission. I can't see that restrictions on copying are something anyone is crying out for; indeed, it only seems to have been mentioned in feedback to this proposal. |
Correct - but the underlying issue of both of these statements is "i can't use the values in slices, map values, structs, or channels" |
Another issue relating to zero values:
|
@deanveloper give the semantics of type assertions, it's fine to have it as a zero-value of the the PCT. Sure, it's a way to create zero values of the PCT outside the package but I don't think it conflicts with its goals in practice. Generally you don't use the value of a type assertion when it is not successful - I've not seen it done once. Again, you certainly can use PCTs in maps etc, you simply use a pointer to them rather than directly referencing them. |
@ianlancetaylor just in case there is an ambiguity, this proposal does not require special behaviour for zero values of PCTs. So it certainly 'supports' zero values, and has carefully considered how to avoid changing their semantics. It is simply a way to opt in to compiler checking to ensure you don't create zero-values outside of the defining package (besides type assertions which I've addressed above). Another way of looking at it is that you can already write a program that acts as if you had marked a subset of its types as package created in Go right now. This proposal just makes it easier by having the compiler check you're doing so. |
One practical concern I'm having with regards to this, is that I suspect most codebases would immediately start using this feature by default for most types, as it's easier and may feel "safer" to "seal" a type by default in such a way than to try and think about zero-values. Which I believe would result in a major de facto change in semantics/look-and-feel of "typical" Go code. I'm not sure this would be necessarily a bad change, more that currently it's hard for me to imagine how such a language would handle on a daily basis, assuming most types were "sealed" by default. |
@timruffles The thing is, as @deanveloper has been saying, zero values are really deeply baked into Go. If you can't create a zero value of a type, there are very basic things you can't do, like |
@ianlancetaylor the benefit is the same as Go's compiler rejecting The proposal clearly explains how the types can be used with slices etc. Namely, you use a pointer to the type to ensure no zero-values are created: intvs := []*interval.Natural{}
intvs = append(intvs, interval.NewNatural(1,2)) Again, this is something you could do with the language currently, this proposal just enforces the pattern (much like you can simply attempt to avoid mixed type arithmetic in JS: it still ends up happening in large programs).
The proposal already covers why I don't think they're acceptable alternatives. |
The fact that there is a workaround to avoid generating zero values doesn't change the fact that ordinary Go usage requires that it be possible to create zero values. In effect types that use this approach can't be used in the same way as ordinary Go types. That is a heavy cost. The benefit we get here does not outweigh that cost. |
No change in consensus. |
It would be useful to permit types to say they may only be initialized by their defining package, for two reasons:
Currently, it's easy for create values of either of these that violate their invariants, especially via zero-value initialization, and that allows bugs to creep in. Existing ways to attempt to prevent invalid values being created are limited and unidiomatic.
This would be a backwards-compatible change - only programs that opted a type into this new behaviour would be affected.
This proposal calls this feature 'package created types', or PCTs.
Answers to language-changes.md questions.
Unmarshal
function (to error on package created types that lack one)Answered by the proposal:
Motivation
Let’s consider an example of a data type that represents a "natural interval" defined as an interval with the invariants
From > 0
andTo > From
.Currently, a function accepting an
interval.Natural
can’t guarantee it is valid according to the invariants above. Invalid or zero interval values can be created outside the package and passed in a number of ways:The author of
interval
could define a validating constructor, which would only return valid natural ranges. However, there is no way to require it's always used, and thus it doesn't provide any guarantees.If it was possible to know that every
interval.Natural
value had been created by theinterval
package, thewithInterval
method would be able to rely on theinterval
package having been able to enforce its invariants at creation time (if the underlying type is immutable, e.g an integer, the guarantees are stronger still).Enums
Enums are generally new types over an underling primitive, e.g an
int
orstring
:Again, this doesn't provide safety if other packages embed, or explicitly create, enum values:
Therefore, code working with enums can't be sure they are valid.
Desired behaviour
In this example,
interval.Natural
andinterval.Kind
are types whose values must be initialized by its package:the examples below assume the following code:
Initialization #
PCTs cannot be initialized outside their defining package, which restricts explicit and implicit initialization:
new
can only be passed a package-created-type in its defining package.The examples above in which attempts were made to initialize non-zero values of
Interval
can be achieved via the exported constructor. The attempts to initialize zero-values of PCTs outside their defining package are impossible, an explicit goal of this proposal:Structs #
Types that have a PCT as a field must explicitly initialized those fields. Thus any struct that contains a package created type will become liable to similar restrictions on its initialization:
Maps, slices and channels #
Package created types must use pointers to be valid map values, or slice/channel element, i.e
[]*interval.Natural
is okay, but[]interval.Natural
is not. This ensures the semantics of maps, slices and channels are unaffected by this proposal, while achieving its goal of restricting value initialisation to the defining package.To fix the errors the author would need to do use pointers to the PCT as the element type:
Operators #
Where a package-created-type's underlying type is a type on which operators are valid (e.g, an int, rather than a struct), it will be a compiler error to attempt an operation with a PCT on the left-hand side (which determines the type of the expression evaluation):
Mechanics
Indicating a type is package created
This proposal has no opinion on the best syntax to indicate a given type in a package should have 'package created type' behaviour. A indicative proposal would be prefixing indicating a type was package create by preceeding it with the package keyword:
Unmarshalling
Unmarshalling packages could be made package created types (PCT) aware. They’d use reflection to check whether a given component of a unmarshalling target was a PCT, and return an error if that type did not export an appropriate unmarshalling function.
Reflect
Reflect would be extended with:
Reflect's
New(typ Type)
function, and others, would be updated to panic on invalid creation of zero values (similar to panicing on accessing unexported struct fields), or of zero values of types that reference PCTs (this would need to be a recursive check, e.g A has a field of B that has a field of C that's a PCT, means you can'tNew()
a A).Compiler implementation
I don't know enough (anything) about Go's compiler to have any opinions on how this could be implemented. I assume it would involve updating the code that compiled/type-checked initializations.
Issues with alternatives #
Unexported types
Unexported types cannot be stored in structures, maps, or slices in other packages:
For unexported structs with exported methods there is a workaround - an appropriate interface can be defined on the consumer side and used to store the values. However, there is nothing to stop other packages defining methods with the same names, and again breaking the invariants.
Further, interface wrapper and accessors are not idiomatic. In Go, data and enums are usually concrete and access is not mediated via getters, unlike Java etc.
If the defining package of the unexported type doesn't define methods, the consuming package would have to wrap the values in an
interface{}
:This is far from idiomatic, and imposes runtime cost, and risk of panics.
Interfaces
Interfaces with unexported methods prevent other packages from defining their own implementation. This can get closer to guaranteeing a value was created by its package.
Again, however, this has meant data and enum members have to be wrapped in an interface value. That isn’t idiomatic in Go - data is typically ‘raw’, without indirection. It also incorrectly suggests to readers it is an abstract data type, which is a more common use for interface-wrapped data in Go. And we've now had to define a nearly identical struct and interface.
Discipline: always use package's constructor
Practically today, many codebases would define
NewNatural(from, to int) (interval.Natural, error)
and trust thatNewNatural
is always used, and that zero or partially initializedinterval.Natural
values are never created.However, this isn't a solution, it's accepting the status quo. It's equivalent to arguing that Go's memory safety isn't valuable as you can "just always remember to check your bounds". In a large codebase, with many engineers, the probability of such rules being violated somewhere approaches 1.
Linting
Linting can implement some of the above (though not changes to stdlib marshaling/unmarshaling), but it'd be faster and more widely supported if it was a compiler feature.
Relation to other proposals
The text was updated successfully, but these errors were encountered: