atomicptr: use atomic.Pointer

[tamird]

• go_generics: add Go generics support
• atomicptr: use atomic.Pointer

The change to the atomicptr package is trivial, but is included here as a
motivating case for the addition of generics support to go_generics.

[tamird]

@ayushr2 could you have a look at this one?

[ayushr2]

IIUC, we are using go_generics package to generate code which is using Go's native generics support. Two levels of generated code. So why not directly replace the go_generics atomic pointer with atomic.Pointer? Maybe I am missing something. Is there a benefit to doing it this way?

[tamird]

I couldn't agree more, but stateify doesn't yet support generics. That requires the first N-1 commits in #12470, so I was planning to send that after that PR (and this one) merge.

[tamird]

Having said that, I could move those commits to this PR, since there is some question about whether #12470 can be merged at all.

[ayushr2]

IIUC 80913f9 requires wrapping the atomic pointer fields with a struct and putting special stateify tags on them. I think that is also not a very clean interface.

We should probably add native support to stateify for atomic.Pointer; such that it should be able to detect atomic.Pointer usage and generate code using .Load() and .Store(). I don't know if the AST parser we use is capable to do this. If not, we can add support for new stateify tags like:

	ptr atomic.Pointer[int] `state: pointer".(int)"`

If you intend to cleanup atomicptr from go_generics, then we could also just use:

// +stateify savable
type S struct {
	ptr atomic.Pointer[int] `state:".(int)"`
}

func (s *S) savePtr() int {
	return s.ptr.Load()
}

func (s *S) loadPtr(_ goContext.Context, ptr int) {
	s.ptr.Store(ptr)
}

[tamird]

IIUC 80913f9 requires wrapping the atomic pointer fields with a struct and putting special stateify tags on them. I think that is also not a very clean interface.

We should probably add native support to stateify for atomic.Pointer; such that it should be able to detect atomic.Pointer usage and generate code using .Load() and .Store(). I don't know if the AST parser we use is capable to do this.

It's not that the AST parser isn't capable, it's that that state package fundamentally wants to register a reflect.Type under a unique name:

gvisor/pkg/state/types.go

Line 339 in 70f9a82

name := t.StateTypeName()

Since there's no reflect.Type associated with an uninstantiated generic, we'd need to do string matching on the type's name and its methods to discover that it is an atomic.Pointer.

If not, we can add support for new stateify tags like:

	ptr atomic.Pointer[int] `state: pointer".(int)"`

We perhaps could, but is that any better? This would make a poor addition to go_stateify because it doesn't generalize beyond atomic.Pointer.

If you intend to cleanup atomicptr from go_generics, then we could also just use:

// +stateify savable
type S struct {
	// descriptorTable holds descriptors.
	ptr atomic.Pointer[int] `state:".(int)"`
}

func (s *S) savePtr() int {
	return s.ptr.Load()
}

func (s *S) loadPtr(_ goContext.Context, ptr int) {
	s.ptr.Store(ptr)
}

We could, but that feels quite manual, and ergonomically worse than what exists today.

Before the codegen deficiencies were pointed out in #12470 (review) my goal was to remove type parameter support from go_generics, leaving only constants.

Now answering your first point:

IIUC 80913f9 requires wrapping the atomic pointer fields with a struct and putting special stateify tags on them. I think that is also not a very clean interface.

Well, I don't disagree, but haven't come up with a cleaner way of supporting generics with stateify. I'm all ears for suggestions!