Re: [Development] What's the status of a moved-from object?

2019-05-27 Thread Konstantin Ritt
пн, 27 мая 2019 г. в 11:46, Edward Welbourne :

> сб, 25 мая 2019 г., 12:30 Mutz, Marc via Development
>  Repeat after me: default ctors do _not_ establish a valid value.
>
> As an aged C programmer I can respect that ;^>
>
> On 2019-05-25 17:24, Konstantin Ritt wrote:
> >>> Perhaps you mean "trivial type's default ctors do _not_ establish a
> >>> valid value"?
>
> сб, 25 мая 2019 г., 20:42 Mutz, Marc via Development <
> development@qt-project.org>:
> >> No, I actually meant default ctor. What should the default value be?
> >> _Obviously_ zero-initialisation. But oh, no, QSize is different. So
> >> maybe it's not _quite_ so obvious what the default value is. Why is
> >> pen's default value a black solid line instead of NoPen? Why is
> >> QBrush's default ctor not solid black, but NoBrush? See? It's just
> >> nonsense to try to pick a default value, so this is to say: Don't
> >> try.
> >>
> >> There's exactly one exception: containers. _Clearly_ no-one would
> >> _ever_ expect a container to start out anything but empty.
>
> Indeed.
>
> Konstantin Ritt (26 May 2019 10:51) replied:
> > Sure we should force a no-op default ctor where possible, and provide
> > some helper code that returns a valid default/specialized value of
> > that type where {0} isn't enough or non-intuitive
> > (i.e. QQuaternion::identity(), QRect::empty(), QSize::invalid()).
>
> (I can't help but wonder whether QRect::empty() should take a point at
> which to place its empty rectangle.  An animated rectangle that shrinks
> and expands repeatedly [0] shouldn't forget its location at the moment
> when it is transiently empty, so empty rectangles at distinct locations
> are distinct.  But I'm unfamiliar with QRect, so maybe not.)
>
> [0] example (in SVG), remembering position and orientation while empty:
> http://www.chaos.org.uk/~eddy/img/geom/transpyth.svg
>
> > There could [be] more exceptions when we're talking about non-trivial
> > types, at least some of them could be resolved by providing
> > convenience/helper initializers.
>
> Many types need an invalid value (this is the current default for
> QDate, QTime, QDateTime and QTimeZone).  But not all.
>
> For arithmetic types, up to and including QQuaternion and matrices,
> you'll need a zero-value; many (including square matrices) shall also
> need a unit value.  While zero is a natural default, being explicit is
> good; and standard integral/floating types don't default to it.
>
> Various others are (at present) constructed with default values.  In the
> case of QLocale, that can be controlled by calling a static method,
> QLocale::setDefault(), and we'd need to add (as someone tried to do a
> little while ago) a QLocale::defaultLocale() or QLocale::getDefault() to
> get that (note that we can't use the name QLocale::default() as it's a
> reserved word) if we stopped making the default constructor produce it.
> Note that QLocale's default isn't the same thing as QLocale::system(),
> although it starts out so, until setDefault() is called.
>
> We're not particularly consistent about what default construction
> produces; sometimes undefined, sometimes defined but invalid, sometimes
> a zero, sometimes a "reasonable" default.  Which, as Marc tripped over,
> is a source of confusion for authors of client code.  Having static
> methods to return invalid, zero, unit, default values could be made a
> consistent pattern, with consistent naming, to reduce confusion.
>
> We can, of course, do that without changing what the default-constructed
> objects are, but changing that also to a consistent pattern - the same
> as the moved-from state - would make matters clearer (for any choice of
> what moved-from state to consistently use).  But it'd be SiC.
>
> Eddy.
>

Completely agreed.


Konstantin
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-27 Thread Edward Welbourne
сб, 25 мая 2019 г., 12:30 Mutz, Marc via Development
 Repeat after me: default ctors do _not_ establish a valid value.

As an aged C programmer I can respect that ;^>

On 2019-05-25 17:24, Konstantin Ritt wrote:
>>> Perhaps you mean "trivial type's default ctors do _not_ establish a
>>> valid value"?

сб, 25 мая 2019 г., 20:42 Mutz, Marc via Development 
mailto:development@qt-project.org>>:
>> No, I actually meant default ctor. What should the default value be?
>> _Obviously_ zero-initialisation. But oh, no, QSize is different. So
>> maybe it's not _quite_ so obvious what the default value is. Why is
>> pen's default value a black solid line instead of NoPen? Why is
>> QBrush's default ctor not solid black, but NoBrush? See? It's just
>> nonsense to try to pick a default value, so this is to say: Don't
>> try.
>>
>> There's exactly one exception: containers. _Clearly_ no-one would
>> _ever_ expect a container to start out anything but empty.

Indeed.

Konstantin Ritt (26 May 2019 10:51) replied:
> Sure we should force a no-op default ctor where possible, and provide
> some helper code that returns a valid default/specialized value of
> that type where {0} isn't enough or non-intuitive
> (i.e. QQuaternion::identity(), QRect::empty(), QSize::invalid()).

(I can't help but wonder whether QRect::empty() should take a point at
which to place its empty rectangle.  An animated rectangle that shrinks
and expands repeatedly [0] shouldn't forget its location at the moment
when it is transiently empty, so empty rectangles at distinct locations
are distinct.  But I'm unfamiliar with QRect, so maybe not.)

[0] example (in SVG), remembering position and orientation while empty:
http://www.chaos.org.uk/~eddy/img/geom/transpyth.svg

> There could [be] more exceptions when we're talking about non-trivial
> types, at least some of them could be resolved by providing
> convenience/helper initializers.

Many types need an invalid value (this is the current default for
QDate, QTime, QDateTime and QTimeZone).  But not all.

For arithmetic types, up to and including QQuaternion and matrices,
you'll need a zero-value; many (including square matrices) shall also
need a unit value.  While zero is a natural default, being explicit is
good; and standard integral/floating types don't default to it.

Various others are (at present) constructed with default values.  In the
case of QLocale, that can be controlled by calling a static method,
QLocale::setDefault(), and we'd need to add (as someone tried to do a
little while ago) a QLocale::defaultLocale() or QLocale::getDefault() to
get that (note that we can't use the name QLocale::default() as it's a
reserved word) if we stopped making the default constructor produce it.
Note that QLocale's default isn't the same thing as QLocale::system(),
although it starts out so, until setDefault() is called.

We're not particularly consistent about what default construction
produces; sometimes undefined, sometimes defined but invalid, sometimes
a zero, sometimes a "reasonable" default.  Which, as Marc tripped over,
is a source of confusion for authors of client code.  Having static
methods to return invalid, zero, unit, default values could be made a
consistent pattern, with consistent naming, to reduce confusion.

We can, of course, do that without changing what the default-constructed
objects are, but changing that also to a consistent pattern - the same
as the moved-from state - would make matters clearer (for any choice of
what moved-from state to consistently use).  But it'd be SiC.

Eddy.
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-26 Thread Konstantin Ritt
сб, 25 мая 2019 г., 20:42 Mutz, Marc via Development <
development@qt-project.org>:

> On 2019-05-25 17:24, Konstantin Ritt wrote:
> > сб, 25 мая 2019 г., 12:30 Mutz, Marc via Development
> >> Repeat after me: default ctors do _not_ establish a valid value.
> >
> > Perhaps you mean "trivial type's default ctors do _not_ establish a
> > valid value"?
>
> No, I actually meant default ctor. What should the default value be?
> _Obviously_ zero-initialisation. But oh, no, QSize is different. So
> maybe it's not _quite_ so obvious what the default value is. Why is
> pen's default value a black solid line instead of NoPen? Why is QBrush's
> default ctor not solid black, but NoBrush? See? It's just nonsense to
> try to pick a default value, so this is to say: Don't try.
>
> There's exactly one exception: containers. _Clearly_ no-one would _ever_
> expect a container to start out anything but empty.



Sure we should force a no-op default ctor where possible, and provide some
helper code that returns a valid default/specialized value of that type
where {0} isn't enough or non-intuitive (i.e. QQuaternion::identity(),
QRect::empty(), QSize::invalid()).

