These are chat archives for dropbox/pyston

21st
Jun 2016
Sun
@Daetalus
Jun 21 2016 19:58
How to set has_getattribute for an extension type?
Kevin Modzelewski
@kmod
Jun 21 2016 20:24
It should be getting set automatically
what issue are you running into?
Marius Wachtler
@undingen
Jun 21 2016 20:25
I think if it's not getting set is a syndrome of a missing PyType_Ready call..
Sun
@Daetalus
Jun 21 2016 20:28
For a PyFortran_Type, it set tp_getattr, but the inst->cls->has_getattribute is false. So trigger an assert error in here: https://github.com/dropbox/pyston/blob/master/src/capi/abstract.cpp#L411.
I think Marius is correct.
Kevin Modzelewski
@kmod
Jun 21 2016 20:30
oh, yeah that makes sense
Sun
@Daetalus
Jun 21 2016 20:31
What about switch to CPython macro about the issue in dropbox/pyston#1260
Kevin Modzelewski
@kmod
Jun 21 2016 20:32
I don't think macro-vs-function is necessarily the culprit
it's that we changed it to a function that has slightly different behavior
we've run into this thing in the past and had to add a new function that macro calls, which has the behavior of the old macro
So in this case it could be _PyCFunction_GetFunction and it allows subclasses
We might also just be able to modify PyCFunction_GetFunction but sometimes that introduces its own compatibility issues
Sun
@Daetalus
Jun 21 2016 20:36
I think this is not the subclass issue. The func in that context was __pyx_CyFunctionType_type, which is "cython_function_or_method". Its base class is type, not PyCFunction_Type. So we can't let it work by allows subclass.
Kevin Modzelewski
@kmod
Jun 21 2016 20:49
hmm that sounds like its metaclass, not its base class
if its base class is type, then PyCFunction_GetFunction will probably segfault
Sun
@Daetalus
Jun 21 2016 20:49
indeed. It currently get segfault.
Kevin Modzelewski
@kmod
Jun 21 2016 20:50
do you have a link to the cython/scipy code?
It will return NULL, then get segfault in 1397
Kevin Modzelewski
@kmod
Jun 21 2016 20:52
I thought you said it would assert?
If you changed it back to the macro, I think that will just do something random right now :(
Sun
@Daetalus
Jun 21 2016 20:52
I thought you said it would assert?
That's another issue. Sorry for mixing.

If you changed it back to the macro, I think that will just do something random right now

But that's what CPython did, right?

Kevin Modzelewski
@kmod
Jun 21 2016 20:53
they have a macro that matches their data structures
if you enable their macro, we're going to have a macro that doesn't match our data structures, since ours our different
we could write a different macro if we wanted
Sun
@Daetalus
Jun 21 2016 20:56
ok. For PyFortran_Type and has_getattribute. Seems it already called PyType_Ready(if PyFortran_Type->tp_dict != NULL means it get initialized), but didn't set has_getattribute.
Kevin Modzelewski
@kmod
Jun 21 2016 20:56
oh interesting, it looks like we have some mixups inside our code
or rather, we took their PyCFunction_Type and split it into two separate types
Sun
@Daetalus
Jun 21 2016 20:57
CPython has PyCFunction_GetFunction, and also has a macro
Kevin Modzelewski
@kmod
Jun 21 2016 20:58
why does the function version not work for you?
I'm not sure what you are focusing on as the difference between the function and the macro
Sun
@Daetalus
Jun 21 2016 21:03
The argument of that function is func. In some situation, the type of it is __pyx_CyFunctionType. Not PyCFunction_Type. So in function version, it will return NULL at here: https://github.com/dropbox/pyston/blob/master/src/runtime/capi.cpp#L1574 But CPython's macro version didn't do that type check:
#define PyCFunction_GET_FUNCTION(func) \
          (((PyCFunctionObject *)func) -> m_ml -> ml_meth)
Kevin Modzelewski
@kmod
Jun 21 2016 21:06
Ok but why don't you like the idea of having PyCFunction_GET_FUNCTION call _PyCFunction_GetFunction that doesn't have the type checking?
Anyway, it looks like I was wrong and we do mirror the PyCFunctionObject struct
so you can reenable those macros if you want
in general though we do sometimes have to use the other approach of adding a new function for the macro to call
Sun
@Daetalus
Jun 21 2016 21:08
I'm ok with this idea. I'm just say it is not related with subclass.
So call _PyCFunction_GetFunction is acceptable?
Kevin Modzelewski
@kmod
Jun 21 2016 21:17
In this particular case, reenabling the macros looks fine
since we already took the time to make sure the data structures line up