int vs long in cython

[Nils Bruin]

I noticed the following cython code

        if S is long:
            return sage.rings.integer.long_to_Z()
        elif S is int:
            return sage.rings.integer.int_to_Z()

https://github.com/sagemath/sage/blob/30fecca1981087a88eb8db2cf05e18edbb50d16f/src/sage/rings/integer_ring.pyx#L589C1-L593C1

However, in cython with python3 we now have:

sage: cython("""
....: def tst():
....:     return int is long
....: """)
sage: tst()
True

so I think the `elif` can be deleted. Is that correct?

[Travis Scrimshaw]

I think so, but it might not hurt to have it.

Best,

Travis

[Nils Bruin]

well, I don't expect the C compiler to be smart enough to recognise the second is an "elif False:", so the "hurt" would be in additional code executed. Plus, having hidden "elif False:"s in a code base is a really bad code smell, so I think there is a penalty. What do you want to guard against? "int" and "long" becoming not synonyms in cython again? There will be probably other py2/3 relics in our code base. I think we should clean them up when encountered, unless we have a good reason not to.

[Nils Bruin]

I tried removal here:

https://github.com/sagemath/sage/pull/37420

and as expected it looks like it's working fine.

[Travis Scrimshaw]

Sorry for the delayed response.

If the C compiler can't tell the difference between an int and a long, then that suggests that there is still a difference. I am not sure that Cython has any specification that these are different. My understanding is Cython treats these as C types. Can we be certain that an int that is not a long will not find its way in? Could it also depend on someone's version of C/Python?

Best,

Travis

[Dima Pasechnik]

On Sun, Feb 25, 2024 at 9:48 AM Travis Scrimshaw <tcsc...@gmail.com> wrote:

Sorry for the delayed response.

If the C compiler can't tell the difference between an int and a long, then that suggests that there is still a difference. I am not sure that Cython has any specification that these are different. My understanding is Cython treats these as C types. Can we be certain that an int that is not a long will not find its way in? Could it also depend on someone's version of C/Python?

IMHO only if you use CPython 2 (which used short ints)

You probably still can shoot yourself in the foot in a Python extension by using short ints instead of long ones to interface with CPython, but you'll be warned  by the compiler about type mismatches.

 

--
You received this message because you are subscribed to the Google Groups "sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/eaebfae5-b265-4519-95dc-a52b8b849314n%40googlegroups.com.

[Nils Bruin]

Hi Travis,

No, these routines are about the python types. They get used as `type(x) is int` and `type(x) is long`. They are used in a different namespace from `cdef int x` and `cdef long x` and cdef long long x` (well, the `x` is in the cython object namespace. The C-types don't make it into the cython object namespace. They are in a C type namespace).

 The cython test routine above confirms that the objects `int` and `long` (not the cdef type names!) are indeed the same object in cython

The code below confirms they are the python `int` type object:

sage: cython("""

....: def test(x):
....:     return (x is long, x is int)
....: """)
sage: test(int)
(True, True)

[Travis Scrimshaw]

I am not sure it is purely about Python types since it gets changed into C code. (For reference, I changed your snippet to cpdef and got the same result too.)  It would be nice if there was an actual specification for this in Cython. I just get slightly worried after getting translated into C code, it then depends on the compiler.

Anyways, such changes should be fine (this was my initial reaction too) except in perhaps very extraordinary circumstances...

Best,

Travis

[Nils Bruin]

On Tuesday 27 February 2024 at 18:09:42 UTC-8 Travis Scrimshaw wrote:

I am not sure it is purely about Python types since it gets changed into C code.

well ... code dealing with python ints in Py3 needs to deal with multi-precision PyInt objects, so it can't unwrap these. Whether they are internally represented as "compact" integers is an implementation detail (see https://docs.python.org/3/c-api/long.html#c.PyUnstable_Long_IsCompact)

I have no succeeded in finding the exact place where the bindings for the `int` and `long` names are made for the cython namespace -- but we've already established that they refer in runtime to identical objects. This was a reasonable indication:

https://github.com/cython/cython/blob/03d982be010b7b94a3c3317c462b2b803ce916af/Cython/Compiler/Code.py#L46-L47

and for pure python mode, there are the type wrappers:

https://github.com/cython/cython/blob/03d982be010b7b94a3c3317c462b2b803ce916af/Cython/Shadow.py#L426-L427

 

(where we also see the instructive comment for `py_long` "# for legacy Py2 code only" -- so I think cython is indeed only keeping "long" bound for legacy reasons)

On the original ticket we actually found that `int_to_Z` was really just literally kept from Py2, so was actually buggy! (in Py2, a py_int was guaranteed to fit in a system integer, but in Py3 it's just a multiprec integer) So removing it was actually beneficial (although it was only used in a dead branch of the code, so can code that is guaranteed to be never called be buggy?)