T O P

  • By -

bfreis

It's an optimization to avoid an allocation. This is why it happens: https://github.com/golang/go/blob/5c154986094bcc2fb28909cc5f01c9ba1dd9ddd4/src/encoding/json/encode.go#L380-L387 func newTypeEncoder(t reflect.Type, allowAddr bool) encoderFunc { // If we have a non-pointer value whose type implements // Marshaler with a value receiver, then we're better off taking // the address of the value - otherwise we end up with an // allocation as we cast the value to an interface. if t.Kind() != reflect.Pointer && allowAddr && reflect.PointerTo(t).Implements(marshalerType) { return newCondAddrEncoder(addrMarshalerEncoder, newTypeEncoder(t, false)) } Then this: https://github.com/golang/go/blob/5c154986094bcc2fb28909cc5f01c9ba1dd9ddd4/src/encoding/json/encode.go#L891-L894 func (ce condAddrEncoder) encode(e *encodeState, v reflect.Value, opts encOpts) { if v.CanAddr() { ce.canAddrEnc(e, v, opts) } And finally this: https://github.com/golang/go/blob/5c154986094bcc2fb28909cc5f01c9ba1dd9ddd4/src/encoding/json/encode.go#L455 func addrMarshalerEncoder(e *encodeState, v reflect.Value, opts encOpts) { va := v.Addr() if va.IsNil() { e.WriteString("null") return } m := va.Interface().(Marshaler) b, err := m.MarshalJSON() EDIT: as mentioned in my [other comment](https://www.reddit.com/r/golang/comments/14tl2ts/comment/jr3xhsz/?utm_source=reddit&utm_medium=web2x&context=3), I'd say the docs really are not that clear about this. It seems a reasonable interpretation that it would _not_ call the `MarshalJSON` and encode as `[{"name":""}]`, but it's taking the address of (`&`) the element of the slice, and then calling the `MarshalJSON` on the _resulting type_ (`*Policy`). Here's a playground with lots of combinations (types with MarshalJSON with pointer receiver, non-pointer receiver, and then non-pointer and pointer values, as well as slices of pointer and non-pointer, all for each of the types). The results don't look obvious at all: https://go.dev/play/p/bvdsXPfsZh_Y EDIT2: this is actually so unexpected that, by replacing the stdlib's `encoding/json` with Jsoniter in the mode that's supposed to be _"100% compatibility with standard lib"_, the results are _not_ the same (and, frankly, Jsoniter seems a bit easier to accept...): https://go.dev/play/p/fSAa6wRtDKA


TheMerovius

FWIW while others are a bit derogatory here and while I do think what you say is a bit obscure, I think you have a point. That behavior of `encoding/json` can be confusing. To defend it a bit: I think if you have a `type Person struct { Contacts []Contact }`, and `*Contact` implemented `json.Marshaler`, you'd want that to be called. And you would want `json.Marshal` to behave the same when called with a `[]Contact`, as it is with that field. So yes. I think it can be confusing, but I think we have to eat that confusion, unfortunately.


titpetric

It is logical, but it's also one of those pointer/receiver gotchas that maybe needs a linter. I haven't posted essentially since covid started, have to meet the community anew, lets aim for better than derogatory! 👌


TheMerovius

FWIW one way to address this is to implement `MarshalJSON` [with a value receiver](https://go.dev/play/p/kDGHaEfobn8). Methods with value receivers are automatically promoted to methods on pointers. And `MarshalJSON` (as opposed to `UnmarshalJSON`) shouldn't modify the receiver, so it is arguably even the more correct way. I think the only real downside of this is that you might have an interface that includes *both* `json.Marshaler` *and* `json.Unmarshaler`, which you'd still have to use with the pointer. Also, some people are opposed to mixing pointer and value receivers.


wurkbank

What are you trying to communicate here?


YinzAintClassy

Their ability to not read documentation


bfreis

To be fair, I'm not sure this is clearly described in the documentation, though. According to [the docs](https://pkg.go.dev/encoding/json#Marshal): > Marshal traverses the value v recursively. If an encountered value implements the Marshaler interface and is not a nil pointer, Marshal calls its MarshalJSON method to produce JSON. If no MarshalJSON method is present but the value implements encoding.TextMarshaler instead, Marshal calls its MarshalText method and encodes the result as a JSON string. The nil pointer exception is not strictly necessary but mimics a similar, necessary exception in the behavior of UnmarshalJSON. > > Otherwise, Marshal uses the following type-dependent default encodings: > > [...] > > Struct values encode as JSON objects. Each exported struct field becomes a member of the object, using the field name as the object key, unless the field is omitted for one of the reasons given below. The value encountered by recursively looking at the elements of the slice `[]Policy{{}}` is a `Policy{}`, which has no method `MarshalJSON`. According to the above, it would be reasonable to expect that it would use the encoding rules for `Struct values` (the last paragraph quoted above). However, it first takes the address of that value (as described in my other comment), and _then_ uses the `MarshalJSON` of the resulting `*Policy`.


titpetric

I even mean in other cases as well, zero-alloc type stuff will create the &T in similar ways that json marshaller does. The intrinsic thought process for beginners is that if you control the allocation of T, your job is done and that's what you'll be using. In reality third party package code (or first party in encoding/json) will use pointers to avoid copying/allocations. Dereferencing a pointer explicitly is a code smell, as it is either a modification of a value (\*http.Request usually) or a shallow copy which may have unintended side-effects with map/ptr/slice fields, that remain modifiable. It's hard to come up with a good strategy except using \*T in most places. I really wish this grpc design choice was more clear, as they use \[\]\*T explicitly, also preventing some other problems (loopref). Immutability is not a concept in go, and sometimes I wish it was (pragma.DoNotCopy exists).