[Development] QProperty and library coding guide

[Thiago Macieira]

On Friday, 17 July 2020 09:41:16 PDT Lars Knoll wrote:
> > Let's say we're going to enable it now for Linux. Ok, good. Please make
> > EVERY  SINGLE Qt build and user's application build unconditionally use
> > [[no_unique_address]] from now until 2028. Regardless of compiler choice
> > (GCC, Clang and ICC), compiler version and build flags.
> 
> This is only about desktop linux here where we want to define an ABI. And
> there we can enable it by default I think.

No, it applies to everywhere where we can't guarantee the whole system is 
rebuilt. That applies to Mac and Windows desktops (yes, MSVC too) as well as 
embedded systems where an application may be compiled after Qt, with an 
upgraded SDK / toolchain. This happens.

In other words, we should default to a fixed value, chosen RIGHT NOW. No auto-
detection. Users will need to explicitly choose a different value for their 
platforms, if they wish.

> > Here's the patch. Add qglobal.h:
> > #if defined(Q_OS_LINUX) && !__has_cpp_attribute(no_unique_address)
> > #  error "Your compiler is too old."
> > #  error "Please upgrade so it supports [[no_unique_address]]."
> > #endif
> 
> 
> No, we error out if Qt build ABI and application builds flags are not in
> sync. If you configure Qt without support for [[no_unique_address]], it’ll
> work, but it’s a different build.

Fair enough, a value hardcoded into qconfig.h instead Q_OS_LINUX.

> > I will give a +2 for this patch, since I prefer it. That means adding 
> > properties doesn't imply an extra 8 bytes per class in the hierarchy.
> > Imagine  a user class hierarcy like QSctpSocket -> QTcpSocket ->
> > QAbstractSocket -> QIODevice -> QObject. If each class has properties,
> > that adds 40 bytes to the full size of QSctpSocket.
> 
> 
> No, it doesn’t. It adds 4/8 bytes (unless your object hierarchy is deeper
> than 4/8 levels). Since every one of those unions in the hierarchy has an
> alignment requirement of 1, the property union for QIODevice will be in the
> byte following the one for QObject.

You're thinking of the tail padding, as in here: https://godbolt.org/z/1oreGx. 
Note MSVC does not use tail padding, so this discussion only applies to the 
IA-64 ABI (GCC, Clang, ICC).

It also only applies if the union is at the tail. In our regular coding style, 
it wouldn't be:

class K1 : public QObject
{
    Q_OBJECT
public:
    // property declarations

private:
    K1Private *d;
};

The union is not at the tail. So a class deriving from K1 wouldn't be able to 
use the tail padding.

Then there's the fact that a class that adds other members would not pass on 
the available padding at the tail of its own to derived classes. We'd need a 
zig-zag of where the properties are declared in order to use that tail 
padding, once.

class QObject
{
    QObjectData *d_ptr;
public:
    // properties at tail
}; // sizeof: 24

class Foo1 : public QObject
{
public:
    // properties at beginning

private:
    void *ptr;  // other members at end
}; // sizeof: 32 (properties used tail padding)

class Foo2 : public Foo1
{
public:
    double d;  // other members at beginning
    // properties at end
}; // sizeof: 48

class Foo3 : public Foo2
{
public:
    // properties at beginning

private:
    qint64 x;  // other members at end
}; // sizeof: 56 (tail padding used)
 
> > [Yes, I know Qt-based classes should just put their properties in the d 
> > pointer, but users don't usually have d pointers]
> 
> 
> Not sure what you mean here. This is only about our Q_PRIVATE_QPROPERTY, not
> about the QProperty class.

Oh, right, never mind.

Ulf, please add this to the Coding Style:

Every public class with Q_OBJECT must have Q_PRIVATE_QPROPERTIES_BEGIN / 
Q_PRIVATE_QPROPERTIES_END, regardless of whether it has any properties, in 
case someone in the future wants to add a property.

No, it can't be automatic as part of Q_OBJECT.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel System Software Products