[Development] Updating holdover API from Qt 1 times

[Sze Howe Koh]

On Tue, 20 Aug 2019 at 22:29, Matthew Woehlke <mwoehlke.floss at gmail.com> wrote:
>
> On 17/08/2019 00.13, Sze Howe Koh wrote:
> >  QLabel returns some CoW types by-pointer as a legacy from Qt 1 times [1]:
> >
> >     QPixmap *QLabel::pixmap() const;
> >     QPixture *QLabel::pixmap() const;
>
> Does this allow one to obtain the internal pixmap and modify it in-place
> without then calling setPixmap? If so, note that changing these to
> return by value represents a non-trivial change to the API. (Although,
> perhaps that's a change we *want*...)

No, it's actually a pointer-to-const-value which forbids in-place modifications:

    const QPixmap *QLabel::pixmap() const;


> > 3) Pick one of the options above, but go one step further and use
> > std::optional (C++17) instead of returning null objects.
>
> Ugh... I'm with Kevin; I think this doesn't make sense and adds a level
> of indirection for no good reason that will only make the API's harder
> to use.
>
> Please choose based on what produces the best API in the end, not the
> least porting effort. Otherwise we are just trading one sub-optimal API
> for another.


Thank you for your input, Marc, Kevin, and Matthew.

I agree with Kevin that std::optional doesn't make sense for
implicitly-shared objects. Furthermore, using std::optional here still
leaves the Qt API in an inconsistent state.

Therefore, I've started implementing option #2:

* https://codereview.qt-project.org/c/qt/qtbase/+/272365
* https://codereview.qt-project.org/c/qt/qtbase/+/274029


Regards,
Sze-Howe