proposal: spec: use zero receiver for value method embedded via nil pointer #18617

Open
bcmills opened this Issue Jan 11, 2017 · 11 comments

Comments

Projects
None yet
6 participants
@bcmills
Member

bcmills commented Jan 11, 2017

Consider this program:

package main

import (
	"fmt"
)

type Pointer struct{}

func (d *Pointer) IsNil() bool {
	return d == nil
}

type Danger struct {
	Pointer
}

func main() {
	var d *Danger
	fmt.Println(d.IsNil())
}

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 is nil.

Specifically, I propose to add the following sentence to https://golang.org/ref/spec#Struct_types:

  • If S contains an anonymous field *T, the method sets of S and *S both include promoted methods with receiver T or *T. Evaluating a call or method value of a promoted method with a nil *S receiver evaluates the corresponding *T method with a nil *T.

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.

@rakyll rakyll added the Proposal label Jan 11, 2017

@rakyll rakyll added this to the Proposal milestone Jan 11, 2017

@rogpeppe

This comment has been minimized.

Show comment
Hide comment
@rogpeppe

rogpeppe Jan 11, 2017

Contributor

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.

Contributor

rogpeppe commented Jan 11, 2017

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.

@bcmills

This comment has been minimized.

Show comment
Hide comment
@bcmills

bcmills Jan 11, 2017

Member

@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)

Member

bcmills commented Jan 11, 2017

@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)

@bcmills

This comment has been minimized.

Show comment
Hide comment
@bcmills

bcmills Jan 11, 2017

Member

To give a concrete use-case: golang/protobuf#276 is my motivating example for this proposal.

Member

bcmills commented Jan 11, 2017

To give a concrete use-case: golang/protobuf#276 is my motivating example for this proposal.

@bcmills bcmills changed the title from proposal: Pointer methods on embedded structs should receive nil instead of panicking. to proposal: Embedded pointer methods should not panic on nil receivers. Jan 12, 2017

@bcmills bcmills changed the title from proposal: Embedded pointer methods should not panic on nil receivers. to proposal: Resolving embedded pointer methods should not panic on nil receivers. Jan 12, 2017

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Jan 23, 2017

Contributor

@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.

Contributor

rsc commented Jan 23, 2017

@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 rsc changed the title from proposal: Resolving embedded pointer methods should not panic on nil receivers. to proposal: spec: use zero receiver for embedded value receivers called using outer nil pointer struct Jan 23, 2017

@bcmills

This comment has been minimized.

Show comment
Hide comment
@bcmills

bcmills Jan 23, 2017

Member

@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.

Member

bcmills commented Jan 23, 2017

@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.

@rsc rsc added the LanguageChange label Jan 23, 2017

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Jan 23, 2017

Contributor

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.

Contributor

rsc commented Jan 23, 2017

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.

@rsc rsc added the Go2 label Jan 23, 2017

@bcmills

This comment has been minimized.

Show comment
Hide comment
@bcmills

bcmills Jan 23, 2017

Member

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.

Member

bcmills commented Jan 23, 2017

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.

@rogpeppe

This comment has been minimized.

Show comment
Hide comment
@rogpeppe

rogpeppe Jan 24, 2017

Contributor

@rsc done #18775

Contributor

rogpeppe commented Jan 24, 2017

@rsc done #18775

@LionNatsu

This comment has been minimized.

Show comment
Hide comment
@LionNatsu

LionNatsu Jan 25, 2017

Contributor

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.

Contributor

LionNatsu commented Jan 25, 2017

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.

@bcmills

This comment has been minimized.

Show comment
Hide comment
@bcmills

bcmills Jan 25, 2017

Member

@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.)

Member

bcmills commented Jan 25, 2017

@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.)

@LionNatsu

This comment has been minimized.

Show comment
Hide comment
@LionNatsu

LionNatsu Jan 25, 2017

Contributor

C++ does not have any feature that is particularly close to Go's embedding.

It is not completely different. Let's say there's a class has the structure we are going to embed at the very beginning, we can tell there is no offset, so the pointer to this class can be safely convert to the embedded one's pointer, and then you can use its member functions.

In particular, member functions in C++ do not allow null this pointers.

Well, it's true. It needs to check the inner structure to find the function. My apologies.

Contributor

LionNatsu commented Jan 25, 2017

C++ does not have any feature that is particularly close to Go's embedding.

It is not completely different. Let's say there's a class has the structure we are going to embed at the very beginning, we can tell there is no offset, so the pointer to this class can be safely convert to the embedded one's pointer, and then you can use its member functions.

In particular, member functions in C++ do not allow null this pointers.

Well, it's true. It needs to check the inner structure to find the function. My apologies.

@rsc rsc changed the title from proposal: spec: use zero receiver for embedded value receivers called using outer nil pointer struct to proposal: spec: use zero receiver for value method embedded via nil pointer Jun 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment