[Development] RFC: Deprecating setSharable / isSharable in our containers and QString

[Knoll Lars]

On 20/02/14 08:32, "Thiago Macieira" <thiago.macieira at intel.com> wrote:

>... and removing them in Qt 6
>
>Please raise your hand if you knew we had that feature.
>
>Please raise your hand if you've ever used them in your own code.
>
>I doubt anyone has raised their hands to the second question and there
>mustn't 
>be more than 2 people who did for the first question. This is a very
>obscure 
>feature in our containers, which are normally implicitly shared: they can
>be 
>made unsharable.
>
>However, this is not a performance optimisation. All non-const calls will
>still check the reference count to see if they need to detach or not.
>Though 
>you may be sure that the calls will not throw exceptions, those functions
>are 
>still noexcept(false). So code generation is still bad.
>
>And most importantly, we're paying the price for this obscure feature in
>EVERY 
>SINGLE ref or deref of any container using QArrayData or QRefCounter.
>That's 
>one extra comparison in each of those, plus the generation of the copy
>code 
>for the ref-up.
>
>I'd like to remove the feature in Qt 6. Opinions?
>
>
>
>Where it's used in Qt 5 (aside from tests):
>
>src/corelib/tools/qiterator.h:    { c->setSharable(false); i =
>c->begin(); n = 
>c->end(); } \
>src/corelib/tools/qiterator.h:    { c->setSharable(true); } \
>src/corelib/tools/qiterator.h:    { c->setSharable(true); c = &container;
>c-
>>setSharable(false); \
>src/corelib/tools/qiterator.h:    { c->setSharable(false); i =
>c->begin(); n = 
>c->end(); } \
>src/corelib/tools/qiterator.h:    { c->setSharable(true); } \
>src/corelib/tools/qiterator.h:    { c->setSharable(true); c = &container;
>c-
>>setSharable(false); i = c->begin(); n = c->end(); return *this; } \
>src/corelib/tools/qset.h:    { c->setSharable(false); i = c->begin(); n =
>c-
>>end(); }
>src/corelib/tools/qset.h:    { c->setSharable(true); }
>src/corelib/tools/qset.h:    { c->setSharable(true); c = &container; c-
>>setSharable(false);
>
>Read: Java-style mutable iterators. And the above is slightly broken,
>because 
>you can always just create a second mutable iterator on the same
>container, 
>which would set it back to sharable when the iterator got destroyed. It
>also 
>doesn't stop the iterator from getting invalidated if you modify the
>container 
>outside the iterator.
>
>STL-style iterators currently have the requirement that you should not
>create 
>a copy of the container while iterating for mutation. And even then, I
>can't 
>see right now an operation in the Java-style mutable iterators that would
>break by removing those setSharable() calls, not after the fixes to
>insert() 
>we've done.


+1 for deprecation/removing from my side.

Cheers,
Lars