about convert sync.Once.done to atomic.Bool

[xie cui]

cl: https://go.dev/cl/427140 has been revert caused by that I change the order of done and m.

before this cl:

```

type Once struct {
    // done indicates whether the action has been performed.
    // It is first in the struct because it is used in the hot path.
    // The hot path is inlined at every call site.
    // Placing done first allows more compact instructions on some architectures (amd64/386),
    // and fewer instructions (to calculate offset) on other architectures.
    done uint32
    m    Mutex
}

```

after cl:

type Once struct {
    m Mutex
    // done indicates whether the action has been performed.
    // It is first in the struct because it is used in the hot path.
    // The hot path is inlined at every call site.
    // Placing done first allows more compact instructions on some architectures (amd64/386),
    // and fewer instructions (to calculate offset) on other architectures.
    done atomic.Bool
}

I change the cl just to avoid the static analysis failed. 

like in 

https://storage.googleapis.com/go-build-log/0fbfe7b1/linux-amd64_2789d15b.log

some of them:

```

    analysistest.go:459: a/copylock.go:53:8: diagnostic "assignment copies lock value to *tp: a.Tlock contains sync.Once contains sync/atomic.Bool contains sync/atomic.noCopy" does not match pattern `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync.Mutex`

```

If i send another cl with that the order of done and m would not be changed and fix the static analysis failed.  It's that ok.

We need to consider that someone may use some code like test on x/tools to check the copy. If the need cl return return like  "assignment copies lock value to *tp: a.Tlock contains sync.Once contains sync/atomic.Bool contains sync/atomic.noCopy" instead of  `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync.Mutex`.  

Wait for your suggestion.

[Bryan C. Mills]

Probably the right fix is to change `copylock.go` so that it passes regardless of the internal implementation of sync.Once, such as:

// want `assignment copies lock value to \*tp: a.Tlock contains sync.Once contains sync\b.*`

You can test the `x/tools` change locally using your locally-modified copy of the main `go` repo, and when they both pass you can merge the x/tools change first, followed by the change in the main repo.

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-dev/3bf40af7-6dbd-4bb4-8ade-86e24701a9f0n%40googlegroups.com.

[xie cui]

It's a good idea. I send cls:

tools  go

[Alan Donovan]

The test failure is trying to tell you that there are now two non-copyable fields in Once--the Mutex and the atomic.Bool--and the analysis reports whichever is first. The appropriate action is to change the analysis test expectation, not to reverse the order of fields in Once (especially since the existing order was important enough to be clearly documented as such).

--

[Rob Pike]

Given the trouble this is causing, I strongly suggest dropping the
attempt to change the synchronization in an already subtle function,
and leaving the code as it was.

-rob

> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-dev/CACKEwjGsXM1mpAtucGYAYwzAHcDinq4C9_KXbLJpq2bhQtM7sQ%40mail.gmail.com.