There could more exceptions when we're talking about non-trivial types, at
least some of them could be resolved by providing convenience/helper
initializers.


Konstantin
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-25 Thread Mutz, Marc via Development

On 2019-05-25 17:24, Konstantin Ritt wrote:

сб, 25 мая 2019 г., 12:30 Mutz, Marc via Development

Repeat after me: default ctors do _not_ establish a valid value.


Perhaps you mean "trivial type's default ctors do _not_ establish a
valid value"?


No, I actually meant default ctor. What should the default value be? 
_Obviously_ zero-initialisation. But oh, no, QSize is different. So 
maybe it's not _quite_ so obvious what the default value is. Why is 
pen's default value a black solid line instead of NoPen? Why is QBrush's 
default ctor not solid black, but NoBrush? See? It's just nonsense to 
try to pick a default value, so this is to say: Don't try.


There's exactly one exception: containers. _Clearly_ no-one would _ever_ 
expect a container to start out anything but empty.


Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-25 Thread Konstantin Ritt
сб, 25 мая 2019 г., 12:30 Mutz, Marc via Development <
development@qt-project.org>:

> Further to the question about default ctors for such "obvious" stuff as
>
> On 2019-05-21 10:27, Mutz, Marc via Development wrote:
> > class QRect {
> >int x, y, w, h;
> > public:
> >QRect() = default;
> > };
> > QRect r; // partially-formed
> > r.x();   // compilers _already_ warn about this
> > QRect r = {}; // zero-initialized
>
> I've been working with Qt for two decades now. Guess what I just wrote
> and had to debug?
>
> QSize zero;
>
> Anyone spot the bug? Hint: the following /is/ correct:
>
> QPoint origin;
>
> Now, anyone here who wants to defend that as good API design?
>
> Anyone?
>
> Repeat after me: default ctors do _not_ establish a valid value.
>

Perhaps you mean "trivial type's default ctors do _not_ establish a valid
value"?

Konstantin

>
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-25 Thread Mutz, Marc via Development

Further to the question about default ctors for such "obvious" stuff as

On 2019-05-21 10:27, Mutz, Marc via Development wrote:

class QRect {
   int x, y, w, h;
public:
   QRect() = default;
};
QRect r; // partially-formed
r.x();   // compilers _already_ warn about this
QRect r = {}; // zero-initialized


I've been working with Qt for two decades now. Guess what I just wrote 
and had to debug?


   QSize zero;

Anyone spot the bug? Hint: the following /is/ correct:

   QPoint origin;

Now, anyone here who wants to defend that as good API design?

Anyone?

Repeat after me: default ctors do _not_ establish a valid value.

Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-25 Thread Giuseppe D'Angelo via Development

Hi,

Il 19/05/19 14:24, Giuseppe D'Angelo via Development ha scritto:

Hence, I'll ask here: what should the status of a moved-from object be?
I'm not really interested in_how_  to achieve such status (although of
course it's very important, and should influence the decision); I'm
interested in what's our contract with our users.



Trying to summarize the discussion so far. It seems to me that there's 
some agreement on these points:


1) We want move constructors for pimpled valued classes.

2) They must be noexcept, incl. not allocating memory.

3) At least for Qt 6's lifetime, the documented state of a moved-from 
object is "valid, but unspecified", à la stdlib, and NOT 
partially-formed. Any function without preconditions can be called on a 
moved-from instance; the results are unspecified. ("Unspecified" used in 
Standardese jargon here).


4) We will deliberately break source compatibility by adding move 
constructors to the classes currently lacking them (and thus copying 
instead of moving; leaving the source object in a well-defined state).


Hopefully, in the next years, static and runtime analyzers will be able 
to catch more and more usages of use-after-move. If we're really 
paranoid about this we could hide the move constructors behind a macro 
to avoid the SC break.


===

With these constraints, then we're left with "how do we do it"?

A) Allow for the d pointer to be nullptr; that's what gets set in the 
source object. To keep the object valid, this means checking the d 
pointer for nullptr in any function without preconditions (... read: in 
any member function, better safe than sorry). The cost of these extra 
checks is to be determined.


B) Introduce a "shared_null" for every pimpled class, under the form of 
a Q_GLOBAL_STATIC or similar. The move constructor resets the source 
object's d pointer to that shared_null.


As a by-product, both A) and B) can be used to make the default 
constructor not allocate and noexcept.



Am I forgetting anything?

Thank you,
--
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
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-24 Thread Giuseppe D'Angelo via Development

Hi,

Il 24/05/19 15:24, Simon Hausmann ha scritto:

Hi,


I don't think our users should have to use a build of Qt that has its 
internal Q_ASSERTs enabled. IMHO Q_ASSERT should be used by developers 
in Qt to express invariants that, if violated, represent a bug in Qt 
that needs fixing. I don't think we should use Q_ASSERT to communicate 
anything to our users - it's a poor interface because it either isn't 
reliable (-release build not enabling it) or hard to understand 
(debugger usage).





I disagree. Although we don't have a distinction between "internal" 
asserts (due to bugs in Qt) and "external" asserts (users misusing the 
APIs), as of now, Q_ASSERT is *also* used to enforce external asserts. 
Following your reasoning, all the assertions Qt has when e.g. checking 
for out of bounds access in a container should be removed or replaced by 
Qt-internal asserts, never firing in user code.


And, there is already everything in place to keep assertions enabled in 
release builds. The bottom problem here IMHO is the challenge at 
educating users (and/or providing builds in the SDK) that they have all 
these knobs they can tune: debug vs. release, asserts enabled vs. 
disabled, with or without debug prints, etc.


My 2c,
--
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
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-24 Thread Simon Hausmann
Hi,


I don't think our users should have to use a build of Qt that has its internal 
Q_ASSERTs enabled. IMHO Q_ASSERT should be used by developers in Qt to express 
invariants that, if violated, represent a bug in Qt that needs fixing. I don't 
think we should use Q_ASSERT to communicate anything to our users - it's a poor 
interface because it either isn't reliable (-release build not enabling it) or 
hard to understand (debugger usage).



Simon



From: Giuseppe D'Angelo 
Sent: Friday, May 24, 2019 14:28
To: Simon Hausmann; development@qt-project.org
Subject: Re: [Development] What's the status of a moved-from object?

Il 23/05/19 09:36, Simon Hausmann ha scritto:
> I think a well-formed state is more likely to enable our users to have a
> productive time. Operating - by accident - on a partially-formed object
> and either
>
>  (1) seeing a back-trace that points into Qt, not my application code
>
>  (2) spending time in the debugger stepping through Qt code
>
> is IMHO a direction that we should avoid.

To nitpick, in the proposed scenario, (2) shouldn't ever happen. In such
a scenario, the d-pointer checks would be replaced by something like
Q_ASSERT_X(d_ptr, "This object has been moved from"), which would fire
in a debug build of Qt. (1) is a bit more tricky as it wouldn't be
obvious at all what the problem is, although again, a debug build would
identify it immediately.

