Is "When in doubt, use a pointer receiver" misleading advice?

[Brad Johnson]

Google's style guide is a popular reference for Go programmers. In it, they list off a number of scenarios where one would use a value receiver vs a pointer receiver. But, ultimately, they end the list with "when in doubt, use a pointer receiver".

In my experience, I've noticed the majority of Go programmers I encounter default to using pointer receivers in all circumstances. And I have a hard time justifying any protest because the scenarios typically don't fall into the "approved" set where value receivers are recommended.

I also notice that my IDE (GoLand) defaults to returning a pointer to a struct when I use its "Generate Constructor" auto-complete functionality. I can't help but suspect this was motivated by the "when in doubt" advice from Google.

I'd be curious to hear thoughts on this topic. I tend to advise developers to default to value receivers because I perceive benefits to avoiding nil pointer exceptions. But it's hard to substantiate my advice as anything idiomatic.

[Oliver Lowe]

> I'd be curious to hear thoughts on this topic.

There was a fun talk at GopherConAU just a few days ago: "What's
The Point? A Guide To Using Pointers Without Panicking" (talk
description at https://gophercon.com.au/) When the recordings are
all finalised I can reply to this thread with a link.

[Mike Schinkel]

That's a really good question, IMO.

I have been wondering for a while why the advice against mixing pointer and value receivers, which GoLang so often flags me for doing.

Ideally I think that I would like to be able use value receivers most of the time when I want the method to leave the state unmodified, and only use pointer receivers when I want state to be modified.  And, I don't want to have to ignore GoLand's warnings or have a CI/CD linter warn me about it considering that I want to do it on purpose.  I could turn off the warning in GoLand, but that doesn't change coding standards on teams I might work on who religiously follow the Go team's recommendations.

Given this thread I finally did some research. Tracking it down in the Go FAQ it says:

> Next is consistency. If some of the methods of the type must have pointer receivers, the rest should too, so the method set is consistent regardless of how the type is used. See the section on method sets for details.

Tracking down method sets it says (abridged):

> the method set of a type T consists of all methods with receiver type T, while that of the corresponding pointer type *T consists of all methods with receiver *T or T. That means the method set of *T includes that of T, but not the reverse.
> This distinction arises because if an interface value contains a pointer *T, a method call can obtain a value by dereferencing the pointer, but if an interface value contains a value T, there is no safe way for a method call to obtain a pointer. (Doing so would allow a method to modify the contents of the value inside the interface, which is not permitted ...)
> Even in cases where the compiler could take the address of a value to pass to the method, if the method modifies the value the changes will be lost in the caller. ... This is almost never the desired behavior.

I decided to so some experimentation and turns out it a compile error occurs when a value variable is used with a pointer receiver:  

- https://goplay.tools/snippet/_AiFRsc9w-r

There are six (6) permutations of an interface and the variable of the interface type, two (2) of which will not compile:

1. A value variable and multiple value receivers  <--- compiles
2. A pointer variable and multiple value receivers <--- compiles
3. A pointer variable and multiple pointer receivers.  <--- compiles 
4. A value variable and multiple pointer receivers.  <--- will NOT compile
5. A pointer variable and mixed value+pointer receivers  <--- compiles 
6. A value variable and mixed value+pointer receivers. <--- will NOT compile

Permutation #4 and #6 are consistent with the description above, and they both have a value variable in common.

However, given than #5 DOES compile, I was left wondering why the standard that mixed receivers should be flagged as an error? 

I ask rhetorically — or better yet, to the Go team — shouldn't mixed receivers be considered acceptable, especially when they would allows ensuring the receiver is not changed by the method that should not change it? And then shouldn't the guidance in the FAQ be to use value receivers unless the receiver needs to be modified within the method?

I do get that changing the guidance could violate the conceptual purity of the "method set" definition, but pragmatically is being able to mix receiver types not more useful in practice?  If the developer makes a mistake and passes a non-pointer interface value to a method with a pointer receiver they will get a compile error, so there does not appear to be any downside for changing the recommendation regarding consistency with method receivers?

Or is there something really obvious I am just missing?

-Mike

[Dan Kortschak]

On Mon, 2023-11-13 at 19:38 -0800, Mike Schinkel wrote:
> I have been wondering for a while why the advice against mixing
> pointer and value receivers, which GoLang so often flags me for
> doing.

https://dave.cheney.net/2015/11/18/wednesday-pop-quiz-spot-the-race

[Robert Engels]

To me this is a limitation of the compiler. If a passed argument is unused it doesn’t even need to be created. Similarly if only a few fields are used in a called function a sparse data object can be sent. It only becomes a race if the data being copied is actually used - then the race is also obvious.

