Re: [Development] Opinions on QTBUG-71545

2018-11-08 Thread Luca Beldi
I'm in favour of just adding a line of documentation and maybe include Thiago's 
workaround. Should take 30seconds and hurt nobody

Luca Beldi (VRonin)
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-06 Thread Elvis Stansvik
Den tis 6 nov. 2018 kl 13:56 skrev André Somers :
>
> Hi,
>
>
> On 05/11/2018 20:56, Elvis Stansvik wrote:
> > Den mån 5 nov. 2018 kl 20:32 skrev Konstantin Shegunov 
> > :
> >> Hello,
> >> Since we couldn't agree, I'd love to see some more opinions about this 
> >> one.[1]
> > I may be missing some detail, but I think what Thiago says makes
> > sense. When children are destroyed, you know you're in the QObject
> > destructor (from QObject::~QObject docs: "Destroys the object,
> > deleting all its child objects."), so you know the object is now a
> > QObject, no longer a QCoreApplication. If you require your
> > QCoreApplication to be alive by the time your child object is
> > destroyed, I think you have to ensure this on your own.
> Problem is, I think, that this requirement is not always obvious. For
> your own objects, you know you cannot rely on your parent still being a
> MyClass iso of just a QObject on destruction (unless you take specific
> measures to make that so), but in this case the reliance on there still
> being a Q*Application around (not necessarily the parent) is usually not
> as obvious as a myParent->doSomethingNotFromQObject call in your
> destructor code...

Yea, very good point. I'll leave it to the grow-ups to decide what to
do with this :) It was just my first gut reaction.

Elvis

> >
> > Like I said, I may be missing something, but that's what it looks like
> > to me. I can't see why there would be an exception to the object model
> > here.
> >
> > Elvis
> >
> >> Specifically:
> >> 1) Is parenting to the application object a thing?
> Yes. But you know that the same goes as for any QObject parent/child
> relationship: the parent is a QObject at the time of destruction (ok,
> with QWidget, you're in luck).
> >> 1.a) ... and should it be allowed (i.e. accepting the proposed change)?
> Yes, it should be allowed, and as you argued I think it is useful. But I
> am not sure that implies the proposed change. OTOH, as so much
> functionality in Qt requires the Q*Application to be alive (and that is
> not always obvious) , perhaps an exception *is* in order.
> >> 1.b) .. if not allowed, should we put a warning in the documentation that 
> >> it is wrong and shouldn't be done at all, or at least that it's 
> >> discouraged.
> >>
> >> [1] https://bugreports.qt.io/browse/QTBUG-71545
> I do think it makes sense to be able to do this, but if that is to be
> discouraged, then best be explicit about that.
>
> André
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-06 Thread André Somers

Hi,


On 05/11/2018 20:56, Elvis Stansvik wrote:

Den mån 5 nov. 2018 kl 20:32 skrev Konstantin Shegunov :

Hello,
Since we couldn't agree, I'd love to see some more opinions about this one.[1]

I may be missing some detail, but I think what Thiago says makes
sense. When children are destroyed, you know you're in the QObject
destructor (from QObject::~QObject docs: "Destroys the object,
deleting all its child objects."), so you know the object is now a
QObject, no longer a QCoreApplication. If you require your
QCoreApplication to be alive by the time your child object is
destroyed, I think you have to ensure this on your own.
Problem is, I think, that this requirement is not always obvious. For 
your own objects, you know you cannot rely on your parent still being a 
MyClass iso of just a QObject on destruction (unless you take specific 
measures to make that so), but in this case the reliance on there still 
being a Q*Application around (not necessarily the parent) is usually not 
as obvious as a myParent->doSomethingNotFromQObject call in your 
destructor code...


Like I said, I may be missing something, but that's what it looks like
to me. I can't see why there would be an exception to the object model
here.

Elvis


Specifically:
1) Is parenting to the application object a thing?
Yes. But you know that the same goes as for any QObject parent/child 
relationship: the parent is a QObject at the time of destruction (ok, 
with QWidget, you're in luck).

1.a) ... and should it be allowed (i.e. accepting the proposed change)?
Yes, it should be allowed, and as you argued I think it is useful. But I 
am not sure that implies the proposed change. OTOH, as so much 
functionality in Qt requires the Q*Application to be alive (and that is 
not always obvious) , perhaps an exception *is* in order.

1.b) .. if not allowed, should we put a warning in the documentation that it is 
wrong and shouldn't be done at all, or at least that it's discouraged.

[1] https://bugreports.qt.io/browse/QTBUG-71545
I do think it makes sense to be able to do this, but if that is to be 
discouraged, then best be explicit about that.


André

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Konstantin Shegunov
On Tue, Nov 6, 2018 at 1:07 AM Giuseppe D'Angelo via Development <
development@qt-project.org> wrote:

> Note however that those children are deleted explicitly (via manual
> calls to delete), and NOT via the parent/child relation.
>

