1

Is there a order to declare widgets in Qt5(perhaps 4 too) ?

Consider the following pieces of code:

(just the a piece of the header to help me explain)

class ConfigDialog : public QDialog
{
    Q_OBJECT

    QGroupBox userAuthBox;
    QGridLayout userAuthLayout;
    QVBoxLayout dialogLayout;

    QLabel userLabel;
    QLabel passLabel;
    QLineEdit userEdit;
    QLineEdit passEdit;
};

this works as expected but just changing to (reordering declarations):

class ConfigDialog : public QDialog
{
    Q_OBJECT

    QLabel userLabel;
    QLabel passLabel;
    QLineEdit userEdit;
    QLineEdit passEdit;
    QGroupBox userAuthBox;
    QGridLayout userAuthLayout;
    QVBoxLayout dialogLayout;
};

this works also, but when the ConfigDialog goes out of scope happen a segfault.

I've saw this on other scenarios too, but always changing the order fix this.

Victor Aurélio
  • 2,355
  • 2
  • 24
  • 48
  • May be you haven't done a full build after reordering. If there were already compiled object files in which the member order is not the same as current order, this can happen. – Lahiru Chandima Aug 24 '15 at 02:46
  • @LahiruChandima it's not the case, I first have build the second piece of code(segfault) then reorder then rebuild (no segfault) and I'm using qmake, AFAIK it takes care of rebuilt the required parts. and also if I'm not mistaken this has occurred also in a python program. – Victor Aurélio Aug 24 '15 at 02:51

2 Answers2

4

My guess would be: you make your QGroupBox a parent of some of the other widgets.

Qt has a concept of parent-child relationship between QObjects. The parent is responsible for deleting its children when it itself is destroyed; it is assumed that those children were allocated on the heap with new.

Further, data members of a C++ class are constructed in the order they are listed in the class, and are destroyed in the reverse order.

Let's say userAuthBox is made a parent of userLabel (via setParent call, in your case executed by addWidget). In the first case, userLabel is destroyed first, and notifies its parent of this fact, whereupon userAuthBox removes it from its list of child widgets, and doesn't attempt to delete it.

In the second case, userAuthBox is destroyed first, and uses delete on its pointer to userLabel. But of course userLabel was not in fact allocated with new. The program then exhibits undefined behavior.

