Code review comment for lp:~mixxxdevelopers/mixxx/atomic-co

There appear to be some issues with class ControlObjectValue<T, true>.

First, assuming sizeof(double) <= sizeof(void*) (which is the case on my machine), this class is used for ControlObjectValue<double>. This results in casts like reinterpret_case<void*>(x), where x is a double. These aren't legal casts (and as such, I can't actually compile the branch). To store a double in a QAtomicPointer<void*>, you have to do something like (assuming sizeof(void*)==sizeof(uint64_t) and sizeof(double)==sizeof(uint64_t)):

    QAtomicPointer<void*> qap;
    double x = 1.5;
    qap = reinterpret_cast<void**>(*reinterpret_cast<uint64_t*>(&x));

    void *qap_val = qap;
    double y = *reinterpret_cast<double*>(&qap_val);
    cout << y << endl; // prints 1.5

(some of the complexity exists because QAtomicPointer doesn't overload the deref (*) operator, it provides an implicit type conversion to T*, which means *qap here actually dereferences our stored "pointer", so we have to use void *p = gap which invokes the operator void*() without dereferencing anything)

But regardless, we're not really gaining anything from QAtomicPointer. It's storing a T* in a data member marked volatile (which doesn't even guarantee alignment afaik, which is something we need), and for store and retrieve operations it's not doing anything special (see src/corelib/thread/q{,basic}atomic.h in the qt source). We'd be better off just using a volatile void * in the class, just as a reduction in complexity.

Along these lines:
        return reinterpret_cast<T>(m_container);

As far as I can tell, this is actually trying to cast the QAtomicPointer<void*> to T (an illegal case). This is almost certainly not desirable, but we really want the thing we've stored in the QAP<void*>, cast to T (which requires some bit-fiddling like what I wrote above).