Noted. When I started that thread I didn't intend it to become one of those
lengthy arguments really. More like a simple "poll" whether the thing's
allowed, and if not how to handle it. As I wrote in the bug report -
"disallowing" the application to be parent is acceptable as an answer to
me, but then I think it should be noted in the documentation otherwise it's
rather misleading from the user's point of view.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Giuseppe D'Angelo via Development

Hi,

Il 05/11/18 23:41, Konstantin Shegunov ha scritto:

But at least for QApplication I would consider having children being
common practice and actually Qt does this too:

[snippet]


Interesting point, I haven't thought about it.


Note however that those children are deleted explicitly (via manual 
calls to delete), and NOT via the parent/child relation.


See for instance the style getting destroyed in ~QApplication


https://code.woboq.org/qt5/qtbase/src/widgets/kernel/qapplication.cpp.html#_ZN12QApplicationD1Ev


Or the session manager, in ~QGuiApplication


https://code.woboq.org/qt5/qtbase/src/gui/kernel/qguiapplication.cpp.html#_ZN15QGuiApplicationD1Ev



My 2 c,
--
Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
KDAB - The Qt, C++ and OpenGL Experts



smime.p7s
Description: Firma crittografica S/MIME
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Konstantin Shegunov
On Mon, Nov 5, 2018 at 11:02 PM Elvis Stansvik  wrote:

> But seems to me it would be a slippery slope to accept more
> exceptions.


You say exception, but I say expected behavior, which is actually the crux
of the disagreement.


> What's next, will I have to implement the destruction
> myself in my own widgets? :)
>

Yes, in the general case you actually have to if your dependent objects are
attached to a QObject holding an application-global state. Which is exactly
what Q*Application is - a QObject that has/is a global state.

On Mon, Nov 5, 2018 at 11:50 PM Uwe Rathmann 
wrote:

> The title of the bug report is about QCoreApplication, while the demo
> code is a QApplication - so I'm not 100% sure if I completely understood
> the discussion.
>

I'm at fault for not attaching the stack trace, but I'll try to explain.
Indeed the example code is for QApplication, the problem however manifests
whenever the destruction of said application object goes through. The
segfault happens because QCoreApplication sets its static member - the
global instance of the application object - to null before the children are
cleaned up by the QObject destructor. This means that objects parented to
the application are destroyed after there's Q*Application object no more.

But at least for QApplication I would consider having children being
> common practice and actually Qt does this too:

[snippet]
>

Interesting point, I haven't thought about it.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Uwe Rathmann
On Mon, 05 Nov 2018 21:31:26 +0200, Konstantin Shegunov wrote:

> 1) Is parenting to the application object a thing?

The title of the bug report is about QCoreApplication, while the demo 
code is a QApplication - so I'm not 100% sure if I completely understood 
the discussion.

But at least for QApplication I would consider having children being 
common practice and actually Qt does this too:

int main ( int argc, char **argv )
{
QApplication a( argc, argv );
a.setStyle( "Windows" );

qDebug() << a.children();
...
}

=>

QPAEventDispatcherGlib(0x10a9af0)
QSessionManager(0x10ac560)
QWindowsStyle(0x10b34f0, name = "windows")

Uwe

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Elvis Stansvik
Den mån 5 nov. 2018 kl 21:35 skrev Thiago Macieira :
>
> On Monday, 5 November 2018 12:07:15 PST Elvis Stansvik wrote:
> > If it is to be the same as all other QObjects, then it should maintain
> > its current behavior I think. The destruction of children happens in
> > the QObject destructor. I don't even think one have to look at the
> > QObject destructor docs to understand that - where else would it be
> > done, considering the parent/child mechanism is a mechanism common to
> > all QObject-derived classes
>
> Ah, but there's an exception: QWidget's destructor destroys its children, so
> that they get to see their parent before the QWidget ceases to be a QWidget.

Ah yes, I was talking about QObjects in general. QWidget is an
exception. Having hardly ever relied on that behavior more than
indirectly, I almost forgot about it.

But seems to me it would be a slippery slope to accept more
exceptions. What's next, will I have to implement the destruction
myself in my own widgets? :)

Elvis

>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
>
>
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Thiago Macieira
On Monday, 5 November 2018 12:07:15 PST Elvis Stansvik wrote:
> If it is to be the same as all other QObjects, then it should maintain
> its current behavior I think. The destruction of children happens in
> the QObject destructor. I don't even think one have to look at the
> QObject destructor docs to understand that - where else would it be
> done, considering the parent/child mechanism is a mechanism common to
> all QObject-derived classes

Ah, but there's an exception: QWidget's destructor destroys its children, so 
that they get to see their parent before the QWidget ceases to be a QWidget.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Elvis Stansvik
Den mån 5 nov. 2018 kl 20:58 skrev Tomasz Siekierda :
>
> On Mon, 5 Nov 2018 at 20:32, Konstantin Shegunov  wrote:
> >
> > Hello,
> > Since we couldn't agree, I'd love to see some more opinions about this 
> > one.[1]
> >
> > Specifically:
> > 1) Is parenting to the application object a thing?
>
> Never done it myself. But Q*Application is clearly marked as derived
> from QObject in the docs, so users can definitely expect it to behave
> the same as all other QObjects and clean up it's children properly.

