Partial fix of #299

[kwmsmith, 2009-05-27 16:27:58]

Hi,

I'm trying out codereview.appspot.com for reviewing my gsoc patches.  Please let
me know if this works.

You should find a link for my partial fix of #299.

[dagss, 2009-05-27 17:30:54]

Seems to work great! See comments.

http://codereview.appspot.com/63153/diff/1/2
File Cython/Compiler/Buffer.py (right):

http://codereview.appspot.com/63153/diff/1/2#newcode218
Line 218: def put_unpack_buffer_aux_into_scope(buf_entry, code):
Future improvement: The code generated here could be done statically within
__Pyx_GetBufferAndValidate instead.

In theory I think such a cleanup could harm performance (compiler could perhaps
no longer optimize away e.g. "suboffsets"), though I can't imagine it really
hurting anything measurable.

http://codereview.appspot.com/63153/diff/1/2#newcode224
Line 224: stobuf = [('stride', 'strides'), ('shape', 'shape')]
I suppose changing our Cython struct to match Py_buffer would simplify the code
and make things easier to remember.

http://codereview.appspot.com/63153/diff/1/2#newcode264
Line 264: return "__Pyx_SafeReleaseBuffer(&%s.pyxbuf->bufinfo)" %
entry.buffer_aux.pyx_buf_nd_var.cname
If __Pyx_GetBufferAndValidate is changed according to my other comments, then
the body of __Pyx_SafeReleaseBuffer should be modified instead so that it takes
"&%s" for consistency.

http://codereview.appspot.com/63153/diff/1/2#newcode274
Line 274: return
("__Pyx_GetBufferAndValidate(&%(pyxbufnd_struct)s.pyxbuf->bufinfo, "
See above comments here...

http://codereview.appspot.com/63153/diff/1/2#newcode682
Line 682: 
Not sure if I like the naming scheme with "pyxbuf"; I'm proposing something else
below, but feel free to improve further or ignore this.

http://codereview.appspot.com/63153/diff/1/2#newcode689
Line 689: Py_buffer bufinfo;
Py_buffer pybuffer;

http://codereview.appspot.com/63153/diff/1/2#newcode693
Line 693: __Pyx_Buffer *pyxbuf;
__Pyx_Buffer *buffer;

http://codereview.appspot.com/63153/diff/1/2#newcode696
Line 696: } __Pyx_Buffer_ND;
__Pyx_LocalBuf

or something like that -- something which makes it clear that this is redundant,
"stack-allocated" information, while __Pyx_Buffer is the real buffer.
__Pyx_Buffer_ND seems too close to __Pyx_Buffer when they are really on seperate
levels.

http://codereview.appspot.com/63153/diff/1/4
File Cython/Compiler/Naming.py (right):

http://codereview.appspot.com/63153/diff/1/4#newcode38
Line 38: pyxbufstruct_prefix  = pyrex_prefix + "pyxbuf_"
I think you can loose the extra pyx prefix here, otherwise it will look like

__pyx_pyxbuf_...

which is kind of redundant. (Perhaps try to find a more descriptive name for the
former, like "localbuf" or "buflocal")