Bug, or changed array assignment in 0.17beta1?
import bug bug.foo()
import bug bug.foo()
Hello All, The exact behavior of array assignment was never entirely clear to me, but I am certain the following behavior did not occur in 0.16: --------bug.pyx---------- def foo(): cdef int i cdef int* p1 = [4, 4] cdef int* p2 = [5, 5] print "p1:", for i in range(2): print p1[i], print "\np2:", for i in range(2): print p2[i], ----------------------------- which in Cython 0.17beta1 gives me p1: 5 5 p2: 5 5 while in Cython 0.16 I get p1: 4 4 p2: 5 5 Has the syntax of array assignment changed, or is this a bug? If the former, how do you assign to an array literal? Thanks for your all your efforts, Mike
Hi, thanks for the report. Mike Zaletel, 25.07.2012 00:40:
The exact behavior of array assignment was never entirely clear to me
Yes, it's not entirely obvious.
but I am certain the following behavior did not occur in 0.16:
--------bug.pyx----------
def foo(): cdef int i cdef int* p1 = [4, 4] cdef int* p2 = [5, 5]
print "p1:", for i in range(2): print p1[i], print "\np2:", for i in range(2): print p2[i],
-----------------------------
which in Cython 0.17beta1 gives me
import bug bug.foo() p1: 5 5 p2: 5 5
while in Cython 0.16 I get
import bug bug.foo() p1: 4 4 p2: 5 5
The problem is that the same (temporary) local array variable is used in both cases to build the array, and then only a pointer is assigned, i.e. p1 and p2 then point to the same array, which gets overwritten with the new values in the second assignment. Stefan
Stefan Behnel, 25.07.2012 07:40:
Mike Zaletel, 25.07.2012 00:40:
--------bug.pyx---------- def foo(): cdef int i cdef int* p1 = [4, 4] cdef int* p2 = [5, 5]
print "p1:", for i in range(2): print p1[i], print "\np2:", for i in range(2): print p2[i], -----------------------------
which in Cython 0.17beta1 gives me
import bug bug.foo() p1: 5 5 p2: 5 5
while in Cython 0.16 I get
import bug bug.foo() p1: 4 4 p2: 5 5
The problem is that the same (temporary) local array variable is used in both cases to build the array, and then only a pointer is assigned, i.e. p1 and p2 then point to the same array, which gets overwritten with the new values in the second assignment.
Oh, just in case I wasn't clear: you've found a bug. Stefan
Stefan Behnel, 25.07.2012 07:40:
Mike Zaletel, 25.07.2012 00:40:
--------bug.pyx----------
def foo(): cdef int i cdef int* p1 = [4, 4] cdef int* p2 = [5, 5]
print "p1:", for i in range(2): print p1[i], print "\np2:", for i in range(2): print p2[i],
-----------------------------
which in Cython 0.17beta1 gives me
import bug bug.foo() p1: 5 5 p2: 5 5
while in Cython 0.16 I get
import bug bug.foo() p1: 4 4 p2: 5 5
The problem is that the same (temporary) local array variable is used in both cases to build the array, and then only a pointer is assigned, i.e. p1 and p2 then point to the same array, which gets overwritten with the new values in the second assignment.
Looking into this some more, the problem arises from the pointer assignment, because the left side is a pointer variable whereas the right side is a temp value. Temps aren't really made for an enduring life. Here, the array temp variable is being freed after the assignment and then reused. We could fix it by not freeing the temp, or at least by not reusing it, but the problem is really in the syntax. The LHS should be an array, not a pointer. It broke (and I broke it) when I fixed pointer type comparisons and made pointer and array types properly hashable. I think it was already subtly broken before in Py2 and just didn't show because it's sufficiently unlikely that two equal array types that use the default hash by object id() end up in the same dict bucket. Only then would the __eq__() comparison strike to reuse the same temp for both types. Stefan
Stefan Behnel, 25.07.2012 08:29:
Stefan Behnel, 25.07.2012 07:40:
Mike Zaletel, 25.07.2012 00:40:
--------bug.pyx----------
def foo(): cdef int i cdef int* p1 = [4, 4] cdef int* p2 = [5, 5]
print "p1:", for i in range(2): print p1[i], print "\np2:", for i in range(2): print p2[i],
-----------------------------
which in Cython 0.17beta1 gives me
import bug bug.foo() p1: 5 5 p2: 5 5
while in Cython 0.16 I get
import bug bug.foo() p1: 4 4 p2: 5 5
The problem is that the same (temporary) local array variable is used in both cases to build the array, and then only a pointer is assigned, i.e. p1 and p2 then point to the same array, which gets overwritten with the new values in the second assignment.
Looking into this some more, the problem arises from the pointer assignment, because the left side is a pointer variable whereas the right side is a temp value. Temps aren't really made for an enduring life. Here, the array temp variable is being freed after the assignment and then reused. We could fix it by not freeing the temp, or at least by not reusing it
Ah, found the existing hack that aimed to do that and fixed it. :) https://github.com/cython/cython/commit/557b8ed7dfdb9155327e481bef4522a92146... Stefan
On 25 July 2012 07:29, Stefan Behnel <stefan_ml@behnel.de> wrote:
Stefan Behnel, 25.07.2012 07:40:
Mike Zaletel, 25.07.2012 00:40:
--------bug.pyx----------
def foo(): cdef int i cdef int* p1 = [4, 4] cdef int* p2 = [5, 5]
print "p1:", for i in range(2): print p1[i], print "\np2:", for i in range(2): print p2[i],
-----------------------------
which in Cython 0.17beta1 gives me
import bug bug.foo() p1: 5 5 p2: 5 5
while in Cython 0.16 I get
import bug bug.foo() p1: 4 4 p2: 5 5
The problem is that the same (temporary) local array variable is used in both cases to build the array, and then only a pointer is assigned, i.e. p1 and p2 then point to the same array, which gets overwritten with the new values in the second assignment.
Looking into this some more, the problem arises from the pointer assignment, because the left side is a pointer variable whereas the right side is a temp value. Temps aren't really made for an enduring life. Here, the array temp variable is being freed after the assignment and then reused. We could fix it by not freeing the temp, or at least by not reusing it, but the problem is really in the syntax. The LHS should be an array, not a pointer.
It broke (and I broke it) when I fixed pointer type comparisons and made pointer and array types properly hashable. I think it was already subtly broken before in Py2 and just didn't show because it's sufficiently unlikely that two equal array types that use the default hash by object id() end up in the same dict bucket. Only then would the __eq__() comparison strike to reuse the same temp for both types.
Stefan
_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
I'm wondering, what was the original motivation to reuse temporaries? I think it leads to more problems than benefits in most cases, all you really care about is clearing up references. Do we ever have C++ stack-allocated object temporaries? Or does it save some stack space (always, or when you take a pointer to the temporary)?
mark florisson, 25.07.2012 12:18:
I'm wondering, what was the original motivation to reuse temporaries?
Shorter C code. Then, at some point, also smaller closures.
I think it leads to more problems than benefits in most cases, all you really care about is clearing up references.
It's usually fine for C types, though. Except for the very specific case of stack allocated C array literals, for which an explicitly allocated (invisible) local variable would have been a better idea, I think.
Do we ever have C++ stack-allocated object temporaries?
Don't think so. They'd be pointers.
Or does it save some stack space (always, or when you take a pointer to the temporary)?
C compilers do variable aliasing anyway, so I don't think this matters. Stefan
mark florisson wrote:
I'm wondering, what was the original motivation to reuse temporaries?
It goes back to Pyrex, where I didn't really give it much thought -- it just seemed like the tidiest thing to do. Once you have the logic to release temp references as soon as it's safe to do so, it's not much harder to return the variable to a pool as well.
Do we ever have C++ stack-allocated object temporaries?
Theoretically, yes -- temporaries aren't necessarily object references, they can be of any type (or they can in Pyrex, at least). -- Greg
On Wed, Jul 25, 2012 at 4:24 AM, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
mark florisson wrote:
I'm wondering, what was the original motivation to reuse temporaries?
It goes back to Pyrex, where I didn't really give it much thought -- it just seemed like the tidiest thing to do. Once you have the logic to release temp references as soon as it's safe to do so, it's not much harder to return the variable to a pool as well.
Somewhat along the lines of what's been said, it also makes cleaning up any temps (on exceptions or function exits) shorter too, as opposed to what would be required if every temporary was its own variable. This cleanup might interfere with the C compiler's ability to alias the stack variables.
Do we ever have C++ stack-allocated object temporaries?
Theoretically, yes -- temporaries aren't necessarily object references, they can be of any type (or they can in Pyrex, at least).
Same in Cython, including C++ stack-allocated objects (assuming a zero-argument constructor), though in this latter case the scope can't be restricted to less than the entire function body. This actually relates to one of the annoying C++isms I've run into: an untouched stack-allocated C++ object will be a "unused variable" warning if and only if its constructors and destructors are side-effect free. - Robert
participants (5)
-
Greg Ewing -
mark florisson -
Mike Zaletel -
Robert Bradshaw -
Stefan Behnel