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.

Reply via email to