[Cython] cython-devel-tests-pyregr regression

[Vitja Makarov]

2012/8/23 Vitja Makarov <vitja.makarov at gmail.com>:
> 2012/8/23 Vitja Makarov <vitja.makarov at gmail.com>:
>> 2012/8/23 Stefan Behnel <stefan_ml at behnel.de>:
>>> Vitja Makarov, 23.08.2012 07:03:
>>>> 2012/8/23 Stefan Behnel <stefan_ml at behnel.de>:
>>>>> Vitja Makarov, 22.08.2012 22:34:
>>>>>> 2012/8/23 Stefan Behnel:
>>>>>>> Vitja Makarov, 22.08.2012 22:11:
>>>>>>>> I've found regression:
>>>>>>>>
>>>>>>>> https://sage.math.washington.edu:8091/hudson/job/cython-devel-tests-pyregr/
>>>>>>>
>>>>>>> Interesting. It's a Py2 list comprehension in a class body that's failing here:
>>>>>>>
>>>>>>> """
>>>>>>>  class TestHelpSubparsersOrdering(HelpTestCase):
>>>>>>>      subparsers_signatures = [Sig(name=name)
>>>>>>>                               for name in ('a', 'b', 'c', 'd', 'e')]
>>>>>>>  """
>>>>>>>
>>>>>>> I wonder why "name" isn't declared as a variable yet at the point where it
>>>>>>> is being looked up in the function call.
>>>>>>
>>>>>>     def lookup_relative(self, name, pos):
>>>>>>         if name == "name":
>>>>>>             print name
>>>>>>             from ipdb import set_trace; set_trace()
>>>>>>         entry = self.lookup_here(name)
>>>>>>         if entry is not None and entry.pos[1:] <= pos[1:]: # Lookup fails here
>>>>>>             return entry
>>>>>>         if self.outer_scope:
>>>>>>             return self.outer_scope.lookup_relative(name, pos)
>>>>>>         return None
>>>>>>
>>>>>>
>>>>>> What is that comparison for?
>>>>>
>>>>> Ah, yes, it is wrong in this context. It was meant to prevent names defined
>>>>> further down in the class body from being considered assignments to the
>>>>> name being looked up. Class bodies are not function bodies, assignments in
>>>>> them do not make a name "local". As long as it's not assigned, it's not
>>>>> defined and must be looked up in the outer scope.
>>>>
>>>> Do you remember this ticket #671
>>>>
>>>> If there is assignment in the class body we first lookup in the class
>>>> dict and then in globals.
>>>>
>>>> B = 0
>>>> def foo():
>>>>     B = 1
>>>>     class Foo():
>>>>         A = B
>>>>         B = B
>>>>     class Bar():
>>>>         A = B
>>>>     print Foo.A, Foo.B, Bar.A
>>>> foo()
>>>>
>>>> prints "0 0 1"
>>>
>>> In the case at hand, it's not an assignment but a method declaration. Maybe
>>> that makes a difference.
>>>
>>> In any case, this needs some more investigation than I did for my change. I
>>> think it can be rolled back completely.
>>>
>>>
>>>>> I think comprehensions are actually the only case where a name is used in
>>>>> the source before its declaration. It should work in all other cases.
>>>>>
>>>>> I had considered solving this problem with the flow control analysis
>>>>> information, but I can't see how that helps me to figure out if an entry is
>>>>> already assigned (i.e. declared) at a given point in the class body.
>>>>>
>>>>> Any idea?
>>>>
>>>> What would you do with maybe assigned case?
>>>
>>> Hmm, yes - I guess we can't solve the general case at compile time. That's
>>> unfortunate, though, because it prevents proper compile time optimisation
>>> of builtins in class bodies when their names are assigned at some point,
>>> e.g. with a ".type()" method or ".set()", as was the case here.
>>>
>>> Stefan
>>>
>>
>>
>>
>> This is NameNode related problem
>>
>> The following code would fail as well:
>> XXX = 123
>> class Foo(object):
>>     t1 = XXX  # XXX   cf_is_null = True
>>     t2 = XXX  # XXX   cf_is_null = False
>>     XXX = 123
>>
>> CF assumes that after first apperence of NULL name it must be then set
>> otherwise exception must be raised.
>> NameNode code relays on CF here, so I think it mustn't. We must
>> allways check result of PyObject_GetItem() and if's NULL look at the
>> globals then.
>>
>> I'll try to fix it soon.
>>
>>
>
> I've reverted your commit and here is my fix:
> https://github.com/vitek/cython/commit/198f254f62360b61c895ba68be0f4dbe07444449
>
> Cause of different class scope lookup rules I think we can't fully
> trust CF here.
>

Another (maybe better) solution is to fix CF. It mustn't set
cf_maybe_null to False at class scope based on reference success.


-- 
vitja.