[Development] Proposal: QVariant/std::any QModelIndex::internalData() for Qt 7

[Giuseppe D'Angelo]

Il 02/07/26 14:35, Bogdan Vatra via Development ha scritto:
> Hi,
> 
> @Igor, @Volker: Thanks for your feedback!
> 
> QPersistentModelIndex is not a proper solution here, for a practical reason:
> are you going to use QPMI *everywhere*? We experience crashes not only in our
> code but inside Qt's own code, QSortFilterProxyModel, QIdentityProxyModel,
> etc. which rely on internalPointer for their own bookkeeping. No amount of
> QPMI in application code fixes that. Using QPMI everywhere is also quite
> hacky; it's like having a QPersistentString that I must use to check whether
> my QString is still valid...
> 
> On "QModelIndex should be thought of like an STL iterator": the analogy is
> forced, because with an STL container *I* have full control. The iterator is
> only invalidated when *my* code changes the container. With QAIM, framework
> code invalidates indices behind my back: any Q*ModelView or delegate may call
> QCoreApplication::processEvents(), and Qt's own signals carry QModelIndex by
> value across queued connections, so an index can already be stale at the
> moment it is delivered. A single posted dataChanged (or any structural signal)
> is enough to make internalPointer point to something that no longer exists.
> This is the daily reality of highly dynamic models, the current contract is
> not just hard to follow, Qt itself breaks it on my behalf.

To be honest I am not following this reasoning.  What you're describing 
in the first message is a lifetime problem, which exists in an identical 
form with containers (well, it exists *everywhere* in C++), so the 
analogy is correct. If you have:

auto it = container.begin();
f();
use(*it);

Is this code OK? Maybe. You can't prove its correctness by local 
reasoning alone -- what if f() mutated the container?

That's pretty much the same situation you have in your original code:

> // get an idx
> myView->setCurrentIndex(idx);


This call might invalidate `idx` so subsequent usages are illegal.

Now maybe QPMI can help locally:

> QPersistentModelIndex pidx = idx;
> myView->setCurrentIndex(pidx);
> myView->scrollTo(pidx);

(possibly with somea additional validity checks, would you really call 
scrollTo with an invalid index?).

But is this a complete fix in the face of "anything can happen"? I don't 
think so; I'm pretty sure that there's stuff *inside* those calls that 
will malfunction if the index is invalidated (while the functions are 
running), and those functions are not going to create QPMIs to protect 
themselves from this eventuality.

Which is really the bottom line: when is a QModelIndex expected to be 
potentially invalidated, and thus one should place safeguards? I don't 
think this is clearly documented anywhere (unlike the STL, which 
documents the conditions for iterator invalidation). It's part of 
"common sense" -- or Hyrum's law around model/view usage. It's really a 
part of the hidden QAbstractItemModel contract.

For instance, would you expect data() to invalidate the passed index? 
Sure, it's a const method, but it could mutate the model in principle; 
if it did, any code that calls data() N times in a row on the same 
QModelIndex (e.g. to extract roles) get broken, or at least they may 
lose consistency.

Similarly, one could face reentrancy issues, like calling data() and 
getting a rowInserted/removed/dataChanged signals emitted -- while 
populating data structures with the contents of data(). This is prone to 
cause corruptions (as one typically sees in QSortFilterProxyModel).

Is making QModelIndex contain more data going to fix all these cases? In 
an adversarial situation, the answer is likely going to be "no". So is 
it really worth it? The original post said:

> myView->scrollTo(idx); // At this point there is no way to know if idx is 
> still valid. It depends on what `myView->setCurrentIndex(idx);` is doing and 
> there is no way to know if internalPointer is still valid or not. If inside of 
> that method QCoreApplication::processEvents is called, myView->scrollTo(idx) 
> gets an invalid idx...

This is a reentrancy issue, and the main reason why one should never 
spawn inner event loops. Can you get rid of those, instead? In an 
adversarial situation, what if that event loop just straight up destroys 
`myView` (or the model)? No amount of extra data in QModelIndex can fix 
that problem.

We're pretty much moving the line in the sand from a place to another 
(where the "invalidation contracts" are), where both places are actually 
very much unclear. However everyone will pay the price for this change 
-- QModelIndex is going to be much heavier, and in the overwhelming 
majority of use cases, no such fancy bookkeeping is needed. This isn't 
easily justifiable, especially since the use case is sketchy to begin with.



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 - Trusted Software Excellence
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4850 bytes
Desc: Firma crittografica S/MIME
URL: <http://lists.qt-project.org/pipermail/development/attachments/20260703/24fb36cf/attachment.bin>