[Development] Reverting the QRegExp change?

Giuseppe D'Angelo dangelog at gmail.com
Wed May 2 11:21:36 CEST 2012


On 2 May 2012 09:48, Thiago Macieira <thiago.macieira at intel.com> 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!)



More information about the Development mailing list