Thanks,
--
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
[https://www.kdab.com/wp-content/uploads/stories/Home-header-20yrs.jpg]<http://www.kdab.com/>
The Qt Experts - KDAB<http://www.kdab.com/>
www.kdab.com
KDAB - Software Experts in Qt, C++ and 3D / OpenGL. We have profund expertise 
in desktop and embedded software services for all kind of industries.


KDAB - The Qt, C++ and OpenGL Experts

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


Re: [Development] What's the status of a moved-from object?

2019-05-24 Thread Giuseppe D'Angelo via Development

Il 23/05/19 09:36, Simon Hausmann ha scritto:
I think a well-formed state is more likely to enable our users to have a 
productive time. Operating - by accident - on a partially-formed object 
and either


     (1) seeing a back-trace that points into Qt, not my application code

     (2) spending time in the debugger stepping through Qt code

is IMHO a direction that we should avoid.


To nitpick, in the proposed scenario, (2) shouldn't ever happen. In such 
a scenario, the d-pointer checks would be replaced by something like 
Q_ASSERT_X(d_ptr, "This object has been moved from"), which would fire 
in a debug build of Qt. (1) is a bit more tricky as it wouldn't be 
obvious at all what the problem is, although again, a debug build would 
identify it immediately.


Thanks,
--
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
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-23 Thread Simon Hausmann
Hi,

I favor a well-formed (null) state over a partially-formed state. I agree with 
the suggestion of adding null pointer checks into member functions where 
applicable.

I think a well-formed state is more likely to enable our users to have a 
productive time. Operating - by accident - on a partially-formed object and 
either

(1) seeing a back-trace that points into Qt, not my application code

(2) spending time in the debugger stepping through Qt code

is IMHO a direction that we should avoid.


Simon

From: Development  on behalf of Giuseppe 
D'Angelo via Development 
Sent: Sunday, May 19, 2019 14:24
To: development@qt-project.org
Subject: [Development] What's the status of a moved-from object?

Hi,

I'm trying to find a resolution for

> https://codereview.qt-project.org/#/c/261564/

TL;DR: the patch removes the move constructor from QColorSpace, because
that move constructor leaves the moved-from object in a partially-formed
state.

I have nothing against that idea (on the contrary), but I am against the
principle of introducing such inconsistencies in Qt without a previous
agreement.

Hence, I'll ask here: what should the status of a moved-from object be?
I'm not really interested in _how_ to achieve such status (although of
course it's very important, and should influence the decision); I'm
interested in what's our contract with our users.

A few datapoints for discussion are in the patch comments.

Thanks,
--
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

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


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Konstantin Ritt
вт, 21 мая 2019 г., 14:25 Mutz, Marc via Development <
development@qt-project.org>:

> On 2019-05-21 13:03, Konstantin Ritt wrote:
> > вт, 21 мая 2019 г., 11:32 Mutz, Marc via Development
> > :
> >
> >> And while the partially-formed
> >> state can be extended to non-pimpled classes easily:
> >>
> >> class QRect {
> >> int x, y, w, h;
> >> public:
> >> QRect() = default;
> >> };
> >> QRect r; // partially-formed
> >> r.x();   // compilers _already_ warn about this
> >> QRect r = {}; // zero-initialized
> >>
> >> That
> >> should be modelled by optional, not by QRect itself.
> >
> > Whilst the statement feels reasonable, this will require tons of API
> > changes and double checks on the user side:
>
> There are no double-checks. If the old code didn't check the rect before
> using it, it was a bug.
>

the first one is "does returned optional contain a rect?"
the second one "is rect empty?" // <- same check that was done in the "old"
code


> > optional Item::childrenRect() const
> > {
> > if (hasChildren()) {
> > QRect r = {};
> > for (auto *child : children())
> > r.unite(child->boundingRect().value_or({});
>
>for (auto *child : children())
>if (auto rect = child->boundingRect())
>r.unite(*rect);
>
> > return r;
> > }
> > return nullopt;
> > }
> >
> > QRect r = item->boundingRect().value_or({});
> > if (!r.isEmpty())
> > ~~~
>
> if (auto r = item->boundingRect())
>  use(*r);
>

there is nothing to use if 'r' has no value.
> The behavior is undefined if *this does not contain a value.
-- so value_or({}).
and then one would have to check if '*r' is valid/empty/normalized, just
like (s)he did before.


> > Note that I'm ok with that, but should we enforce such a huge efforts
> > all over Qt API just for making the default-constructible QRect a
> > no-op?
>
> This is not proposed for Qt 6. Not by me, at least. But it's the logical
> extension and once Qt can depend on C++17, iow: std::optional, it's the
> correct way forward, IMNSHO.
>
> Thanks,
> Marc
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development
>
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Mutz, Marc via Development

On 2019-05-21 13:03, Konstantin Ritt wrote:

вт, 21 мая 2019 г., 11:32 Mutz, Marc via Development
:


And while the partially-formed
state can be extended to non-pimpled classes easily:

class QRect {
int x, y, w, h;
public:
QRect() = default;
};
QRect r; // partially-formed
r.x();   // compilers _already_ warn about this
QRect r = {}; // zero-initialized

That
should be modelled by optional, not by QRect itself.


Whilst the statement feels reasonable, this will require tons of API
changes and double checks on the user side:


There are no double-checks. If the old code didn't check the rect before 
using it, it was a bug.



optional Item::childrenRect() const
{
if (hasChildren()) {
QRect r = {};
for (auto *child : children())
r.unite(child->boundingRect().value_or({});


  for (auto *child : children())
  if (auto rect = child->boundingRect())
  r.unite(*rect);


return r;
}
return nullopt;
}

QRect r = item->boundingRect().value_or({});
if (!r.isEmpty())
~~~


if (auto r = item->boundingRect())
use(*r);


Note that I'm ok with that, but should we enforce such a huge efforts
all over Qt API just for making the default-constructible QRect a
no-op?


This is not proposed for Qt 6. Not by me, at least. But it's the logical 
extension and once Qt can depend on C++17, iow: std::optional, it's the 
correct way forward, IMNSHO.


Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Mutz, Marc via Development

On 2019-05-21 12:52, Julien Cugnière wrote:

Le mar. 21 mai 2019 à 10:32, Mutz, Marc via Development
 a écrit :
It is, however, be an acceptable stop-gap measure, and here a 
compromise

may emerge, for keeping old code working while preparing for a
fully-implemented partially-formed state. This would mean we do assign
meaning to a nullptr d consistent with the documented default value in
Qt 5, but deprecate the use of the default ctor for establishing said
value in the docs and warn about the use of a default-constructed 
object

other than for destruction and assignment at runtime, even in release
mode. In Qt 7, we then crash, as we do for some moved-from objects
already.


Hi, I'm just a bystander, but I'm curious about this, and I wonder if
I misunderstood.

Are you saying that in modern C++, the recommendation is that the
default constructor of classes should leave the object in an
uninitialized state, where most member functions called will crash or
lead to undefined behavior? Do you have any links on the subject?


This has nothing to do with 'Modern C++'. It's how an int behaves since 
the dawn of time:


   int i; // uninited, can only be assigned to, or go out of scope 
(destroyed)

   int i = {}; // zero-initialized

In 2009 Alex Stepanov et al. wrote a little book called Elements of 
Programming in which they formalized this behaviour, calling it the 
Partially-Formed State, and extending it to all other value classes. 
Crucially, this was before C++ got move semantics. To my knowledge, 
https://www.kdab.com/stepanov-regularity-partially-formed-objects-vs-c-value-types/ 
is the first treatise that extends EoP's notion to C++11 and, in 
particular, move semantics.


A type doesn't need to establish just the partially-formed state. But 
users of types (and generic algorithms) may not assume more than that, 
either.



Are you suggesting that in Qt 7, we will have the following :

QRect r;
qDebug() << r.width(); // random value


No: UB. But also:

Compiler warning: r.m_w might be used uninitialised in this function.


QRect r = {};
qDebug() << r.width(); // OK, prints 0



QVector v;
qDebug() << v.size(); // crash


No. QVector is a container, their default is empty, and that's all 
right, because their default ctor already does the minimum amount of 
work necessary for the dtor to run. There's also no extra runtime check 
involved: size() { return begin() - end(); } where begin() == nullptr 
and end() == nullptr. And yes, just as now, v.front() invokes UB.


std::list is more interesting...


QString s;
qDebug() << s.indexOf("foo"); // crash


No the second. QString, too, is a container.


What about std::string and std::vector ?


They, too, are containers. And no, the standard does not use the 
partially-formed state. As a consequence, we have now 
std::variant::valueless_by_exception.


For more info, read EoP (it's out of print, though, and my copy ain't 
for sale) or my article linked above.


Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Konstantin Ritt
вт, 21 мая 2019 г., 11:32 Mutz, Marc via Development <
development@qt-project.org>:

> And while the partially-formed
> state can be extended to non-pimpled classes easily:
>
>  class QRect {
> int x, y, w, h;
>  public:
> QRect() = default;
>  };
>  QRect r; // partially-formed
>  r.x();   // compilers _already_ warn about this
>  QRect r = {}; // zero-initialized
>
> That
> should be modelled by optional, not by QRect itself.
>

Whilst the statement feels reasonable, this will require tons of API
changes and double checks on the user side:

optional Item::childrenRect() const
{
if (hasChildren()) {
QRect r = {};
for (auto *child : children())
r.unite(child->boundingRect().value_or({});
return r;
}
return nullopt;
}

QRect r = item->boundingRect().value_or({});
if (!r.isEmpty())
~~~


Note that I'm ok with that, but should we enforce such a huge efforts all
over Qt API just for making the default-constructible QRect a no-op?



Konstantin
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Julien Cugnière
Le mar. 21 mai 2019 à 10:32, Mutz, Marc via Development
 a écrit :
> It is, however, be an acceptable stop-gap measure, and here a compromise
> may emerge, for keeping old code working while preparing for a
> fully-implemented partially-formed state. This would mean we do assign
> meaning to a nullptr d consistent with the documented default value in
> Qt 5, but deprecate the use of the default ctor for establishing said
> value in the docs and warn about the use of a default-constructed object
> other than for destruction and assignment at runtime, even in release
> mode. In Qt 7, we then crash, as we do for some moved-from objects
> already.

Hi, I'm just a bystander, but I'm curious about this, and I wonder if
I misunderstood.

Are you saying that in modern C++, the recommendation is that the
default constructor of classes should leave the object in an
uninitialized state, where most member functions called will crash or
lead to undefined behavior? Do you have any links on the subject?

Are you suggesting that in Qt 7, we will have the following :

QRect r;
qDebug() << r.width(); // random value

QVector v;
qDebug() << v.size(); // crash

QString s;
qDebug() << s.indexOf("foo"); // crash

What about std::string and std::vector ?

-- 
Julien Cugnière
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Elvis Stansvik
Den tis 21 maj 2019 kl 10:57 skrev Giuseppe D'Angelo via Development
:
>
> Il 21/05/19 10:26, Elvis Stansvik ha scritto:
> > They will not show up in the web search (I think).
> >
> > They are however available in the debian-debug archive, see
> > https://wiki.debian.org/AutomaticDebugPackages
>
> [snip]
>
> Genuine question, are those just the debug builds of a release build, or
> are those debug builds? There is a difference when compiling Qt itself
> (amongst other things, release builds do not have Q_ASSERTs in them).

I'm not 100% sure, I think they are just packages containing symbols,
so the build is the same (a release build). I think Debian packagers
hang on this list so they would know for sure.

That would mean Q_ASSERT is a no-op then, even with those packages
installed, so they are just good for getting better backtraces.

Elvis

>
> Thanks,
> --
> 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
>
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Giuseppe D'Angelo via Development

Il 21/05/19 10:26, Elvis Stansvik ha scritto:

They will not show up in the web search (I think).

They are however available in the debian-debug archive, see
https://wiki.debian.org/AutomaticDebugPackages


[snip]

Genuine question, are those just the debug builds of a release build, or 
are those debug builds? There is a difference when compiling Qt itself 
(amongst other things, release builds do not have Q_ASSERTs in them).


Thanks,
--
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
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Konstantin Shegunov
On Tue, May 21, 2019 at 11:26 AM Elvis Stansvik  wrote:

> They will not show up in the web search (I think).
>
> They are however available in the debian-debug archive, see
> https://wiki.debian.org/AutomaticDebugPackages


Oh! You got me there. I was not aware of that. Thanks for pointing it out!
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Mutz, Marc via Development

Hi Lars,

There is a fine line between a proper partially-formed state and 
basically folding std::optional into every value class.


The former is a compile-time state. It is not observable at runtime, 
because any code which would observe a difference invokes UB. This is 
the same principle as C++11 data races, btw: Sequential Consistency for 
Data-Race-Free Programs.


The latter is a run-time property. As such, it will cost cpu cycles, 
death by a thousand paper cuts, sure, but cost. The cost is not 
necessarily in the extra branch, it's mostly in impairng the compiler's 
ability to optimize on the impossiblity of UB, which is important for, 
amongst other things, dead code removal. And while the partially-formed 
state can be extended to non-pimpled classes easily:


class QRect {
   int x, y, w, h;
public:
   QRect() = default;
};
QRect r; // partially-formed
r.x();   // compilers _already_ warn about this
QRect r = {}; // zero-initialized

QRect::isValid() has always been weird API, since it needs to reserve 
some valid, but non-normalized, values (remember, there's 
QRect::normalize()), to implement an optional absence of a rect. That 
should be modelled by optional, not by QRect itself.


To summarize: the partially-formed state is a compile-time only concept 
with no runtime cost and is applicable to all C++ types (from int over 
structs to Qt classes). It is consistent across all types, improves 
performance as well as readability of source code, and can be detected 
both at runtime (with asserts) and compile time (with static analysis).


A d == nullptr state as a valid value is a runtime property, has 
associated costs, and makes source code harder to understand.


It is, however, be an acceptable stop-gap measure, and here a compromise 
may emerge, for keeping old code working while preparing for a 
fully-implemented partially-formed state. This would mean we do assign 
meaning to a nullptr d consistent with the documented default value in 
Qt 5, but deprecate the use of the default ctor for establishing said 
value in the docs and warn about the use of a default-constructed object 
other than for destruction and assignment at runtime, even in release 
mode. In Qt 7, we then crash, as we do for some moved-from objects 
already.


Would that find your approval?

Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Elvis Stansvik
Den tis 21 maj 2019 kl 09:55 skrev Konstantin Shegunov :
>
> On Tue, May 21, 2019 at 9:49 AM Mutz, Marc via Development 
>  wrote:
>>
>> Oh, a nullptr deref is pretty clear in a backtrace.
>
>
> Indeed, but my point is that it's relatively useless for the user (or for the 
> developer for that matter). As you pointed out the dev sees the nullptr 
> dereferencing instantly, the user just gets a crash. The assert is just 
> superfluous is what I'm saying.
>
>>
>> Even in a release
>> build, if you installed the debug symbol package that does with the
>> distribution's version of Qt.
>
>
> Humor me, will you? [1]

They will not show up in the web search (I think).

They are however available in the debian-debug archive, see
https://wiki.debian.org/AutomaticDebugPackages

"The automatic dbgsym packages are available from a separate archive.
Please fetch these from http://deb.debian.org/debian-debug/ (backed by
http://debug.mirrors.debian.org/debian-debug/). They are also
available on http://snapshot.debian.org/. They are only available for
stable, proposed-updates, backports, testing, unstable and
experimental at this point in time, optionally also under the
release's codename. "

E.g. I if you run testing, think you would add:

deb http://deb.debian.org/debian-debug/ testing-debug main

to your sources.list.

For Ubuntu, I think the archive is called http://ddebs.ubuntu.com. E.g. I have

deb http://ddebs.ubuntu.com bionic main restricted universe multiverse
deb http://ddebs.ubuntu.com bionic-updates main restricted universe multiverse

here on Kubuntu 18.04, and I have e.g:

[estan@newton ~]$ apt-cache policy libqt5widgets5-dbgsym
libqt5widgets5-dbgsym:
  Installerad: 5.9.5+dfsg-0ubuntu2
  Kandidat:5.9.5+dfsg-0ubuntu2
  Versionstabell:
 *** 5.9.5+dfsg-0ubuntu2 500
500 http://ddebs.ubuntu.com bionic-updates/main amd64 Packages
100 /var/lib/dpkg/status
 5.9.5+dfsg-0ubuntu1 500
500 http://ddebs.ubuntu.com bionic/main amd64 Packages
[estan@newton ~]$

Cheers,
Elvis

>
>>
>> Please, you debug against a release build, and complain that you don't
>> get debug checks?
>
>
> To take your previous example.
>
> QPen pen1 = ~~~;
> QPen pen2 = std::move(pen1);
>
> // ... Some code
>
> pen1.color(); // Ooops
>
>> You're missing out on 4000 other checks that way, too!
>
>
> Yes, I already aknowledged that. And it's fine if I'm debugging into Qt. If 
> not, I get segfault. And as it happens I have enough builds, so even if that 
> happens and I can't switch kits and step into Qt, not every one user does 
> have that at their fingertips, however.
> Having documentation that spells such behaviour clearly, would be 
> satisfactory to me, that's why I was inquiring on *how* the problem, which is 
> a real one, can be mitigated.
>
>>
>> That said, I'm pretty sure that a static analyzer can find uses of
>> objects in partially-formed state with local analysis.
>
>
> QPen pen;
> QColor color = pen.color();
>
> If the analyzer can see the above as dangerous, i.e. touching an object with 
> an invalid state (i.e. d-ptr is null) as dangerous, I'm satisfied.
>
> [1]: 
> https://packages.debian.org/search?suite=testing=all=any=names=libqt5core
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Konstantin Shegunov
On Tue, May 21, 2019 at 9:49 AM Mutz, Marc via Development <
development@qt-project.org> wrote:

> Oh, a nullptr deref is pretty clear in a backtrace.


Indeed, but my point is that it's relatively useless for the user (or for
the developer for that matter). As you pointed out the dev sees the nullptr
dereferencing instantly, the user just gets a crash. The assert is just
superfluous is what I'm saying.


> Even in a release
> build, if you installed the debug symbol package that does with the
> distribution's version of Qt.
>

Humor me, will you? [1]


> Please, you debug against a release build, and complain that you don't
> get debug checks?
>

To take your previous example.

QPen pen1 = ~~~;
QPen pen2 = std::move(pen1);

// ... Some code

pen1.color(); // Ooops

You're missing out on 4000 other checks that way, too!
>

Yes, I already aknowledged that. And it's fine if I'm debugging into Qt. If
not, I get segfault. And as it happens I have enough builds, so even if
that happens and I can't switch kits and step into Qt, not every one user
does have that at their fingertips, however.
Having documentation that spells such behaviour clearly, would be
satisfactory to me, that's why I was inquiring on *how* the problem, which
is a real one, can be mitigated.


> That said, I'm pretty sure that a static analyzer can find uses of
> objects in partially-formed state with local analysis.
>

QPen pen;
QColor color = pen.color();

If the analyzer can see the above as dangerous, i.e. touching an object
with an invalid state (i.e. d-ptr is null) as dangerous, I'm satisfied.

[1]:
https://packages.debian.org/search?suite=testing=all=any=names=libqt5core
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Lars Knoll


> On 20 May 2019, at 20:18, Mutz, Marc via Development 
>  wrote:
> 
> On 2019-05-20 17:16, Thiago Macieira wrote:
>> On Monday, 20 May 2019 05:51:49 PDT Mutz, Marc via Development wrote:
>>> Or maybe we don't disagree at all and Thiago would accept allocating
>>> memory (or, by extension, anything that's noexcept(false)) as a very
>>> good reason to have a nullptr d?
>> I hadn't thought of noexcept, but let's be clear: yes, move constructors must
>> be noexcept. That might be the good reason why it can't reset to a valid
>> state.
> 
> What a feat it would be if Qt celebrated the 10th anniversary of the 
> publication of Elements of Programming with embracing the partially-formed 
> state :)
> 
> So, we seem to agree that moved-from objects may have d == nullptr. In the 
> following, let this be our premiss.

I fully agree with this/
> 
> Where we still, maybe, disagree, is whether d == nullptr is a valid state. 
> The difference is whether member functions other then destruction and 
> assignment check for a nullptr d. I'd propose that on classes under the above 
> premiss, Q_D contains Q_ASSERT(d). This, I think, strikes the best balance 
> between safety and speed. I think it's important to make using moved-from 
> objects an error, because it is. Trying to pamper it over by assigning some 
> magic meaning to a nullptr d is going to cause more problems than it solves 
> (std::variant::valueless_by_exception, anyone?).

If the default constructed object has a pointer to a shared_null or similar, I 
agree that Q_ASSERT(d) is the best option. But I’d like to question that. IMO 
we should reconsider that and change the default constructor to also set d to a 
nullptr.
> 
> If and when we accept this as policy going forward, the next question 
> becomes: What does the default ctor do? I fully realize that after decades of 
> constructing magic values at default construction time, Qt is in no position 
> to make default constructors set d = nullptr.

Why? I see no reason why we couldn’t do this and stay 100% source compatible. 
Yes, it would require checking d for nullptr in all methods, but I don’t think 
that’s costing a lot. And we gain something because default constructed objects 
don’t require a relocation to find the shared_null.

> For existing classes, the documented behaviour of the default constructor is 
> to establish a particular state (cf. QPen). But at least for new classes, we 
> should really think about having the default ctor do nothing more than d = 
> nullptr. And maybe deprecate the default constructor's value for Qt 7 or 8.

Agree with new classes, but as said above, I think we should strongly consider 
doing this for existing classes as well.
> 
> Why is a almost-no-op default ctor so important? Performance, yes. Noexcept, 
> yes. And there's need for this. Grep Qt::Initialization. People _need_ the 
> default ctors to do less work. Let's give it to them.

Fully agree here.
> 
> But I'd like to focus on something else here: Qt likes to pride itself for 
> good API design. So let's look at it from that angle:
> 
>   QPen pen1 = ~~~;
>   QPen pen2 = std::move(pen1);
>   QPen pen3;
> 
> What's the state of 'pen1' now? Well, QPen is actually a class that sets d = 
> nullptr in the moved-from object. So pen1 does not represent a value. This 
> behaviour is in Qt for ten(!) minor releases now. I didn't find a bugreport 
> about that. What about pen3? Well, black, solid, width=1 pen. Why do I know? 
> Because I looked it up. It makes the code hard to understand, because for 
> each class, you need to know what the default constructor does. Worse: We 
> don't know what the intent of the developer is here. Does she want to create 
> (a) a black pen, or does she (b) simply want to have _a_ pen, so she can 
> specify later what it should be? We don't know. We need to scan the code 
> further.

Having the default constructor set d to a nullptr nicely solves that. Pen1 and 
pen3 will have the same state, namely the one you get with a default 
constructor.

Cheers,
Lars

> 
> What is striking here is that a moved-from pen is _different_ than a 
> default-constructed one. Wouldn't it be ore intuitive if the states were the 
> same?
> 
> Under Stepanov's model
> 
>   QPen pen1 = Qt::black; // clearly (a)
>   QPen pen2; // clearly (b)
> 
> it's 100% clear that pen2 is just there to be assigned to later. So, (b). It 
> cannot possibly be (a), because a default-constructed QPen object does not 
> represent a valid pen. Furthermore, when pen1 is moved from, it will end up 
> in the same state as pen2 - partially-formed.
> 
> Can it get any simpler?
> 
> Thanks,
> Marc
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development

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


Re: [Development] What's the status of a moved-from object?

2019-05-21 Thread Mutz, Marc via Development

On 2019-05-20 22:58, Konstantin Shegunov wrote:
[...]

I agree as well, although I have a minor nitpick. Q_ASSERT works only
if it was not stripped while building Qt. Meaning that I'm often
working, as I'm sure other users, on Linux especially, with the
default (i.e. release) version of Qt from the repo. Granted it may be
my own fault, but in this case the assert isn't tripped and this code
simply segfaults for reasons that aren't so obvious.


Oh, a nullptr deref is pretty clear in a backtrace. Even in a release 
build, if you installed the debug symbol package that does with the 
distribution's version of Qt.



Any suggestions
how to mitigate that? Somehow *suggest* to the user that [s]he's
supposed to build in debug mode otherwise they're on their own?


Please, you debug against a release build, and complain that you don't 
get debug checks?


marc:~/Qt/qt5/qtbase$ git grep -we Q_ASSERT -- src | grep .cpp: | wc -l
3907

You're missing out on 4000 other checks that way, too!

And I'm pretty sure that there's a way to keep the Q_ASSERTs in release 
mode.


That said, I'm pretty sure that a static analyzer can find uses of 
objects in partially-formed state with local analysis. Proof:


   const char *p;

    complicated code ~

   char ch = *p; // WARNING: p might be used uninitialized in this 
function


I guess what's missing is a way to tag processes in which 
partially-formed states are created for the compiler.

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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Thiago Macieira
On Monday, 20 May 2019 12:05:58 PDT Allan Sandfeld Jensen wrote:
> I agree. I would consider anything other than deleting or reassigning  a
> moved object undefined behavior, so asserting on it seems like a good help
> to users of our APIs.

I agree, but we should leave this as a fallback case, albeit legitimate and 
not too difficult to justify. Here are a couple justifications:

- allocating memory (violates noexcept)
- retaining unknown quantities of memory (in the case of the assignment op)
- complex or slow code
- pessimises code elsewhere

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



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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Allan Sandfeld Jensen
On Montag, 20. Mai 2019 22:58:49 CEST Konstantin Shegunov wrote:
> On Mon, May 20, 2019 at 10:07 PM Allan Sandfeld Jensen 
> 
> wrote:
> > On Montag, 20. Mai 2019 20:18:32 CEST Mutz, Marc via Development wrote:
> > > Where we still, maybe, disagree, is whether d == nullptr is a valid
> > > state. The difference is whether member functions other then destruction
> > > and assignment check for a nullptr d. I'd propose that on classes under
> > > the above premiss, Q_D contains Q_ASSERT(d). This, I think, strikes the
> > > best balance between safety and speed.
> > 
> > I agree. I would consider anything other than deleting or reassigning  a
> > moved
> > object undefined behavior, so asserting on it seems like a good help to
> > users
> > of our APIs.
> 
> I agree as well, although I have a minor nitpick. Q_ASSERT works only if it
> was not stripped while building Qt. Meaning that I'm often working, as I'm
> sure other users, on Linux especially, with the default (i.e. release)
> version of Qt from the repo. Granted it may be my own fault, but in this
> case the assert isn't tripped and this code simply segfaults for reasons
> that aren't so obvious. Any suggestions how to mitigate that? Somehow
> *suggest* to the user that [s]he's supposed to build in debug mode
> otherwise they're on their own?

If you are using Linux, they typically has debug-packages, and some crash 
handlers (I know at least DrKonqi does), can suggest installing them and 
reloading the backgrace producing even better bugreporting information than 
you began with (even the basic ones would still tell you it is a nullptr 
access and in which function). I don't see how we could improve on that. A log 
entry or graceful error handling or exit would be more likely to missed by 
users.

Q_ASSERT is for developers, it is has nothing to do with end-users. Though 
actually now that I think about it, a developer would get just as good details 
from a nullptr segfault with full debug info, as from a Q_ASSERT, so it might 
not even be needed.


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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Giuseppe D'Angelo via Development

Il 20/05/19 22:58, Konstantin Shegunov ha scritto:
I agree as well, although I have a minor nitpick. Q_ASSERT works only if 
it was not stripped while building Qt. Meaning that I'm often working, 
as I'm sure other users, on Linux especially, with the default (i.e. 
release) version of Qt from the repo. Granted it may be my own fault, 
but in this case the assert isn't tripped and this code simply segfaults 
for reasons that aren't so obvious. Any suggestions how to mitigate 
that? Somehow *suggest* to the user that [s]he's supposed to build in 
debug mode otherwise they're on their own?


There is a long standing open feature request against Qt for having 
debug+release builds working on Linux.


TL;DR: we could come up with a Qt-specific solution, not a general 
purpose one (that would require agreement at the FHS level, 
distributions, and some ld.so mechanism à la OSX; in other words, 10 
years). No one wants to get their hands dirty in qmake code though, 
especially given it's dying; maybe something to consider during the 
CMake port.




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
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Konstantin Shegunov
On Tue, May 21, 2019 at 12:27 AM André Pönitz  wrote:

> > Somehow  *suggest* to the user that [s]he's supposed to build in
> > debug mode otherwise they're on their own?
>
> Nobody forces you to use Q_ASSERT.
>

Now you lost me. I don't, it was suggested as a means to prevent at-library
site accessing a partially formed state - i.e. for prevention of touching a
moved-from object that had its d-ptr nulled. I don't have access to the
d-ptr from the outside.
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread André Pönitz
On Mon, May 20, 2019 at 11:58:49PM +0300, Konstantin Shegunov wrote:
> I agree as well, although I have a minor nitpick. Q_ASSERT works only if it
> was not stripped while building Qt. Meaning that I'm often working, as I'm
> sure other users, on Linux especially, with the default (i.e. release)
> version of Qt from the repo. Granted it may be my own fault, but in this
> case the assert isn't tripped and this code simply segfaults for reasons
> that aren't so obvious. Any suggestions how to mitigate that?

Use your own fallback code that does what you want.

If yoy drive the Mars Rover and have a backup system that can take over
when you fail, signalling your failure early and decisively might be the
way to go (and yes, Q_ASSERT doesn't help in that case).

If you are the only one around, you are the master of ceremony, and
"The Show Must Go On", then doing "something" (logging, returning 
something likely/not completely broken) might be the only way out
(and yes, Q_ASSERT doesn't help in that case either).

> Somehow  *suggest* to the user that [s]he's supposed to build in
> debug mode otherwise they're on their own?

Nobody forces you to use Q_ASSERT.

Andre'

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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Konstantin Shegunov
On Mon, May 20, 2019 at 10:07 PM Allan Sandfeld Jensen 
wrote:

> On Montag, 20. Mai 2019 20:18:32 CEST Mutz, Marc via Development wrote:
> > Where we still, maybe, disagree, is whether d == nullptr is a valid
> > state. The difference is whether member functions other then destruction
> > and assignment check for a nullptr d. I'd propose that on classes under
> > the above premiss, Q_D contains Q_ASSERT(d). This, I think, strikes the
> > best balance between safety and speed.
>
> I agree. I would consider anything other than deleting or reassigning  a
> moved
> object undefined behavior, so asserting on it seems like a good help to
> users
> of our APIs.
>

I agree as well, although I have a minor nitpick. Q_ASSERT works only if it
was not stripped while building Qt. Meaning that I'm often working, as I'm
sure other users, on Linux especially, with the default (i.e. release)
version of Qt from the repo. Granted it may be my own fault, but in this
case the assert isn't tripped and this code simply segfaults for reasons
that aren't so obvious. Any suggestions how to mitigate that? Somehow
*suggest* to the user that [s]he's supposed to build in debug mode
otherwise they're on their own?
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Giuseppe D'Angelo via Development

Il 20/05/19 20:36, André Pönitz ha scritto:

I actually think we should consider getting rid of shared_null and
instead have d == nullptr as the null/default constructed state of the
object. Yes, that means we need to check for d == nullptr in member
functions, but I don’t think the overhead is a problem, as d will have
to be loaded into a register in any case.

In case someone submits such change I'd appreciate some performance
checking.


Not related to this email, but: please, we're switching from _what_ we 
want to _how_ we get it.


I understand the two are very tied (given the implicit sharing), but it 
seems to me that we are not agreeing on what a move constructor should 
do in the first place?


Thanks,
--
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
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Allan Sandfeld Jensen
On Montag, 20. Mai 2019 20:18:32 CEST Mutz, Marc via Development wrote:
> On 2019-05-20 17:16, Thiago Macieira wrote:
> > On Monday, 20 May 2019 05:51:49 PDT Mutz, Marc via Development wrote:
> >> Or maybe we don't disagree at all and Thiago would accept allocating
> >> memory (or, by extension, anything that's noexcept(false)) as a very
> >> good reason to have a nullptr d?
> > 
> > I hadn't thought of noexcept, but let's be clear: yes, move
> > constructors must
> > be noexcept. That might be the good reason why it can't reset to a
> > valid
> > state.
> 
> What a feat it would be if Qt celebrated the 10th anniversary of the
> publication of Elements of Programming with embracing the
> partially-formed state :)
> 
> So, we seem to agree that moved-from objects may have d == nullptr. In
> the following, let this be our premiss.
> 
> Where we still, maybe, disagree, is whether d == nullptr is a valid
> state. The difference is whether member functions other then destruction
> and assignment check for a nullptr d. I'd propose that on classes under
> the above premiss, Q_D contains Q_ASSERT(d). This, I think, strikes the
> best balance between safety and speed.

I agree. I would consider anything other than deleting or reassigning  a moved 
object undefined behavior, so asserting on it seems like a good help to users 
of our APIs.

I would add though that if the class has an isValid() method that method 
should be callable, and must returns false.  Not sure about a possible 
isNull() method :/ That whole subject btw, probably needs some cleanup. 
Wrappers with undefined, invalid and null values have been getting a bit mixed 
up in Qt APIs, and that gets confusing when some the values we wrap are stuff 
like JSON and SQL that has defined null values.

Best regards
'Allan







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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread André Pönitz
On Mon, May 20, 2019 at 01:56:56PM +, Lars Knoll wrote:
> > On 20 May 2019, at 14:51, Mutz, Marc via Development
> >  wrote:
> > 
> > On 2019-05-20 11:25, Giuseppe D'Angelo via Development wrote:
> >> Hi, Il 19/05/19 18:54, Thiago Macieira ha scritto:
> >>> But I think all Qt classes should go beyond that, unless they have
> >>> VERY good reasons not to do so (and document so). The moved-from
> >>> object should also be in a valid state so all the accessor and
> >>> mutation API in the class can operate in the object without ill
> >>> effects. What they actually do, we can't tell, since the initial
> >>> state is unknowable. So apply the principle of GIGO.
> >> So basically the same stance as the Standard Library? One should be
> >> able to invoke any function without preconditions on a moved-from
> >> object?
> > 
> > Except that the standard library has an easy way of implementing
> > that, since there're no PIMPLs. For a PIMPLed class, it means that
> > the move constructor either must allocate memory or that each and
> > every PIMPLed value class needs to have a static unsharable null
> > instance. This is relatively easy for some, but try that for QBrush.
> > Or we litter all member functions with nullptr checks.
> > 
> > I agree that a moved-from object should be in the same state as a
> > default-constructed one. I disagree with that that state must always
> > be a valid value of the class. I agree with Stepanov that the
> > default constructor should be establishing the partially-formed
> > state, ie, only destruction and assignment are valid. It _can_ do
> > more, but only if it stays noexcept.
> > 
> > Or maybe we don't disagree at all and Thiago would accept allocating
> > memory (or, by extension, anything that's noexcept(false)) as a very
> > good reason to have a nullptr d?
> 
> I actually think we should consider getting rid of shared_null and
> instead have d == nullptr as the null/default constructed state of the
> object. Yes, that means we need to check for d == nullptr in member
> functions, but I don’t think the overhead is a problem, as d will have
> to be loaded into a register in any case.

In case someone submits such change I'd appreciate some performance
checking.

Andre'
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Mutz, Marc via Development

On 2019-05-20 17:16, Thiago Macieira wrote:

On Monday, 20 May 2019 05:51:49 PDT Mutz, Marc via Development wrote:

Or maybe we don't disagree at all and Thiago would accept allocating
memory (or, by extension, anything that's noexcept(false)) as a very
good reason to have a nullptr d?


I hadn't thought of noexcept, but let's be clear: yes, move 
constructors must
be noexcept. That might be the good reason why it can't reset to a 
valid

state.


What a feat it would be if Qt celebrated the 10th anniversary of the 
publication of Elements of Programming with embracing the 
partially-formed state :)


So, we seem to agree that moved-from objects may have d == nullptr. In 
the following, let this be our premiss.


Where we still, maybe, disagree, is whether d == nullptr is a valid 
state. The difference is whether member functions other then destruction 
and assignment check for a nullptr d. I'd propose that on classes under 
the above premiss, Q_D contains Q_ASSERT(d). This, I think, strikes the 
best balance between safety and speed. I think it's important to make 
using moved-from objects an error, because it is. Trying to pamper it 
over by assigning some magic meaning to a nullptr d is going to cause 
more problems than it solves (std::variant::valueless_by_exception, 
anyone?).


If and when we accept this as policy going forward, the next question 
becomes: What does the default ctor do? I fully realize that after 
decades of constructing magic values at default construction time, Qt is 
in no position to make default constructors set d = nullptr. For 
existing classes, the documented behaviour of the default constructor is 
to establish a particular state (cf. QPen). But at least for new 
classes, we should really think about having the default ctor do nothing 
more than d = nullptr. And maybe deprecate the default constructor's 
value for Qt 7 or 8.


Why is a almost-no-op default ctor so important? Performance, yes. 
Noexcept, yes. And there's need for this. Grep Qt::Initialization. 
People _need_ the default ctors to do less work. Let's give it to them.


But I'd like to focus on something else here: Qt likes to pride itself 
for good API design. So let's look at it from that angle:


   QPen pen1 = ~~~;
   QPen pen2 = std::move(pen1);
   QPen pen3;

What's the state of 'pen1' now? Well, QPen is actually a class that sets 
d = nullptr in the moved-from object. So pen1 does not represent a 
value. This behaviour is in Qt for ten(!) minor releases now. I didn't 
find a bugreport about that. What about pen3? Well, black, solid, 
width=1 pen. Why do I know? Because I looked it up. It makes the code 
hard to understand, because for each class, you need to know what the 
default constructor does. Worse: We don't know what the intent of the 
developer is here. Does she want to create (a) a black pen, or does she 
(b) simply want to have _a_ pen, so she can specify later what it should 
be? We don't know. We need to scan the code further.


What is striking here is that a moved-from pen is _different_ than a 
default-constructed one. Wouldn't it be ore intuitive if the states were 
the same?


Under Stepanov's model

   QPen pen1 = Qt::black; // clearly (a)
   QPen pen2; // clearly (b)

it's 100% clear that pen2 is just there to be assigned to later. So, 
(b). It cannot possibly be (a), because a default-constructed QPen 
object does not represent a valid pen. Furthermore, when pen1 is moved 
from, it will end up in the same state as pen2 - partially-formed.


Can it get any simpler?

Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Edward Welbourne
On Monday, 20 May 2019 06:56:56 PDT Lars Knoll wrote:
 I actually think we should consider getting rid of shared_null and
 instead have d == nullptr as the null/default constructed state of
 the object. Yes, that means we need to check for d == nullptr in
 member functions, but I don’t think the overhead is a problem, as d
 will have to be loaded into a register in any case.

20.05.2019, 18:21, "Thiago Macieira" :
>>> It does introduce a compare-and-branch that wouldn't otherwise be
>>> there, but the cost is minimal compared to what most API would be
>>> doing, indeed.

In plenty of cases, it's just calling the same-named method on the d-ptr.

Result QThing::method(Type arg)
{
return Q_LIKELY(d) ? d->method(arg) : Result();
}

On 20.05.19 17:27, Konstantin Tokarev wrote:
>> We should wrap d == nullptr in Q_UNLIKELY() to make branch predictor
>> assume non-null path by default.

Olivier Goffart (20 May 2019 18:34) wrote:
> I would not do that. Q_UNLIKELY does very little to the branch
> predictor, it only tell the compiler to put that part of the branch in
> a cold area (and also optimize for size instead of for speed)
> Might not be a good idea considering that default constructed object
> is actually quite likely.

Is it ?  I would hope that code relatively seldom calls methods on
default-constructed objects or moved-from objects.  Is the behaviour
even well-defined ?  Aside, of course, from assignment and destruction,
the two places where I would indeed omit Q_UNLIKELY().

Eddy.
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Olivier Goffart

On 20.05.19 17:27, Konstantin Tokarev wrote:



20.05.2019, 18:21, "Thiago Macieira" :

On Monday, 20 May 2019 06:56:56 PDT Lars Knoll wrote:

  I actually think we should consider getting rid of shared_null and instead
  have d == nullptr as the null/default constructed state of the object. Yes,
  that means we need to check for d == nullptr in member functions, but I
  don’t think the overhead is a problem, as d will have to be loaded into a
  register in any case.


It does introduce a compare-and-branch that wouldn't otherwise be there, but
the cost is minimal compared to what most API would be doing, indeed.


We should wrap d == nullptr in Q_UNLIKELY() to make branch predictor assume
non-null path by default.


I would not do that. Q_UNLIKELY does very little to the branch predictor, it 
only tell the compiler to put that part of the branch in a cold area (and also 
optimize for size instead of for speed)
Might not be a good idea considering that default constructed object is 
actually quite likely.


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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Konstantin Tokarev


20.05.2019, 18:21, "Thiago Macieira" :
> On Monday, 20 May 2019 06:56:56 PDT Lars Knoll wrote:
>>  I actually think we should consider getting rid of shared_null and instead
>>  have d == nullptr as the null/default constructed state of the object. Yes,
>>  that means we need to check for d == nullptr in member functions, but I
>>  don’t think the overhead is a problem, as d will have to be loaded into a
>>  register in any case.
>
> It does introduce a compare-and-branch that wouldn't otherwise be there, but
> the cost is minimal compared to what most API would be doing, indeed.

We should wrap d == nullptr in Q_UNLIKELY() to make branch predictor assume
non-null path by default.

-- 
Regards,
Konstantin

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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Thiago Macieira
On Monday, 20 May 2019 06:56:56 PDT Lars Knoll wrote:
> I actually think we should consider getting rid of shared_null and instead
> have d == nullptr as the null/default constructed state of the object. Yes,
> that means we need to check for d == nullptr in member functions, but I
> don’t think the overhead is a problem, as d will have to be loaded into a
> register in any case.

It does introduce a compare-and-branch that wouldn't otherwise be there, but 
the cost is minimal compared to what most API would be doing, indeed.

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



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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Thiago Macieira
On Monday, 20 May 2019 05:51:49 PDT Mutz, Marc via Development wrote:
> Or maybe we don't disagree at all and Thiago would accept allocating
> memory (or, by extension, anything that's noexcept(false)) as a very
> good reason to have a nullptr d?

I hadn't thought of noexcept, but let's be clear: yes, move constructors must 
be noexcept. That might be the good reason why it can't reset to a valid 
state.

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



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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Lars Knoll
> On 20 May 2019, at 14:51, Mutz, Marc via Development 
>  wrote:
> 
> On 2019-05-20 11:25, Giuseppe D'Angelo via Development wrote:
>> Hi,
>> Il 19/05/19 18:54, Thiago Macieira ha scritto:
>>> But I think all Qt classes should go beyond that, unless they have VERY good
>>> reasons not to do so (and document so). The moved-from object should also be
>>> in a valid state so all the accessor and mutation API in the class can 
>>> operate
>>> in the object without ill effects. What they actually do, we can't tell, 
>>> since
>>> the initial state is unknowable. So apply the principle of GIGO.
>> So basically the same stance as the Standard Library? One should be
>> able to invoke any function without preconditions on a moved-from
>> object?
> 
> Except that the standard library has an easy way of implementing that, since 
> there're no PIMPLs. For a PIMPLed class, it means that the move constructor 
> either must allocate memory or that each and every PIMPLed value class needs 
> to have a static unsharable null instance. This is relatively easy for some, 
> but try that for QBrush. Or we litter all member functions with nullptr 
> checks.
> 
> I agree that a moved-from object should be in the same state as a 
> default-constructed one. I disagree with that that state must always be a 
> valid value of the class. I agree with Stepanov that the default constructor 
> should be establishing the partially-formed state, ie, only destruction and 
> assignment are valid. It _can_ do more, but only if it stays noexcept.
> 
> Or maybe we don't disagree at all and Thiago would accept allocating memory 
> (or, by extension, anything that's noexcept(false)) as a very good reason to 
> have a nullptr d?

I actually think we should consider getting rid of shared_null and instead have 
d == nullptr as the null/default constructed state of the object. Yes, that 
means we need to check for d == nullptr in member functions, but I don’t think 
the overhead is a problem, as d will have to be loaded into a register in any 
case.

Cheers,
Lars

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


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Mutz, Marc via Development

On 2019-05-20 11:25, Giuseppe D'Angelo via Development wrote:

Hi,

Il 19/05/19 18:54, Thiago Macieira ha scritto:
But I think all Qt classes should go beyond that, unless they have 
VERY good
reasons not to do so (and document so). The moved-from object should 
also be
in a valid state so all the accessor and mutation API in the class can 
operate
in the object without ill effects. What they actually do, we can't 
tell, since

the initial state is unknowable. So apply the principle of GIGO.


So basically the same stance as the Standard Library? One should be
able to invoke any function without preconditions on a moved-from
object?


Except that the standard library has an easy way of implementing that, 
since there're no PIMPLs. For a PIMPLed class, it means that the move 
constructor either must allocate memory or that each and every PIMPLed 
value class needs to have a static unsharable null instance. This is 
relatively easy for some, but try that for QBrush. Or we litter all 
member functions with nullptr checks.


I agree that a moved-from object should be in the same state as a 
default-constructed one. I disagree with that that state must always be 
a valid value of the class. I agree with Stepanov that the default 
constructor should be establishing the partially-formed state, ie, only 
destruction and assignment are valid. It _can_ do more, but only if it 
stays noexcept.


Or maybe we don't disagree at all and Thiago would accept allocating 
memory (or, by extension, anything that's noexcept(false)) as a very 
good reason to have a nullptr d?


Thanks,
Marc
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-20 Thread Giuseppe D'Angelo via Development

Hi,

Il 19/05/19 18:54, Thiago Macieira ha scritto:

But I think all Qt classes should go beyond that, unless they have VERY good
reasons not to do so (and document so). The moved-from object should also be
in a valid state so all the accessor and mutation API in the class can operate
in the object without ill effects. What they actually do, we can't tell, since
the initial state is unknowable. So apply the principle of GIGO.


So basically the same stance as the Standard Library? One should be able 
to invoke any function without preconditions on a moved-from object?


Cheers,
--
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
https://lists.qt-project.org/listinfo/development


Re: [Development] What's the status of a moved-from object?

2019-05-19 Thread Thiago Macieira
On Sunday, 19 May 2019 05:24:14 PDT Giuseppe D'Angelo via Development wrote:
> Hence, I'll ask here: what should the status of a moved-from object be?
> I'm not really interested in _how_ to achieve such status (although of
> course it's very important, and should influence the decision); I'm
> interested in what's our contract with our users.

I think there are two answers: what an object MUST be and what it SHOULD be. 
Obviously, the object must be destructible. And at a minimum, the object 
should be assignable, so a new, valid state can be created.

This is the bare minimum. I'd also add that the moved-from object does not 
hold non-negligible resources still allocated. Classes that allocate O(n) or 
worse memory, not O(1) like the d pointer.

But I think all Qt classes should go beyond that, unless they have VERY good 
reasons not to do so (and document so). The moved-from object should also be 
in a valid state so all the accessor and mutation API in the class can operate 
in the object without ill effects. What they actually do, we can't tell, since 
the initial state is unknowable. So apply the principle of GIGO.

Usually, the easiest way to accomplish that is for the moved-from object to be 
reset to the default-constructed state. So this is what the Qt move 
constructors and move assignment operators should be aiming at, by default, 
unless they have reasons not to.

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



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


[Development] What's the status of a moved-from object?

2019-05-19 Thread Giuseppe D'Angelo via Development

Hi,

I'm trying to find a resolution for


https://codereview.qt-project.org/#/c/261564/


TL;DR: the patch removes the move constructor from QColorSpace, because 
that move constructor leaves the moved-from object in a partially-formed 
state.


I have nothing against that idea (on the contrary), but I am against the 
principle of introducing such inconsistencies in Qt without a previous 
agreement.


Hence, I'll ask here: what should the status of a moved-from object be? 
I'm not really interested in _how_ to achieve such status (although of 
course it's very important, and should influence the decision); I'm 
interested in what's our contract with our users.


A few datapoints for discussion are in the patch comments.

Thanks,
--
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: S/MIME Cryptographic Signature
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development