[Eric] Bugs in latest eric4 snapshot

[Jürgen Urner]

Sorry, this went out private, not to the mailing list as intendet.



Andreas Pakulat schrieb:
> On 05.04.07 12:39:16, Jürgen Urner wrote:
>  
>> Andreas Pakulat schrieb:
>>    
>>> On 05.04.07 00:03:18, Jürgen Urner wrote:
>>>  
>>>      
>>>> Andreas Pakulat schrieb:
>>>>           
>>>>> Model indices are not persistent across application runs, but the Qt
>>>>> views do cache model indices that they retrieve from the model. As I
>>>>> said above, if you use internalPointer in python you need to make 
>>>>> sure
>>>>> to keep a reference to the object yourself, i.e. in a list or dict.
>>>>> Thats why I tend to use internalId and keep a dict of id->object 
>>>>> in the
>>>>> model.
>>>>>           
> ...
>  
>>> Anyway, I don't quite understand what you wanted to point at? The
>>> problematic code in eric4 does something like
>>>
>>> index = sourceModel.index(row,col)
>>> item = sourceModel.item(index)
>>>
>>> which is defined to be perfectly ok, i.e. the model can expect to get a
>>> proper index with a proper internalPointer or internalId, depending on
>>> which createIndex() overload it used. If an index turns out to be not
>>> invalid _and_ not have a proper id/pointer then the caller is doing
>>> something wrong. In the eric4 case I'm not sure yet, wether its the 
>>> view
>>> or the filter model, because I haven't looked at the rest of the code
>>> yet...
>>>
>>>       
>> In BrowserModel  a mapping is maintaned index.internalId --> 
>> BrowserItem.
>>     
>
> Right.
>
>  
>> In cretaeIndex() a reference to the parent item is attatched to an
>> index and later retrieved in calls to self.item_dict[index.internalId()]
>>     
>
> And here's at least 1 error. eric calls createIndex with the actual
> item, which means the internalPointer will return this item, however
> internalId will return some totally random value, which can't be used
> for indexing into the dict.
>   

Dono about the details... internalId() vs. internalPointer().
I stopped using these methods when running into these
problems. Anyway, you pass a python object. Qt should treat
is as pointer and don't care any longer, that is, wether the pointer
is valid or not at any time. Off course I may be wrrong here.


>  
>> The (Python) id or an item is mapped to items and, if I understand it 
>> right,
>> the item is retrieved later by its (Qt | Python ?) id.
>>     
>
> The model as it is available in the snapshot creates an index from an
> item+row+col, but later on tries to retrieve the id from the index. The
> pointer and Id don't have any connection between them, so one must be
> consistent by either using the id or the pointer but never mix them.
>   

The model retrieves an item by calling index.internalId() in most calls
as far as I can see. internalId() is not documented in depth (what is 
it? Is it
guaranteed to be unique? If yes, for how long?).

>  
>> client calls model.index(row,col)
>>    atatch pointer to parent python object to
>>    index and return index
>>     
>
> Its not the parent, but the current item for which the index is created
> normally. If you do that you can use internalPointer.
>   

The current item gets its parent item assigned as pointer for use
in model.parent() as far as I can see.

parent = self.item_dict[index.internalId()]


>  
>> Qt calls model.parent() and maybe queries other indices
>>    do something with pointer(s)
>>     
>
> No, Qt doesn't do anything with the internal pointers, or do you mean
> inside parent()? If so, yes the index that is given to parent will have
> its internalPointer set properly when you did the above thing, i.e.
> createIndex(row,col,object)
>   

Misunderstood. Qt does not do anything with the pointers. But it
queries index() or parent() multiple times and the implementation
does something with the pointers to process.


>  
>> client calls model.item(index)
>>    do something with pointer
>>     
>
> Right, and the client gets the index right before this call, so the
> index' internalPointer or Id must be valid. If its not something is very
> broken.
>   

Not necessarily broken. Just some pointer madness ;-) Qt does not
guarantee anything regarding these pointers ...or indices when it comes
to that.

>  
>> Somewhere in between these calls the pointer may get lost
>> and you may access an arbitrary memory location or get an index error.
>>     
>
> No, as long as the object that you gave to createIndex is still
> referenced somewhere in your application the internalPointer will still
> be the valid object. 

This is waht I would like to assume aswell, but I think this is a wrong
assumption. Your python object may be alive, but the index may point to
anywhere (for example to the refcount object in your traceback).
Most frameworks do it like this: keep a list of pointers and retrieve
items on demand from this list (whatever). Maybe that's why I expected
Qt do allow for the same processing. But the list is useless unless
someone guarantess that indices don't become invalid over time.
You may well pass arbitrary mem locations in the call to 
self.items_dict[index.internalId()].



> Thats why you need a list or dict of objects you
> already created (or create a second tree with those objects like the
> simpletreemodel in the PyQt4 examples). As I said earlier what the Qt
> views do internally doesn't concern us here, we can assume they do
> something along this if they need an index:
>
> index = model.index(row,col,parent)
> data = model.data(index, role)
>
> So the model can assume that every QModelIndex it gets was just created
> a few milli or microseconds ago by a call to index(). If the view wants
> to do caching, it needs to take special care to invalidate indexes in
> the model that are no longer valid. This is possible by connecting to
> the various signals a model sends out (rowsAboutToBeRemoved,
> layoutChanged, reset and so on). And thats also the reason why a model
> always has to call beginXXXRows/endXXXRows, to notify the view and
> others.
>   

Qt actually does much more. There may be dozens off calls to parent()
index() (...) in between two client calls.

++ rowsAboutToBeRemoved() is nowhere documented to cache indices.



>  
>> I guess all comes down to the interpretation of the word "emidiate" in
>> "indices are intended for emidiate use". I may be wrong, but that's 
>> how I
>> figured it out for myself.
>>     
>
> See above, generally you're assumed to retrieve an index first via the
> index() function and then do your computations with it. However it is
> possible to cache indices as long as care is taken that you forget about
> cached indices once the model has changed.
>
> Andreas
>
>   


Jürgen