uber-go / atomic

Wrapper types for sync/atomic which enforce atomic access
https://go.uber.org/atomic
MIT License
1.35k stars 100 forks source link

Remove methods duplicated in `go1.19` `sync/atomic` #119

Open eNV25 opened 2 years ago

eNV25 commented 2 years ago

Since the go1.19 update, there are lots of duplicate functionality between this package and sync/atomic. This duplication is currently needed because go1.18 is still supported.

sync/atomic has the following types that are duplicated here:

All types in sync/atomic have the following methods:

Integer-like types have the following, in addition to the above:

But there is still some functionality in here go.uber.org/atomic that is not there in sync/atomic.

All thing considered, after go1.18 becomes unsupported, we could do the following:

I just wanted to document all this stuff before I forget.

abhinav commented 2 years ago

This sounds reasonable. With Pointer[T] and Go 1.19, I considered embedding but opted against it so that we still had control of the exact API surface of the type exported by this package. There were two reasons to do this:

I'm not opposed to embedding, but I do want to weigh the documentation accessibility issue against it.

Regardless, whether we embed or not, we should at least reuse the standard library types' implementations now.

eNV25 commented 2 years ago

I am fine with both embedding and wrapping. I don't think it makes much of a difference in practice.

If we decide to embed, we should explain that in the docs like for Value.

Else if we decide to wrap, we should do that for Value as well.

hrissan commented 3 months ago

Wrappers are mandatory at least for some types, because if you look into standard package, you'll find that there is special code ensuring alignment of int64 values on platforms that require it.

std: type Int64 struct { noCopy align64 v int64 }

Without that align64, code is simply incorrect and relies on the chance to be correctly aligned.

uber: type Int64 struct { _ nocmp v int64 }