Igor Tandetnik
  • 50,461
  • 4
  • 56
  • 85
  • In that case `QGroupBox` is child of a `QVBoxLayout`(addWidget) which is child of the dialog(setLayout). so what you're telling is that childs of dialog should be in first place, and childs of childs in second and so on ? – Victor Aurélio Aug 24 '15 at 13:35
  • @VictorAurélio A `QWidget` can only be a child of other widgets, so no, it can't be a child of a layout. – Kuba hasn't forgotten Monica Aug 24 '15 at 16:12
  • 1
    Layouts don't own widgets added to them. Parent-child relationship is unrelated to and independent from visual containment; it's only about ownership and lifetime. In fact, I shouldn't have emphasized "widget" in my answer - parent-child relationship is established between `QObject`s, it doesn't require visual aspect at all. – Igor Tandetnik Aug 24 '15 at 16:21
  • "you shouldn't also introduce parent-child relationships between them" I disagree. In most cases it's impossible and incorrect not to introduce such relationships. If you have a child `QObject` (not a widget), you should parent it so that if your object gets moved to another thread, your children automatically get moved as well. Not doing that makes objects not usable but from the main thread. OTOH, declaring sub-objects as indirect (pointer type) members is a premature pessimization for obvious reasons. For widgets, the reason is not threading, but visual hierarchy following the object tree. – Kuba hasn't forgotten Monica Aug 24 '15 at 16:35
  • "Parent-child relationship is unrelated to and independent from visual containment" In case of `QWidgets` it is, unfortunately, completely intertwined and inseparable. It is impossible to have visually-child widgets that are also not `QObject` children. Worse, it's [impossible to have `QWidget`s owned by a `QObject` for memory management purposes only](http://stackoverflow.com/q/28992276/1329652). It's true that one must think of `QWidget` ownership as that of `QObject` ownership, since that's what it is. But visual hierarchy takes completely over this :( – Kuba hasn't forgotten Monica Aug 24 '15 at 16:39
  • @KubaOber I see. I had a wrong mental model of the relationships. I edited the answer further to remove incorrect conclusions. – Igor Tandetnik Aug 24 '15 at 16:42
  • @KubaOber I now wonder - what's the best practice then? What's the solution to the conundrum? The two options I see are a) always allocate all widgets on the heap, and let Qt machinery clean them up, or b) you can create widgets on the stack or as class members, but you must be extremely careful to put them in just the right order (bottom up in the visual hierarchy). Neither feels especially satisfactory. – Igor Tandetnik Aug 24 '15 at 16:48
  • I don't see a conundrum at all. The order of declarations has meaning, it always had. Just as you don't expect random C++ expressions to magically follow your intended behavior, you similarly can't expect randomly ordered C++ member declarations to work correctly. This doesn't only affect `QObject`-using code, you'll run into serious problems if you don't understand this and then use (as you well should) initializer lists in constructors. You *can* refer to uninitialized members from initializer lists, and this *has* undefined behavior. **TL;DR** Know and understand your C++. Declare by value! – Kuba hasn't forgotten Monica Aug 24 '15 at 16:52
  • "you must be extremely careful to put them in just the right order" Not extremely careful, just ordinarily careful, IMHO. Once you start writing modern-style C++11 code with RAII and functionally rich initializer lists, you'll run into identical problems from not understanding the impact the order of declaration has on the behavior of the constructors and destructors. Finally, given that MSVS static analyzer is quite powerful, free, and will pick up such issues if you explicitly mark dependencies via modern val initialization, you can't even really claim that this error will be undetected... – Kuba hasn't forgotten Monica Aug 24 '15 at 16:56
2

TL;DR: Yes! The order of declarations has a strictly defined meaning in C++. A random order will not work, as you've happened to notice.

You're not showing all the code. What is important is that one of the widgets is a child of the group box. Suppose you had:

class ConfigDialog : public QDialog
{
  // WRONG
  Q_OBJECT
  QLabel userLabel;
  QGroupBox userAuthBox;
  QGridLayout userAuthLayout;
  QVBoxLayout dialogLayout;
public:
  ConfigDialog(QWidget * parent = 0) :
    QDialog(parent),
    dialogLayout(this),
    userAuthLayout(&userAuthBox) {
    // Here userLabel is parent-less.
    Q_ASSERT(! userLabel.parent());
    userAuthLayout.addWidget(&userLabel, 0, 0);
    // But here userLabel is a child of userAuthBox
    Q_ASSERT(userLabel.parent() == &userAuthBox);
  }
};

The default destructor will invoke the destructors in the following order - it literally is as if you wrote the following valid C++ code in the destructor.

  1. dialogLayout.~QVBoxLayout() - OK. At this point, the dialog is simply layout-less. All the child widgets remain.
  2. userAuthLayout.~QGridLayout() - OK. At this point, the group box is simply layout-less. All the child widgets remain.
  3. userAuthBox.~QGroupBox() - oops. Since userLabel is a child of this object, the nested userAuthox.~QObject call will execute the eqivalent of the following line:

    delete &userLabel;
    

    Since userLabel was never allocated using new, you get undefined behavior and, in your case, a crash.

Instead, you should:

  1. Declare child widgets and QObjects after their parents.

  2. Use C++11 value initialization if possible, or initializer lists in the constructor to indicate to the maintainer that there is a dependency between the children and the parents.

See this answer for details and a C++11 and C++98 solution that will force the mistakes to be caught by all popular modern static C++ code analyzers. Use them if you can.

Community
  • 1
  • 1
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313