Re: [pygccxml-development] Serious bug in pygccxml (scopedef.py)

[Matthias B. <ba...@ir...>]

Roman Yakovenko wrote:
> I committed proposed fixed

Thanks!

>> I have to admit that I dont't really understand why those operators are
>> overridden at all.
> 
> pygccxml.project_reader_t has 2 modes to parse group of files. I
> needed to write some
> test to check that both methods return same answer.
> 
> Also I needed to test cache algorithm.
> 
> In general those 2 methods exists for unit tests.
> 
>> In which case does it happen that two separate
>> declaration objects refer to the same declaration?
> 
> There is no such case
> 
>> (probably when a
>> declaration is included in two separate include files, right? But why
>> isn't the first declaration then reused in the first place?).
> 
> It is do re-used.

But if those special == and != operators are only required for unit 
testing, wouldn't it then be better to move them to the unit tests and 
keep the standard operators for the regular users? From what I've seen 
the operators implement rather expensive operations (copying, sorting, 
comparing several values, etc.) and if this could just as well be done 
by comparing the reference of the instance, this would be much more 
efficient.

>> But I think for someone else reading the code this is not really
>> obvious why I have to do that...
> 
> Matthias, it is not the first time I provide "not so user friendly" interface.
> Can you show some code you have to write now and how I think it should
> be written.

It's in the __call__() method of the ParentFilter class in 
experimental/filters.py. First, the method looked like this:

     def __call__(self, decl):
         return decl.parent==self.parent

Because of the bug in the == operator I had a look at the implementation 
and found out that the operator is doing quite a lot things. With the 
latest fix you've applied to scopedef.py the above code would work 
again, but it would not be as efficient as the following line:

         return id(decl.parent)==id(self.parent)

>> PS: I also don't understand the purpose of the (static!) method
>> _sorted_list() as this method just calls sort() and does nothing else.
>> Why wasn't sort() used in the first place?
> 
> Convenience. Python sorts list in place and return value of sort
> function is None.
> This function allows you to write something like this:
> 
> soorted_list ( x ) == sorted_list( y )
> 
> while without function you need to write
> 
> x.sort()
> y.sort()
> x == y

ok, I see. But sorted_list() has the side effect of modifying its 
argument (which might not have been intentional). That's why the Python 
developers chose not to return the list from sort() (see note (7) here: 
http://docs.python.org/lib/typesseq-mutable.html)
Maybe the semantics you actually had in mind was this:

     def _sorted_list( some_list ):
         result = copy.copy(some_list)
         result.sort()
         return result

But of course, this makes the operation more expensive...

- Matthias -