Add CompareAndSwap and Swap for String and Error. Deperecate CAS to reflect stdlib Value.

[eNV25]

Before opening your pull request, please make sure that you've:

• updated the changelog if the change is user-facing;
• added tests to cover your changes;
• run the test suite locally (make test); and finally,
• run the linters locally (make lint).

Thanks for your contribution!

Add CompareAndSwap and Swap for String and Error. Deperecate CAS to reflect stdlib Value.

This is not done for Time since it is not meant to be compared.

[CLAassistant]

All committers have signed the CLA.

[eNV25]

I updated the PR. I removed the atomic type for func() here, since that was not the main point of the PR.

[rabbbit]

Thank you for the contribution, but could you explain why you're doing this? What is the ultimate goal?

Go <1.17 is EOL, so it doesn't seem very useful to support that. It looks like Go 1.19 will bring it's own changes to the atomic as well.

[eNV25]

I was looking for the CompareAndSwap and Swap methods for atomic.String (and atomic.Error) and I couldn't find them. I thought it must have been since atomic.Value didn't support them before go1.17, but now it just looks like nobody bothered to update them.

I will change the PR to remove the backport, since you are fine with old versions being unsupported, but I will keep the rest of the changes that add CompareAndSwap and Swap, and deprecate CAS.

[rabbbit]

Hey,

Generally, since the PR title is now "do X and do Y", and X is independent from Y, I would keep them as separate PRs.

Regarding the change itself:

• I'm not sure whether we want to deprecate the naming. Doesn't seem worth it, unless you convince other maintainers.
• I don't have much context on string.Swap, but it seems to seem reasonable.

[eNV25]

I'm not sure whether we want to deprecate the naming. Doesn't seem worth it, unless you convince other maintainers.

This is to align with stdlib sync/atomic. From go1.19 it has types much like this package, (e.g. sync/atomic.Bool.CompareAndSwap).

https://pkg.go.dev/sync/atomic

I don't have much context on string.Swap, but it seems to seem reasonable.

There was the following comment in the source code:

// Note: No Swap as String wraps Value, which wraps the stdlib sync/atomic.Value which
// only supports Swap as of go1.17: golang/go#39351

Now the situation is different. go1.17 added Value.CompareAndSwap and Value.Swap, and go1.19 added T.CompareAndSwap, T.Swap.

Go <1.17 is EOL, so it doesn't seem very useful to support that. It looks like Go 1.19 will bring it's own changes to the atomic as well.

[rabbbit]

Could you put up all the 3 changes (names, swap, formatting) in separate PRs, please?

We can discuss merits of each of them individually.

[eNV25]

Could you put up all the 3 changes (names, swap, formatting) in separate PRs, please?

You want like this?

• all: go fmt ./... go1.19
• Add CAS, Swap for String, Error
• Deprecate CAS, Add CompareAndSwap

[eNV25]

Now I have 4 commits in this branch.

The deprecation change depends on the previous one, and so can't submit simultaneous PRs without the history looking weird. I will have to keeps those together.

I will submit the other commits as separate PRs.