On 2 May 2012 09:48, Thiago Macieira <[email protected]> wrote: > Given the negative reaction here and on Gerrit, I'm wondering if we should > revert it. > > In fact, for the simple fact that the SIC change wasn't discussed here before > it went in (my bad, sorry for that), it deserves to be reverted. > > What do you think?
That two kind of assestments for end-user code must be done: 1) checking the actual breakages (f.i. compiling Creator or KDE are good testcases) 2) checking if it's worth it to spend N days of work (Brad said he spent a full day to fix Creator) going around and replacing every rx.indexIn(...) with sth like QRegExp copy(rx); copy.indexIn() > Another option is what Olivier has proposed: > https://codereview.qt-project.org/#change,24986 Adding the const overloads to avoid breaking the source is a good idea. I'm not too happy with *that* implementation... the point of your change was 1) that a const method is not supposed to modify the object in any "visible" way; 2) to avoid to break the (totally undocumented) assumption that calling const methods on a reentrant class is actually thread safe; well: now those const overloads are not doing any of 1+2, bringing us to the situation before your change. So why dropping the const in the first place?! :) In a related patch I suggested something like int QRegExp::foo() const { QRegExp copy = *this; return copy.foo(); } which although being source compatible, is behaviour incompatible (QRegExp users do expect const methods to change the object -- it's even documented!). So it's a matter of deciding what's the least worst option. -- Giuseppe D'Angelo (For an extreme bikeshedding round, is anyone really relying on a const QRegExp object to modify itself? I tend to believe that if one is really interested in the captured texts, etc. then it's very likely that she's already using a non-const object!) _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
