proposal: sync/atomic: add JSON and Text Marshal and Unmarshal functions to scalar types

[erikdubbelboer]

Simplified use case:

type Metrics struct {
	Requests int64 `json:"requests"` // I want to replace this with an atomic.Int64
}

func main() {
	var metrics atomic.Pointer[Metrics]

	metrics.Store(&Metrics{})

	// Standin for somehthing like a HTTP handler or something else generating metrics...
	go func() {
		for {
			time.Sleep(time.Millisecond)

			m := metrics.Load()
			atomic.AddInt64(&m.Requests, 1) // Would become m.Add(1)
		}
	}()

	for {
		time.Sleep(time.Second)

		m := metrics.Swap(&Metrics{})

		// Give other goroutines another 100 milliseconds to do
		// their atomic operations on m.
		time.Sleep(time.Millisecond * 100)

		b, _ := json.Marshal(m) // This would become safe with atomic.Int64
		fmt.Println(string(b)) // Send to some other place etc...
	}
}

Adding these methods would allow changing Metrics.Requests to an atomic.Int64 to make it simpler and safer.

The previous time this was suggested it was reject because sync/atomic can't import encoding/json. But this is not required to implement the interfaces. Only strconv would be required.

Sample implementation for atomic.Int64:

// MarshalText implements the encoding.TextMarshaler interface.
func (x *Int64) MarshalText() ([]byte, error) {
	if x == nil {
		return []byte("<nil>"), nil
	}
	return []byte(strconv.FormatInt(x.Load(), 10)), nil
}

// UnmarshalText implements the encoding.TextUnmarshaler interface.
func (x *Int64) UnmarshalText(text []byte) error {
	n, err := strconv.ParseInt(string(text), 10, 64)
	if err != nil {
		return err
	}
	x.Store(n)
	return nil
}

// MarshalJSON implements the json.Marshaler interface.
func (x *Int64) MarshalJSON() ([]byte, error) {
	if x == nil {
		return []byte("null"), nil
	}
	return x.MarshalText()
}

// UnmarshalJSON implements the json.Unmarshaler interface.
func (x *Int64) UnmarshalJSON(text []byte) error {
	// Ignore null, like in the main JSON package.
	if string(text) == "null" {
		return nil
	}
	return x.UnmarshalText(text)
}

atomic.Bool shouldn't use strconv.ParseBool as it isn't JSON compatible. Instead if can just compare against "true" and "false".

atomic.Pointer, atomic.Uintptr and atomic.Value are omitted from this proposal as they would require importing of encoding/json which is not allowed.

[dsnet]

We don't need the MarshalJSON and UnmarshalJSON methods since "encoding/json" already has support for MarshalText and UnmarshalText. The only odd case is Bool, where we might need the MarshalJSON methods.

[erikdubbelboer]

The problem with using MarshalText for JSON support is that it will be a string in the JSON instead of a number as expected:

type N int

func (n N) MarshalText() ([]byte, error) {
	return []byte("1234"), nil
}

type S struct {
	A N
}

func main() {
	b, _ := json.Marshal(S{})
	fmt.Println(string(b))
}

{"A":"1234"}

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[dsnet]

There is some movement on taking a holistic look at all encoding/json issues. I'm a bit concerned about adding MarshalJSON methods, which are not very performant since the implementation would always have to allocate. Should we delay this until after potential changes to encoding/json stabilizes?

[dsnet]

I proposed #56235, which would affect what happens here.

If that proposal is accepted, we would add something like:

func (*Bool) MarshalScalar() (bool, error)
func (*Bool) UnmarshalScalar(bool) error

func (*Int32) MarshalScalar() (int64, error)
func (*Int32) UnmarshalScalar(int64) error

func (*Int64) MarshalScalar() (int64, error)
func (*Int64) UnmarshalScalar(int64) error

func (*Uint32) MarshalScalar() (uint64, error)
func (*Uint32) UnmarshalScalar(uint64) error

func (*Uint32) MarshalScalar() (uint64, error)
func (*Uint32) UnmarshalScalar(uint64) error

[rsc]

Let's hold this proposal for #56235.

[rsc]

Placed on hold.
— rsc for the proposal review group

[trim21]

https://go.uber.org/atomic support this