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 | [email protected] | 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
smime.p7s
Description: Firma crittografica S/MIME
-- Development mailing list [email protected] https://lists.qt-project.org/listinfo/development
