[with patch, positive review] extern ctypedef integral <-> python object conversion

[dalcinl]

A long-waited feature: Cython should not assume the sizes for extern ctypedef integrals. That should be delegated to the C compiler using autogenerated conversion functions (ab)using of sizeof() for dispatching. The only required bit from users should be the specification of signed/unsigned.

Note: this ticket is also related to #255 and #303

[dagss]

According to mailing list comment this is awaiting review

[robertwb]

Mostly looks good, but I have some questions.

(1) It seems odd to have one method for non-typedef types, and another for typedef types. Perhaps we should do the same for all? (Perhaps not, I'm willing to hear arguments for it). (2) I was just thinking, we can automatically detect signness as well. ((type)-1) < ((type)0). Perhaps this is out of the scope of this patch. (3) __Pyx_PyInt_As%(SignWord)s%(TypeName)s makes a recursive call, which of course can't be inlined. I'm not sure there is an easy way to avoid this without making things ugly (like putting the whole body in a while true loop, which is essentially a goto). Maybe this isn't worth being optimized. (4) PyInt_Check and PyLong_Check are potentially unsafe (a subclass could override int) and slow, probably best to fall back on Pyx_PyNumber_Int if it's not exact.

[robertwb]

[Should havve previewed...]

(1) It seems odd to have one method for non-typedef types, and another for typedef types. Perhaps we should do the same for all? (Perhaps not, I'm willing to hear arguments for it).

(2) I was just thinking, we can automatically detect signness as well. ((type)-1) < ((type)0). Perhaps this is out of the scope of this patch.

(3) __Pyx_PyInt_As%(SignWord)s%(TypeName)s makes a recursive call, which of course can't be inlined. I'm not sure there is an easy way to avoid this without making things ugly (like putting the whole body in a while true loop, which is essentially a goto). Maybe this isn't worth being optimized.

(4) PyInt_Check and PyLong_Check are potentially unsafe (a subclass could override __int__) and slow, probably best to fall back on Pyx_PyNumber_Int if it's not exact.

[dagss]

This should also be done for floating point (#360, but can very likely be folded into this code with little effort).

Robert: -1 on your (2), there's no benefit to not assuming correct signedness declaration in external typedefs and it just complicates everything (like temp allocation of external typedefs!). (I could see generating code during module loading to just ensure it, purely for ensuring correctness, but that is definitely another patch/ticket.)

[dalcinl]

Very valuable input, Robert...

About (1): I do not see this as two different methods. The converters for builtin types are also employed for implementing the converters for typedef types.

About (2): Nice tick! I am trying this, and it seems to work... Dag: Why do you say this complicates things? temp allocation of external typedefs uses the typedef cname, where is the complication here?

About (3): AFAIK, some compilers can inline recursive functions up to some recursion depth (a depth of 1 would be enough for our case, right?). Not sure what GCC does here. However, if we can the change the implementation to a non-recursive one, I do not have any problem with that change.

About (4): I agree, this has to improve (and change Pyx_PyNumber_Int accordingly). However, just FYI note that in Py>=2.6, PyInt?_Check/PyLong_Check (and many other PyXXX_Check for builtin types) now use PyType?_FastSubclass. Howerver, this relies on ob_type->tp_flags inheritance, which is tricky for C types... I know that this Py2.6 change caused some headaches to NumPy? folks related to scalar types.

[dagss]

Re (2): Well, there's just so many things to consider and this seems to have little gain.

For instance, what happens with the usual "a//(b+c)" (where a temporary is needed to see if b+c == 0) if a, b and c are of three different external typedef types, and you decide that you don't want to know Cython-compile-time whether b and c are unsigned or not? What type should the temporary of the denominator be allocated as?

(Yes, there's a problem even with size being unknown, but there's insentive to get that resolved at some point because delaying resolving the type until C compilation time has huge benefits. However I can't see any benefits to delaying whether a type is signed or not.)

[dalcinl]

OK, I now understand your point, Dag...

Anyway, I'm working on an updated patch that does not requires knowledge of signed for implementing the from/to Python converters by using Robert's ((type)-1) < ((type)0) trick.

[robertwb]

Let me clarify for (1). What I'm seeing is that we already have size-agnostic conversions, so why not use them for the extern types as well. This would also fit in with only generating the definitions we need (i.e. not generating the builtin conversion for all the signed, unsigned, long long, short, etc. unless they're used)

[dalcinl]

Attached a reworked patch.

- The implementation is now simpler, the (type)-1)<((type)0) trick for signed/unsigned detection (Robert's comment [2]).

- Removed usage of PyInt?_Check/PyLong_Check, but I'm not 100% sure if this is fine (infinite recursion?) (Robert's comment [4])

[robertwb]

See also #176

[robertwb]

OK, looks good. I agree about [4], I've changed them back to check (no exact) just in case--perhaps we should revisit this later but this looks good and should go in. (That's nice the checks are a lot faster now.)

[robertwb]

I would still like to revisit the issue that conversions for unsigned int, short, etc. are generated for every module, whether or not they're needed, and how to avoid that.