[Cython] Fwd: Re: [cython-users] checking for "None" in nogil function

[mark florisson]

On 7 May 2012 16:48, Dag Sverre Seljebotn <d.s.seljebotn at astro.uio.no> wrote:
> On 05/07/2012 03:04 PM, Stefan Behnel wrote:
>>
>> Dag Sverre Seljebotn, 07.05.2012 13:48:
>>
>>> Here you go:
>>>
>>> def foo(np.ndarray[double] a, np.ndarray[double] out=None):
>>>     if out is None:
>>>         out = np.empty_like(a)
>>
>>
>> Ah, right - output arguments. Hadn't thought of those.
>>
>> Still, since you pass None explicitly as a default argument, this code
>> wouldn't be impacted by disallowing None for buffers by default. That case
>> is already handled specially in the compiler. But a better default would
>> prevent the *first* argument from being None.
>>
>> So, basically, it would do the right thing straight away in your case and
>> generate safer and more efficient code for it, whereas now you have to
>> test
>> 'a' for being None explicitly and Cython won't understand that hint due to
>> insufficient static analysis. At least, since my last commit you can make
>> Cython do the same thing by declaring it "not None".
>
>
> Yes, thanks!
>
>
>>>>> It's really no different from cdef classes.
>>>>
>>>>
>>>> I find it at least a bit more surprising because a buffer unpacking
>>>> argument is a rather strong hint that you expect something that supports
>>>> this protocol. The fact that you type your function argument with it
>>>> hints
>>>> at the intention to properly unpack it on entry. I'm sure there are lots
>>>> of
>>>> users who were or will be surprised when they realise that that doesn't
>>>> exclude None values.
>>>
>>>
>>> Whereas I think there would be more users surprised by the opposite.
>>
>>
>> We've had enough complaints from users about None being allowed for typed
>> arguments already to consider it at least a gotcha of the language.
>>
>> The main reason we didn't change this behaviour back then was that it
>> would
>> clearly break user code and we thought we could do without that. That's
>> different from considering it "right" and "good".
>>
>>
>>>>>> And I remember that we wanted to change the default settings for
>>>>>> extension
>>>>>> type arguments from "or None" to "not None" years ago but never
>>>>>> actually
>>>>>> did it.
>>>>>
>>>>>
>>>>> I remember that there was such a debate, but I certainly don't remember
>>>>> that this was the conclusion :-)
>>>>
>>>>
>>>> Maybe not, yes.
>>>>
>>>>
>>>>> I didn't agree with that view then and
>>>>> I don't now. I don't remember what Robert's view was...
>>>>>
>>>>> As far as I can remember (which might be biased towards my personal
>>>>> view), the conclusion was that we left the current semantics in place,
>>>>> relying on better control flow analysis to make None-checks cheaper,
>>>>> and
>>>>> when those are cheap enough, make the nonecheck directive default to
>>>>> True
>>>>
>>>>
>>>> At least for buffer arguments, it silently corrupts data or segfaults in
>>>> the current state of affairs, as you pointed out. Not exactly ideal.
>>>
>>>
>>> No different than writing to a field in a cdef class...
>>
>>
>> Hmm, aren't those None checked? At least cdef method calls are AFAIR.
>
>
> Not at all. That's my whole point -- currently, the rule for None in Cython
> is "it's your responsibility to never do a native operation on None".
>
> I don't like that either, but that's just inherited from Pyrex (and many
> projects would get speed regressions etc.).
>
> I'm not against changing that to "we safely None-check", if done nicely --
> it's just that that should be done everywhere at once.
>
> In current master (and as far back as I can remember), this code:
>
> cdef class A:
>    cdef int field
>    cdef int method(self):
>        print self.field
> def f():
>    cdef A a = None
>    a.field = 3
>    a.method()
>
> Turns into:
>
>  __pyx_v_a = ((struct __pyx_obj_5test2_A *)Py_None);
>  __pyx_v_a->field = 3;
>  ((struct __pyx_vtabstruct_5test2_A *)
> __pyx_v_a->__pyx_vtab)->method(__pyx_v_a);
>
>
>
>
>> I think we should really get back to the habit of making code safe first
>> and fast afterwards.
>
>
> Nobody has argued otherwise for some time (since the cdivision thread I
> believe), this is all about Pyrex legacy. Guess part of the story is that
> there's lots of performance-sensitive code in SAGE using cdef classes which
> was written in Pyrex before Cython was around...
>
> In fact, the nonecheck directive was written by yours truly! And I argued
> for making it the default at the time!
>
>
>> Because it uses syntax that is expected to unpack the buffer. If that
>> buffer doesn't exist, I'd expect an error. It's like using interfaces: I
>> want something here that implements the buffer interface. If it doesn't -
>> reject it.
>>
>> Besides, I hope you are aware that your argumentation stands on the (IMHO
>> questionable) fact that "np.ndarray" by itself can be None by default. If
>> np.ndarray should not be be allowed to be None by default, why should
>> np.ndarray[double]? That argument works in both ways.
>
>
> I'm well aware of it...
>
> Dag
>
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> http://mail.python.org/mailman/listinfo/cython-devel

None checking could and should be optimized, it can be done but is a
bit tricky. A problem are class attributes, as you can at certain
point determine that it's not None, but after any function call etc it
can suddenly be None because some code decided to set it to None
(maybe weird, but possible).

We could do well for local variables of which no address is taken,
though. You'd have to recheck after each assignment though. We should
probably start implementing single static assignment first, also to
implement boundschecking and eliminating some common subexpressions.