[Interest] QSharedDataPointer detach too much ( don't call const )

Philippe philwave at gmail.com
Thu Sep 20 14:21:09 CEST 2018


I recommend you to use the data() / constData() API to reduce the risk of mistakes.

I personaly used a similar class, but in my case, the API names are called
data() / mutableData()

because I wish even more hilight on state changes.

Philippe

On Thu, 20 Sep 2018 13:19:47 +0200
Michal Lazo <xlazom00 at gmail.com> wrote:

Sorry 
> I just found, there will be just one detach.
> but still I wasn't aware that it don't call cost variant.
> So thx for that link  http://eel.is/c++draft/over.match
> 
> 
> 
> On Thu, Sep 20, 2018 at 1:15 PM Michal Lazo <xlazom00 at gmail.com> wrote:
> 
>> But for example 
>> http://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/text/qtextcursor.cpp#n1487
>> 
>> in this code I found at least 4 x detach
>> void QTextCursor::deleteChar()
>> {
>> if (!d || !d->priv)    1x detach
>> return;
>> 
>> 
>> if (d->position != d->anchor)            2x detach removeSelectedText(); return; }
>> {    
>> 
>> if (!d->canDelete(d->position))       only 1x detach
>> return;
>> d->adjusted_anchor = d->anchor = d->priv->nextCursorPosition(d->anchor, QTextLayout::SkipCharacters);    1x detach
>> d->remove();
>> d->setX();
>> }
>> 
>> 
>> 
>> 
>> 
>> On Thu, Sep 20, 2018 at 1:01 PM Giuseppe D'Angelo via Interest <interest at qt-project.org> wrote:
>> 
>> On 20/09/18 12:43, Michal Lazo wrote:
>>> > "So my whole C++ world just changed."
>>> > Can you point me to right article in C++ documentation ?
>>> 
>>> http://eel.is/c++draft/over.match
>>> 
>>> See in particular [over.match.funcs] ยง11.3.1.4 , and then you need to 
>>> reason about the implications for the overload resolution.
>>> 
>>> 
>>> > So Qt devs I found some code for example
>>> > http://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/text/qtextcursor.cpp#n1151
>>> > And it looks like it should be better to call const variant
>>> > what do you think ??
>>> 
>>> Possibly, but what's the big drama about that line?
>>> 
>>> 
>>> It's not a mistery that QSharedDataPointer is very prone to "accidental" 
>>> detaches, so its usage is frowned upon in new code, in favour of 
>>> QExplictlySharedDataPointer (which however requires manual calls to 
>>> detach(), and one needs to remember about them).
>>> 
>>> For instance, in a setter such as
>>> 
>>> > void MyClass::setFoo(int newFoo) {
>>> >     if (d->foo == newFoo)
>>> >         return;
>>> >     d->foo = newFoo;
>>> > }
>>> 
>>> (assuming d is a QSharedDataPointer<MyClassPrivate>) the d->foo access 
>>> in the if is supposed to be "readonly", yet it happily detaches the 
>>> private, even if the new value passed to the setter is identical to the 
>>> old value and thus one might expect that nothing needs to be done.
>>> 
>>> (No, noone is porting QTextCursor away from QSharedDataPointer at this 
>>> moment in time.)
>>> 
>>> 
>>> My 2 c,
>>> -- 
>>> Giuseppe D'Angelo | giuseppe.dangelo at 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
>>> 
>>> _______________________________________________
>>> Interest mailing list
>>> Interest at qt-project.org
>>> http://lists.qt-project.org/mailman/listinfo/interest



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/interest/attachments/20180920/d784b051/attachment.html>


More information about the Interest mailing list