Open bcmills opened 7 years ago
I've wondered about a related proposal for a while (I'm not sure if it covers this too): change the semantics so that if a value method is declared on a pointer type and the method is called on a nil pointer, the method would receive the zero value rather than panicking. This would make declaring value methods more useful and would get around the current fact that you can't have (for example) a String method that's declared on the value type but works without panicking when called on a nil pointer.
@rogpeppe
change the semantics so that if a value method is declared on a pointer type and the method is called on a nil pointer, the method would receive the zero value rather than panicking.
That may be an interesting idea for a potential Go 2, but it would break a lot of fmt.Stringer
implementations in Go 1. (https://play.golang.org/p/krlvX0wbO5)
To give a concrete use-case: https://github.com/golang/protobuf/issues/276 is my motivating example for this proposal.
@bcmills
it would break a lot of fmt.Stringer implementations
I think you mean "fix". In your playground snippet, which prints:
*main.ValueStringer(<nil>)
*main.ValueStringer()
the <nil>
is not coming from the fmt.Stringer at all but instead is the fmt package covering up the fact that the String method panicked.
Compare with https://play.golang.org/p/4gWM_cFVNM.
@rsc
I think you mean "fix".
I think I really mean "produce non-backward-compatible changes in".
I would like to consider the zero-receiver-forwarding proposal (this proposal) separately from @rogpeppe's suggestion for value-receivers in general. I do not think there are many real-world programs that this proposal would change, but I suspect that there are many such programs impacted by the broader value-receiver change.
That is: I think this proposal is minor enough to be more-or-less compatible with Go 1, whereas @rogpeppe's proposed extension is invasive enough that it would need to wait for a Go 2.
One potential question here is that d.IsNil() is just shorthand for d.Pointer.IsNil(). Does the latter also not panic?
@rogpeppe, can you file a new issue with your proposal?
We probably should leave both of them open for an eventual discussion for Go 2.
One potential question here is that d.IsNil() is just shorthand for d.Pointer.IsNil(). Does the latter also not panic?
It does panic, but the panic is arguably less surprising: d.IsNil()
calls a method in the method set of d
, so at the call site it does not look like "accessing a field of d
".
In the embedded-call case, the fact that it is a field access and not only a method call is not visible in the caller-side code.
Under this proposal, d.IsNil()
would be changed to forward a nil receiver, but d.Pointer.IsNil()
would continue to panic as it does today.
I think the key of this issue is that:
d.IsNil()
is the shorthand for d.Pointer.IsNil()
.
I'm not good at terms, but if I write it in C++, it should be something like:
d->Pointer::IsNil()
The operator ::
does not access anything but just pass the object before it as the 'this pointer' to the function (aka 'thiscall').
But operator ->
accesses the pointer before it which MUST NOT be a null pointer.
Thus the dilemma here is to tell the difference of d::IsNil()
and d->embeddedMember::IsNil()
which are both wrote as d.IsNil()
in Go, that the latter needs a non-nil pointer but the former doesn't care.
@LionNatsu I don't think it's all that helpful to reason by analogy to C++ here: C++ does not have any feature that is particularly close to Go's embedding.
(Specifically, member functions in C++ do not allow null this
pointers.)
If we adopt this it would change the behavior of existing code. That is, the exact same code would compile and run before and after making this change to the language, but it would behave differently. That makes it difficult to implement according to the guidelines at https://github.com/golang/proposal/blob/master/design/28221-go2-transitions.md . I'm not sure the benefit of making this change is worth the cost of having the same code behave differently in different versions of Go.
As an alternative that does align with the guidelines from #28221, we could prohibit embedding entirely for value types with pointer methods.
That would make the Danger
struct in the example a compile-time error, which the developer could then resolve by adding an explicit wrapper for the IsNil
method or embedding a pointer instead of a value.
Am I mistaken, or does using a pointer receiver not help? There's still a panic when trying to look up the value of the field.
Consider this program:
If a struct type embeds another struct type with pointer methods, the pointer method set of the outer struct (
*Danger
in this example) includes the pointer methods of the embedded value.However, calling these embedded methods is dangerous: the mere act of computing the address of the receiver results in a nil panic (see https://play.golang.org/p/jfrCruVC6l). (You can more clearly see that the panic occurs when computing the receiver address by using a method expression instead of actually calling the method: https://play.golang.org/p/629rZ7rs6l.)
This somewhat limits the usefulness of embedding for composition, as the caller must either know the concrete type in which the struct is embedded (https://play.golang.org/p/3ciUa4kiOQ) or use reflection to check whether the concrete pointer is nil before making the call (https://play.golang.org/p/zGDh6Hk5U9).
This style of embedding could be made significantly more useful by defining pointer methods on embedded structs to receive
nil
if the pointer to the struct in which they are embedded isnil
.Specifically, I propose to add the following sentence to https://golang.org/ref/spec#Struct_types:
The spec is currently a bit vague on the exact semantics of calls to methods obtained by embedding. This proposal certainly represents a change to the language as implemented, but it is not obvious to me whether it is a "language change" in the Go 1 compatibility sense or merely a spec clarification.