Hi Lisandro,

thanks for the feed-back.

I should note that I actually consider the current implementation also a
bit of a hack. The right place to implement this would be as part of the
builtin 'type' object, so that we could more easily support efficient
references to __new__, __init__, __call__, __add__, etc. But I think what's
there now will make life easier for users, as they no longer have to keep
header files around to do this. I'll be very happy if this can be ripped
back out in favour of a more general solution, but until then, it'll do its
job IMO.


Lisandro Dalcin, 31.10.2009 21:43:
> On Sat, Oct 31, 2009 at 10:18 AM, Stefan Behnel wrote:
>> I think this is safe, but I wanted to mention it here so that others can
>> give it another bit of thought.
> 
> Stefan, could you cythonize the line below:
> 
> cdef str s = str.__new__(str)
> 
> In the generated C code you will see a superfluous check "if(str is
> None): raise TypeError()" ... Can you figure out why this is
> happening?

Well, the above is also part of the tests, so I actually know that this is
happening - and I even know why! :)

The problem is that I wasn't sure how to figure out that a variable refers
to a builtin type. There isn't a None check generated for extension types
as they are easy to recognise, so if someone knows a good way to disable it
also for builtin types, please fix it.

That said, I doubt that the above has a major use case, and even with the
None pointer comparison (assuming that the C compiler actually generates
code for it), the optimised tp_new() call will still be faster than a
Python call to the __new__() method.

Note that there's also a str type check generated during the above
assignment, unless you add an additional <str> cast to the rhs. We could
potentially discard it, but given that __new__() can actually return an
arbitrary object (and given that it's not trivial to know when it's safe to
disable that type check), I chose to leave it in for now.


> BTW, this piece of code if far from being what CPython actually does :-) ...
> 
>   1064         elif type_arg.type_entry != obj.type_entry:
>   1065             # different types - do what CPython does at runtime
>   1066             error(type_arg.pos, "%s.__new__(%s) is not safe,
> use %s.__new__()" %
>   1067                   (obj.type_entry.name, type_arg.type_entry.name,
>   1068                    type_arg.type_entry.name))
>   1069             return node

You think so? I didn't read the code before, just did a couple of tests,
but I wasn't able to get anything but an exception out of __new__()
whenever I used two different types on both sides.

Anyway, the above check means that a user is trying to call tp_new() of one
type to instantiate another type. Given that Cython generates a separate
tp_new() for each extension type, this is close enough to the tp_new
pointer comparison check that CPython does here.

But I wouldn't mind dropping the above error in favour of a runtime
failure. That's not a major concern of the optimisation.

Stefan
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev

Reply via email to