Discussion:
[Development] Un-inlining members allowed?
Christian Ehrlicher
2018-10-21 18:07:38 UTC
Permalink
Hi,

one more question - is it ok to un-inline a function? For example I want
to move QListWidgetItem::isSelected() to the cpp file so I can properly
mark QListWidget::isItemSelected() as deprecated but I'm unsure if this
is allowed.

Thx,
Christian
Thiago Macieira
2018-10-21 20:12:21 UTC
Permalink
On Sunday, 21 October 2018 11:07:38 PDT Christian Ehrlicher wrote:
> Hi,
>
> one more question - is it ok to un-inline a function? For example I want
> to move QListWidgetItem::isSelected() to the cpp file so I can properly
> mark QListWidget::isItemSelected() as deprecated but I'm unsure if this
> is allowed.

De-inlining is binary and source compatible, so long as you accept that the
old code that did inline the function continues to do what it used o do.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Allan Sandfeld Jensen
2018-10-22 07:55:48 UTC
Permalink
On Sonntag, 21. Oktober 2018 20:07:38 CEST Christian Ehrlicher wrote:
> Hi,
>
> one more question - is it ok to un-inline a function? For example I want
> to move QListWidgetItem::isSelected() to the cpp file so I can properly
> mark QListWidget::isItemSelected() as deprecated but I'm unsure if this
> is allowed.
>
That should work, though it seems unnecessary. In any case you will need to
disable the warnings around the code using the deprecated API.

'Allan
Edward Welbourne
2018-10-22 09:52:52 UTC
Permalink
On Sonntag, 21. Oktober 2018 20:07:38 CEST Christian Ehrlicher wrote:
>>> one more question - is it ok to un-inline a function? For example I
>>> want to move QListWidgetItem::isSelected() to the cpp file so I can
>>> properly mark QListWidget::isItemSelected() as deprecated but I'm
>>> unsure if this is allowed.

Please be sure to summarise the change in a
[ChangeLog][Potentially Source-Incompatible Changes]
in your commit message; see [QUIP 6]

* [QUIP 6] https://quips-qt-io.herokuapp.com/quip-0006.html

Thiago Macieira (21 October 2018 22:12) replied:
>> De-inlining is binary and source compatible, so long as you accept
>> that the old code that did inline the function continues to do what it
>> used o do.

and (independently)
Allan Sandfeld Jensen (22 October 2018 09:55) replied:
> That should work, though it seems unnecessary. In any case you will
> need to disable the warnings around the code using the deprecated API.

This appears to be an instance of [QUIP 6]'s Examples section saying:

Issues not listed here should be discussed on the mailing-list and
then added here.

Unless someone else (e.g. Allan or Thiago) beats me to it (I'm busy with
a 3rd-party review ...), I guess I'll try to summarise the above as
another example; apparently this is in Category A. If someone else gets
there first, please add me as a reviewer, so I know when to update the
published version. (hmm ... I think an update may be in order anyway.)

Eddy.
Richard Gustavsen
2018-10-23 13:32:35 UTC
Permalink
Even if it's binary compatible, it sounds fragile. If the inlined function is e.g accessing symbols from the private class, someone might later come along and remove those symbols if they appear not to be used from anywhere, unknowingly of the fact that some apps still do through an old inlined version.


-Richard

________________________________
Fra: Development <development-bounces+richard.gustavsen=***@qt-project.org> på vegne av Edward Welbourne <***@qt.io>
Sendt: mandag 22. oktober 2018 11.52.52
Til: Christian Ehrlicher
Kopi: ***@qt-project.org; ***@qt-project.org; Thiago Macieira
Emne: Re: [Development] Un-inlining members allowed?

On Sonntag, 21. Oktober 2018 20:07:38 CEST Christian Ehrlicher wrote:
>>> one more question - is it ok to un-inline a function? For example I
>>> want to move QListWidgetItem::isSelected() to the cpp file so I can
>>> properly mark QListWidget::isItemSelected() as deprecated but I'm
>>> unsure if this is allowed.

Please be sure to summarise the change in a
[ChangeLog][Potentially Source-Incompatible Changes]
in your commit message; see [QUIP 6]

* [QUIP 6] https://quips-qt-io.herokuapp.com/quip-0006.html

Thiago Macieira (21 October 2018 22:12) replied:
>> De-inlining is binary and source compatible, so long as you accept
>> that the old code that did inline the function continues to do what it
>> used o do.

and (independently)
Allan Sandfeld Jensen (22 October 2018 09:55) replied:
> That should work, though it seems unnecessary. In any case you will
> need to disable the warnings around the code using the deprecated API.

This appears to be an instance of [QUIP 6]'s Examples section saying:

Issues not listed here should be discussed on the mailing-list and
then added here.

Unless someone else (e.g. Allan or Thiago) beats me to it (I'm busy with
a 3rd-party review ...), I guess I'll try to summarise the above as
another example; apparently this is in Category A. If someone else gets
there first, please add me as a reviewer, so I know when to update the
published version. (hmm ... I think an update may be in order anyway.)

Eddy.
Edward Welbourne
2018-12-04 14:14:06 UTC
Permalink
Back in October we discussed un-inlining of member functions.

On 22 October 2018 at 11:52 I promised:
> Unless someone else (e.g. Allan or Thiago) beats me to it (I'm busy with
> a 3rd-party review ...), I guess I'll try to summarise the above as
> another example; apparently this is in Category A. If someone else gets
> there first, please add me as a reviewer, so I know when to update the
> published version. (hmm ... I think an update may be in order anyway.)

Finally I got round to that:
https://codereview.qt-project.org/247357

I shall now update https://quips-qt-io.herokuapp.com/ to include that.
It already has updates for QUIPs 12 and 13.

Eddy.
Loading...