> On Nov 13, 2023, at 10:27 PM, 'Dan Kortschak' via golang-nuts <golan...@googlegroups.com> wrote:

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/3cc8112142c151ffea7fe640a2e7150fafcf202d.camel%40kortschak.io.

[Robert Engels]

Similarly, even in a single threaded Java program objects are copied/moved behind the scenes - these don’t generate race conditions as the runtime ensures there isn’t one.

> On Nov 13, 2023, at 10:47 PM, Robert Engels <ren...@ix.netcom.com> wrote:
>
> To me this is a limitation of the compiler. If a passed argument is unused it doesn’t even need to be created. Similarly if only a few fields are used in a called function a sparse data object can be sent. It only becomes a race if the data being copied is actually used - then the race is also obvious.

[Robert Engels]

Lastly, the only reason that changing to a pointer receiver “solves” the race is because the field in the object isn’t accessed - something the compiler could detect in the other version as well.

> On Nov 13, 2023, at 10:50 PM, Robert Engels <ren...@ix.netcom.com> wrote:
>
> Similarly, even in a single threaded Java program objects are copied/moved behind the scenes - these don’t generate race conditions as the runtime ensures there isn’t one.

[Tamás Gulácsi]

I've always try to start with a value receiver,

change to pointer receiver (EVERYWHERE - do not mix pointer and value receivers!),
when the method modifies the sruct's members (or just have a sync.Mutex member),

or used as interface (i.e. error  - for comparison to nil)

[Vladimir Varankin]

> [..] when the method modifies the sruct's members (or just have a sync.Mutex member)

From my experience, these are the major points for defaulting to use the pointer receiver "when in doubt". The difference between "func (t T) Foo" and "func (t *T) Foo", is often too subtle to spot a bug in a code change of a real production project. This is for sure, not the ultimate solution for human-errors. But it's pragmatic.

[Mike Schinkel]

On Monday, November 13, 2023 at 11:28:00 PM UTC-5 Dan Kortschak wrote:

https://dave.cheney.net/2015/11/18/wednesday-pop-quiz-spot-the-race

Thanks for that link. 

However, after studying it for well more than an hour I cannot figure out why it is a data race, and unfortunately Dave Cheney didn't explain it in his post. 

Robert Engels seems to be saying this isn't conceptually a data race but it is an unfortunate artifact of how the compiler works?

-Mike

[burak serdar]

It is a data race because calling rpc.version() makes a copy of rpc,
which causes reading the field rpc.result concurrently while it is
being written by the goroutine.

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/241c13d1-9d9a-4028-9bce-ba856405f9edn%40googlegroups.com.

[burak serdar]

I do not agree that this is because how the compiler works. A value
receiver is equivalent to pass-by-value argument, that is:

rcp.version()

is equivalent to:

RPC.version(rpc)

thus, creating the copy of the rpc variable. So, the compiler may
choose to avoid the race by not copying it, or by inlining the version
function, but according to the spec, it is passed by value, i.e., it
is copied.

[Mike Schinkel]

On Tuesday, November 14, 2023 at 6:16:58 PM UTC-5 burak serdar wrote:

It is a data race because calling rpc.version() makes a copy of rpc,
which causes reading the field rpc.result concurrently while it is
being written by the goroutine.

 

Thank you for explaining. I think I am starting to see it.

On Tuesday, November 14, 2023 at 7:08:57 PM UTC-5 burak serdar wrote:

I do not agree that this is because how the compiler works. A value
receiver is equivalent to pass-by-value argument, that is:

rcp.version()

is equivalent to:

RPC.version(rpc)

thus, creating the copy of the rpc variable. So, the compiler may
choose to avoid the race by not copying it, or by inlining the version
function, but according to the spec, it is passed by value, i.e., it
is copied.

I agree with your assertion — if you fix it to work correctly by using RPC.version(*rpc)— but that is actually a different concern than I and I think Robert Engels were discussing.  

Your example above calls a method directly on the struct, we were discussing calling via an interface. 

Given that Go already performs some "magic" when it calls an interface — i.e. when the interface value contains a pointer and the method receive is a value — I think he was exploring whether it would be viable to avoid creating a copy when the method does not read or update any property.

Still, that doesn't seem to be all that useful because most of the time a value method would want to read a property so it is probably not worth the effort.

--------

But back to the original concern. It seems that race conditions do not arise when using mixed receivers while only executing the main goroutine, right?

And if using only pointer receivers then we can still have a data race if those methods are being called in multiple goroutines where methods both read and update the same properties, right?.  

Given that, the race conditions and the mixed pointers seem orthogonal concerns?   But I do now recognize that if you are using the same objects across goroutines and you have an updating pointer method then calling any value method would create a race condition so mixing receivers is more likely to cause a race condition when sharing objects across goroutines.