If it is to be the same as all other QObjects, then it should maintain
its current behavior I think. The destruction of children happens in
the QObject destructor. I don't even think one have to look at the
QObject destructor docs to understand that - where else would it be
done, considering the parent/child mechanism is a mechanism common to
all QObject-derived classes

Elvis

>
> > 1.a) ... and should it be allowed (i.e. accepting the proposed change)?
> > 1.b) .. if not allowed, should we put a warning in the documentation that 
> > it is wrong and shouldn't be done at all, or at least that it's discouraged.
>
> Either is OK I think, with preference for 1.a). Note: these are not
> mutually exclusive - we could have the patch integrated & a warning in
> the docs that this is not encouraged.
>
>
> Disclaimer: I'm not a reviewer, nor approver, barely a contributor
> even, so feel free to ignore my opinion.
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Thiago Macieira
On Monday, 5 November 2018 11:57:44 PST Tomasz Siekierda wrote:
> On Mon, 5 Nov 2018 at 20:32, Konstantin Shegunov  
wrote:
> > Hello,
> > Since we couldn't agree, I'd love to see some more opinions about this
> > one.[1]
> > 
> > Specifically:
> > 1) Is parenting to the application object a thing?
> 
> Never done it myself. But Q*Application is clearly marked as derived
> from QObject in the docs, so users can definitely expect it to behave
> the same as all other QObjects and clean up it's children properly.
> 
> > 1.a) ... and should it be allowed (i.e. accepting the proposed change)?
> > 1.b) .. if not allowed, should we put a warning in the documentation that
> > it is wrong and shouldn't be done at all, or at least that it's
> > discouraged.
> Either is OK I think, with preference for 1.a). Note: these are not
> mutually exclusive - we could have the patch integrated & a warning in
> the docs that this is not encouraged.

The problem is that most Qt API is only supported while QCoreApplication still 
exists (and GUI stuff while QGuiApplication exists; widgets while QApplication 
exists). By the time the QObject destructor destroys the children of any of 
those, the QCoreApplication object no longer exists, so you're out of support.

That's why my opinion in the bug report was that you shouldn't do it.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Tomasz Siekierda
On Mon, 5 Nov 2018 at 20:32, Konstantin Shegunov  wrote:
>
> Hello,
> Since we couldn't agree, I'd love to see some more opinions about this one.[1]
>
> Specifically:
> 1) Is parenting to the application object a thing?

Never done it myself. But Q*Application is clearly marked as derived
from QObject in the docs, so users can definitely expect it to behave
the same as all other QObjects and clean up it's children properly.

> 1.a) ... and should it be allowed (i.e. accepting the proposed change)?
> 1.b) .. if not allowed, should we put a warning in the documentation that it 
> is wrong and shouldn't be done at all, or at least that it's discouraged.

Either is OK I think, with preference for 1.a). Note: these are not
mutually exclusive - we could have the patch integrated & a warning in
the docs that this is not encouraged.


Disclaimer: I'm not a reviewer, nor approver, barely a contributor
even, so feel free to ignore my opinion.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Re: [Development] Opinions on QTBUG-71545

2018-11-05 Thread Elvis Stansvik
Den mån 5 nov. 2018 kl 20:32 skrev Konstantin Shegunov :
>
> Hello,
> Since we couldn't agree, I'd love to see some more opinions about this one.[1]

I may be missing some detail, but I think what Thiago says makes
sense. When children are destroyed, you know you're in the QObject
destructor (from QObject::~QObject docs: "Destroys the object,
deleting all its child objects."), so you know the object is now a
QObject, no longer a QCoreApplication. If you require your
QCoreApplication to be alive by the time your child object is
destroyed, I think you have to ensure this on your own.

Like I said, I may be missing something, but that's what it looks like
to me. I can't see why there would be an exception to the object model
here.

Elvis

>
> Specifically:
> 1) Is parenting to the application object a thing?
> 1.a) ... and should it be allowed (i.e. accepting the proposed change)?
> 1.b) .. if not allowed, should we put a warning in the documentation that it 
> is wrong and shouldn't be done at all, or at least that it's discouraged.
>
> [1] https://bugreports.qt.io/browse/QTBUG-71545
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


[Development] Opinions on QTBUG-71545

2018-11-05 Thread Konstantin Shegunov
Hello,
Since we couldn't agree, I'd love to see some more opinions about this
one.[1]

Specifically:
1) Is parenting to the application object a thing?
1.a) ... and should it be allowed (i.e. accepting the proposed change)?
1.b) .. if not allowed, should we put a warning in the documentation that
it is wrong and shouldn't be done at all, or at least that it's discouraged.

[1] https://bugreports.qt.io/browse/QTBUG-71545
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development