On Sat, Feb 4, 2017 at 2:38 PM, Axel Wagner <axel.wagner...@googlemail.com> wrote: > On Sat, Feb 4, 2017 at 11:13 PM, Ian Lance Taylor <i...@golang.org> wrote: >> >> The spec does not say that unsafe.Alignof(s.B) will return 8. In >> fact, on 32-bit targets, with the gc toolchain, it will return 4. > > > To me, that seems to contradict the combination of "The functions Alignof > and Sizeof take an expression x of any type and return the alignment or > size, respectively, of a hypothetical variable v as if v was declared via > var v = x" and the table at the very bottom, saying an uint64 has Alignof 8. > The latter seems to strongly imply that a "var v = s.B" must be 8-byte > aligned, as v is a variable (even an allocated one; one of the few mentions > of "allocate" in the spec :) ) of type uint64. Then, the former would imply > that the same happens to s.B, as that is how the function is defined.
The table at the very bottom is only type sizes, not type alignments. >> That is sufficient to ensure that your code works correctly. It does >> mean that certain kinds of operations don't work. sync.WaitGroup >> plays the games it does to avoid introducing an extra allocation. > > > I don't understand. Where is the extra allocation, if you where to change > state to be an uint64 and just dereference it, instead of using > unsafe.Pointer casting in state()? > > Am I understanding it correctly that you are saying a) yes, fields like, say > expvar.Float can not be embedded, unless you take special care (recursively) > of having it aligned and b) the only safe way to use atomic with fields > would be, to put a pointer in and allocate with new (and that's the "extra > allocation" WaitGroup is trying to avoid?). > > I would find that situation understandable, but also pretty unfortunate. It > seems very easy to make a mistake that way (embedding a type, not knowing > that it relies on atomics). It also means, it's harder to make the zero > value of a struct useful; while having a uint64 intiialized to zero is a > perfectly fine default for a lot of cases, needing to allocate it with new > requires a constructor. Yes. > I would hope we could at least a) recommend that (and maybe the WaitGroup > trick) as "the" ways to use atomic somehow and b) have some kind of check to > detect misuse. The memory layout of all structs is statically known, so it > should be theoretically possible to detect usages of atomic.AddUint64(&s.f) > (or the like) with a misaligned field, no? Sounds like a good cmd/vet check. Ian -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.