Still, it seems to me then that the guidance should be to not to access any object concurrently in more than one goroutine instead of to avoid mixed receivers, since that laters only addresses part of the race condition problem?  (BTW, by "not-concurrently" I mean that you could first access in goroutine 1 then pass it via a channel to goroutine 2 where you could access it, but once goroutine 1 submits the object to the channel it should no longer attempt to access it.)

Or again, am I missing something obvious?   

-Mike

[Robert Engels]

Switching to pointer receivers everywhere actually makes this worse. Any access is potentially a data race. 

It stills seems like this is a compiler issue. There needs to be a way to synchronize the pointer to value copy in conjunction with other synchronization. 

The only way to do this would be to write your own pointer to value conversion methods that have synchronization. 

On Nov 14, 2023, at 8:36 PM, Mike Schinkel <mi...@newclarity.net> wrote:



--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/8f46fecc-1d5f-4507-8209-76adef294bafn%40googlegroups.com.

[Marvin Renich]

* burak serdar <bse...@computer.org> [231114 19:09]:

> I do not agree that this is because how the compiler works. A value
> receiver is equivalent to pass-by-value argument, that is:
>
> rcp.version()
>
> is equivalent to:
>
> RPC.version(rpc)
>
> thus, creating the copy of the rpc variable. So, the compiler may
> choose to avoid the race by not copying it, or by inlining the version
> function, but according to the spec, it is passed by value, i.e., it
> is copied.

I was going to say that the compiler must not optimize away the copy
because it changes behavior, but thinking it through, it may optimize
away the copy if it can determine that doing so will not change the
behavior of a program that does not violate the language spec (or memory
model).

On a more esoteric note, while this is definitely a race and there are
no "benign" data races, I am unaware of any current, commodity computer
architectures on which the race in Dave Cheney's article will create any
output other than what was expected.

This is based on the assumption that current mass-marketed computers
have the property that a read-during-concurrent-write will never end up
with non-deterministic values anywhere but the result of the read. I.e.
if memory location A is being copied to memory location B, while
concurrently value Y is being written to A, then when the two concurrent
operations are finished, A will have value Y, B will have a
non-deterministic value, and no other memory locations will be affected.

If there are CPU/architectures that do not conform to this, that are
currently being produced, and to which Go has been ported, I would be
interested to hear about them (even if they are not mass-marketed
computers).

What makes a data race _never_ benign is portability. Compiler writers
must obey both the language spec and the CPU/architecture spec. A data
race (according to the language spec) that is thought to be benign adds
a third spec that the compiler writers _do not_ need to obey: the
specific behavior of the hardware architecture when code written in that
language does not obey the language spec (or memory model). The race
may be benign on specific combinations of compiler version and hardware
architecture version, but both hardware designers and compiler writers
are free to build new versions that do not conform to the spec that the
programmer implicitly created when writing the "benign" data race.

...Marvin

[Marvin Renich]

* Robert Engels <ren...@ix.netcom.com> [231114 21:55]:

> Switching to pointer receivers everywhere actually makes this worse. Any
> access is potentially a data race. 
> It stills seems like this is a compiler issue. There needs to be a way to
> synchronize the pointer to value copy in conjunction with other
> synchronization. 
> The only way to do this would be to write your own pointer to value
> conversion methods that have synchronization. 

I don't understand what you are saying. Are you saying that if you take
the code from Dave Cheney's article, and change the receiver for the
version method from value to pointer:

func (*RPC) version() int {

that there is still a race in his code? Or are you saying that with
that change you could add code where the act of calling a method is
inherently a race? If the latter, can you give an example?

...Marvin

[Robert Engels]

What I’m suggesting is that imagine a dev changes that code and has version() access the request property…

This is why if you are sharing data in a concurrent way you need to be very careful of all usages.

The safest solution is to use immutable objects - of which the non pointer are.

So saying - just use pointer receivers - woefully underestimates the work to do shared concurrent code correctly.

> On Nov 15, 2023, at 10:33 AM, Marvin Renich <mr...@renich.org> wrote:
>
> * Robert Engels <ren...@ix.netcom.com> [231114 21:55]:

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/ZVTyykWUgiwPLH9n%40basil.wdw.

[Marvin Renich]

* Robert Engels <ren...@ix.netcom.com> [231115 12:29]:

> What I’m suggesting is that imagine a dev changes that code and has
> version() access the request property…

Okay, that makes sense.

> This is why if you are sharing data in a concurrent way you need to be
> very careful of all usages.

Absolutely.

> The safest solution is to use immutable objects - of which the non
> pointer are.

Perhaps, but it is not appropriate in many cases.

> So saying - just use pointer receivers - woefully underestimates the
> work to do shared concurrent code correctly.

True.

I don't think there should be a default answer to "Which kind of
receiver should be used by default?" Each type deserves a deliberate
choice.

The thought process should include questions like:

Must the object be mutable?
Is the object large enough and will it be passed around enough that
copying it is likely to be a performance issue?
Will the type be assigned to an interface? How will its receiver type
affect its usage there?

When an object of a particular type is to be used concurrently, the
design of the type and its methods deserve careful thought, and no
"default answer" for any part of the design is going to do that thought
for you, whether it is receiver type, access control, or
synchronization.

...Marvin

[Mike Schinkel]

On Nov 15, 2023, at 7:08 AM, 'Brian Candler' via golang-nuts <golan...@googlegroups.com> wrote:

On Tuesday, 14 November 2023 at 03:38:04 UTC Mike Schinkel wrote:

1. A value variable and multiple value receivers  <--- compiles
2. A pointer variable and multiple value receivers <--- compiles
3. A pointer variable and multiple pointer receivers.  <--- compiles 
4. A value variable and multiple pointer receivers.  <--- will NOT compile
5. A pointer variable and mixed value+pointer receivers  <--- compiles 
6. A value variable and mixed value+pointer receivers. <--- will NOT compile

Permutation #4 and #6 are consistent with the description above, and they both have a value variable in common.

However, given than #5 DOES compile, I was left wondering why the standard that mixed receivers should be flagged as an error? 

I am not sure that "the standard" is for mixed receivers to be flagged as an error. Can you give a specific example of where the standard Go toolchain does this?

Thanks for asking.

In hindsight I was in-artful in my use of words. I should not have said "flagged as an error" even though that is what GoLand does.  I should have instead asked why the Go FAQ advised to use all pointer receivers when there are any pointer receivers:

> Next is consistency. If some of the methods of the type must have pointer receivers, the rest should too, so the method set is consistent regardless of how the type is used. See the section on method sets for details.

BTW, I am assuming that GoLand flags mixed receivers as a concern because of what the Go FAQ recommends.

-Mike

Is this question really about Goland, a third-party product, and/or some underlying third-party linter that it uses?  If so, it's an issue for that third party.

Arguably it's not great style to do this, and it might make it confusing for users of your package. But it's not an error, either for the compiler or "go vet". https://go.dev/play/p/fBgrltIEjA2

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/4fe6a954-696c-40f1-86cf-ea9148e42ba2n%40googlegroups.com.

[Victor Giordano]

Regarding the Original question:

I guess the advice of using a pointer receiver can be OK as long you know what you are coding.

quoting "https://go.dev/tour/methods/8"

> There are two reasons to use a pointer receiver.

> The first is so that the method can modify the value that its receiver points to.

> The second is to avoid copying the value on each method call. This can be more efficient if the receiver is a large struct, for example.

TD;DR;

Lads, I guess this entry and the subsequent entries are very important regarding the discussion.

Also we shall recall that T and *T are different types. 

Golang performs some implicit conversions in order to make the code more readable when we use the methods "directly" on a concrete type, but when we use interfaces there is no implicit conversion.  I guess this entry has key information about this affair.

[Mike Schinkel]

On Thursday, November 16, 2023 at 9:16:22 AM UTC-5 Victor Giordano wrote:

Lads, I guess this entry and the subsequent entries are very important regarding the discussion.

Also we shall recall that T and *T are different types. 

Golang performs some implicit conversions in order to make the code more readable when we use the methods "directly" on a concrete type, but when we use interfaces there is no implicit conversion.  I guess this entry has key information about this affair.

For clarity, that was mentioned in this discussion, here. 

-Mike

[Victor Giordano]

You may forgive me.

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/Qb4IAEbpziU/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/9f7c1d63-aa73-41a8-aeaf-1b191bec4cebn%40googlegroups.com.

--

V

[Ken Lee]

Hi Oliver is the link already available? Would like to learn more about this

[Tobias Klausmann]

Hi!

On Wed, 09 Oct 2024, Ken Lee wrote:
> On Tuesday 14 November 2023 at 7:23:44 am UTC+8 Oliver Lowe wrote:
>
> > > I'd be curious to hear thoughts on this topic.
> >
> > There was a fun talk at GopherConAU just a few days ago: "What's
> > The Point? A Guide To Using Pointers Without Panicking" (talk
> > description at https://gophercon.com.au/) When the recordings are
> > all finalised I can reply to this thread with a link.
> >
> Hi Oliver is the link already available? Would like to learn more about this
>

I think this is the talk in question:

https://www.youtube.com/watch?v=XyTZ6Q-uOn4

HTH,
Tobias

--
We are sorry, but the number you have dialed is imaginary.
Please rotate your phone 90 degrees and try again.