Re: [Cython] adding support for __dict__ in extension types

2011-02-28 Thread Lisandro Dalcin
Bringing up this old post...

On 21 June 2010 15:41, Robert Bradshaw  wrote:
> On Jun 17, 2010, at 9:31 AM, Lisandro Dalcin wrote:
>
>> If we special case a __dict__ attribute in extension types, i.e:
>>
>> cdef class Foo:
>>    cdef dict __dict__
>>
>> and fill type->tp_dictoffset, then we can support __dict__ in
>> extension types.
>>
>> What do you think?
>
> Sounds like a good idea to me. Note that we check tp_dictoffset for
> fast dispatching for cpdef methods (which would be correct as a dict
> lookup *would* be needed if __dict__ is available).
>

I still have this patch lying around in my disk. I remember Stefan had
some objections. For example, when the user ask for __dict__, a new
dict is unconditionally created (in CPython, type dict are allocated
on-demand).  I propose to get this patch pushed now, and optimize
later (however, I really don't know how to safely implement this
optimization).


-- 
Lisandro Dalcin
---
CIMEC (INTEC/CONICET-UNL)
Predio CONICET-Santa Fe
Colectora RN 168 Km 472, Paraje El Pozo
3000 Santa Fe, Argentina
Tel: +54-342-4511594 (ext 1011)
Tel/Fax: +54-342-4511169
diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py
index a66d43b..204d474 100644
--- a/Cython/Compiler/ModuleNode.py
+++ b/Cython/Compiler/ModuleNode.py
@@ -24,6 +24,7 @@ import DebugFlags
 
 from Errors import error, warning
 from PyrexTypes import py_object_type
+from Builtin import dict_type as py_dict_type
 from Cython.Utils import open_new_file, replace_suffix
 from Code import UtilityCode
 from StringEncoding import EncodedString
@@ -1088,10 +1089,18 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
 code.putln("p->%s = %s%s;" % (
 type.vtabslot_cname,
 struct_type_cast, type.vtabptr_cname))
+entry = scope.lookup_here("__dict__")
+if entry and entry in py_attrs:
+code.putln("p->%s = PyDict_New();" % entry.cname)
+code.putln("if (p->%s == 0) { Py_DECREF(o); return 0;}" % entry.cname)
 for entry in py_attrs:
-if scope.is_internal or entry.name == "__weakref__":
+if scope.is_internal:
 # internal classes do not need None inits
 code.putln("p->%s = 0;" % entry.cname)
+elif entry.name == "__dict__":
+pass
+elif entry.name == "__weakref__":
+code.putln("p->%s = 0;" % entry.cname)
 else:
 code.put_init_var_to_py_none(entry, "p->%s", nanny=False)
 entry = scope.lookup_here("__new__")
@@ -2162,9 +2171,14 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
 code.putln(
 'if (__Pyx_SetAttrString(%s, "%s", (PyObject *)&%s) < 0) %s' % (
 Naming.module_cname,
-scope.class_name,
-typeobj_cname,
-code.error_goto(entry.pos)))
+scope.class_name,
+typeobj_cname,
+code.error_goto(entry.pos)))
+dict_entry = scope.lookup_here("__dict__")
+if dict_entry:
+if not (dict_entry.type is py_object_type or
+dict_entry.type is py_dict_type):
+error(dict_entry.pos, "__dict__ slot must be of type 'object' or 'dict'")
 weakref_entry = scope.lookup_here("__weakref__")
 if weakref_entry:
 if weakref_entry.type is py_object_type:
diff --git a/Cython/Compiler/Symtab.py b/Cython/Compiler/Symtab.py
index 0d0e576..afb54c2 100644
--- a/Cython/Compiler/Symtab.py
+++ b/Cython/Compiler/Symtab.py
@@ -37,7 +37,8 @@ def c_safe_identifier(cname):
 # There are some C limitations on struct entry names.
 if ((cname[:2] == '__'
  and not (cname.startswith(Naming.pyrex_prefix)
-  or cname == '__weakref__'))
+  or cname == '__weakref__'
+  or cname == '__dict__'))
 or cname in iso_c99_keywords):
 cname = Naming.pyrex_prefix + cname
 return cname
diff --git a/Cython/Compiler/TypeSlots.py b/Cython/Compiler/TypeSlots.py
index 4565c7e..aaba929 100644
--- a/Cython/Compiler/TypeSlots.py
+++ b/Cython/Compiler/TypeSlots.py
@@ -427,7 +427,24 @@ class BaseClassSlot(SlotDescriptor):
 self.slot_name,
 base_type.typeptr_cname))
 
+class DictOffsetSlot(SlotDescriptor):
+#  Slot descriptor for the dict offset slot.
 
+def __init__(self, name):
+SlotDescriptor.__init__(self, name, dynamic = 1)
+
+def generate_dynamic_init_code(self, scope, code):
+entry = scope.lookup_here('__dict__')
+if entry:
+type =  scope.parent_type
+objstruct = "struct %s" % type.objstruct_cname
+code.putln("%s.%s = offsetof(%s, %s);" % (
+type.typeobj_cname,

Re: [Cython] adding support for __dict__ in extension types

2011-02-28 Thread Stefan Behnel

Lisandro Dalcin, 28.02.2011 17:33:

Bringing up this old post...

On 21 June 2010 15:41, Robert Bradshaw wrote:

On Jun 17, 2010, at 9:31 AM, Lisandro Dalcin wrote:


If we special case a __dict__ attribute in extension types, i.e:

cdef class Foo:
cdef dict __dict__

and fill type->tp_dictoffset, then we can support __dict__ in
extension types.

What do you think?


Sounds like a good idea to me. Note that we check tp_dictoffset for
fast dispatching for cpdef methods (which would be correct as a dict
lookup *would* be needed if __dict__ is available).



I still have this patch lying around in my disk. I remember Stefan had
some objections. For example, when the user ask for __dict__, a new
dict is unconditionally created (in CPython, type dict are allocated
on-demand).  I propose to get this patch pushed now, and optimize
later (however, I really don't know how to safely implement this
optimization).


For reference:

http://thread.gmane.org/gmane.comp.python.cython.devel/10189

Stefan
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] adding support for __dict__ in extension types

2011-02-28 Thread Robert Bradshaw
On Mon, Feb 28, 2011 at 8:33 AM, Lisandro Dalcin  wrote:
> Bringing up this old post...
>
> On 21 June 2010 15:41, Robert Bradshaw  wrote:
>> On Jun 17, 2010, at 9:31 AM, Lisandro Dalcin wrote:
>>
>>> If we special case a __dict__ attribute in extension types, i.e:
>>>
>>> cdef class Foo:
>>>    cdef dict __dict__
>>>
>>> and fill type->tp_dictoffset, then we can support __dict__ in
>>> extension types.
>>>
>>> What do you think?
>>
>> Sounds like a good idea to me. Note that we check tp_dictoffset for
>> fast dispatching for cpdef methods (which would be correct as a dict
>> lookup *would* be needed if __dict__ is available).
>>
>
> I still have this patch lying around in my disk. I remember Stefan had
> some objections. For example, when the user ask for __dict__, a new
> dict is unconditionally created (in CPython, type dict are allocated
> on-demand).  I propose to get this patch pushed now, and optimize
> later (however, I really don't know how to safely implement this
> optimization).

Note there's also the issue of cpdef methods--if the instance has a
__dict__ then a dict lookup must be performed for every method call
(to make sure it's not overridden).

- Robert
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] adding support for __dict__ in extension types

2011-02-28 Thread Lisandro Dalcin
On 28 February 2011 15:09, Robert Bradshaw  wrote:
> On Mon, Feb 28, 2011 at 8:33 AM, Lisandro Dalcin  wrote:
>> Bringing up this old post...
>>
>> On 21 June 2010 15:41, Robert Bradshaw  wrote:
>>> On Jun 17, 2010, at 9:31 AM, Lisandro Dalcin wrote:
>>>
 If we special case a __dict__ attribute in extension types, i.e:

 cdef class Foo:
    cdef dict __dict__

 and fill type->tp_dictoffset, then we can support __dict__ in
 extension types.

 What do you think?
>>>
>>> Sounds like a good idea to me. Note that we check tp_dictoffset for
>>> fast dispatching for cpdef methods (which would be correct as a dict
>>> lookup *would* be needed if __dict__ is available).
>>>
>>
>> I still have this patch lying around in my disk. I remember Stefan had
>> some objections. For example, when the user ask for __dict__, a new
>> dict is unconditionally created (in CPython, type dict are allocated
>> on-demand).  I propose to get this patch pushed now, and optimize
>> later (however, I really don't know how to safely implement this
>> optimization).
>
> Note there's also the issue of cpdef methods--if the instance has a
> __dict__ then a dict lookup must be performed for every method call
> (to make sure it's not overridden).
>

But if the type do have a __dict__, then tp_dictoffset will be filled
(this is in my patch), and cpdef methods should always go the dict
lookup... Am I missing something?

Now that you mention it, my patch should include tests for all this.
I'll work on that.


-- 
Lisandro Dalcin
---
CIMEC (INTEC/CONICET-UNL)
Predio CONICET-Santa Fe
Colectora RN 168 Km 472, Paraje El Pozo
3000 Santa Fe, Argentina
Tel: +54-342-4511594 (ext 1011)
Tel/Fax: +54-342-4511169
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


[Cython] lurker thanks ya'll for all the awesome new stuff you're tinkering on

2011-02-28 Thread Thomas Keller
I don't have the time or knowledge to contribute to Cython right now, but I
just want to thank everybody for the hard work and time they've spent on the
project.

I do usually read the emails that come from this listserv, and it seems like
things are progressing rapidly!

I sincerely hope everyone has a great day.

Cheers,
Thomas
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] Control flow graph

2011-02-28 Thread Vitja Makarov
2011/2/23 Vitja Makarov :
> 2011/2/23 Vitja Makarov :
>> 2011/2/22 Stefan Behnel :
>>> Vitja Makarov, 20.02.2011 18:23:

 2011/2/16 Vitja Makarov:
>
> Hmm... both python and codespeaks in the thread
>>>
>>> Yes, we should keep it to cython-devel only. Sorry for mixing it up.
>>>
>>>
> Here is my commit it's mostly broken now but anyway
>
> https://github.com/vitek/cython/commit/5579b23c3c1c06981331b6427a73e5cb19980b8a
>>>
>>> Flow control support is large enough to merit its own module. Not sure how
>>> 'smart' git is here, but you can always keep the history by explicitly
>>> copying ParseTreeTransforms.py to FlowControl.py and removing the unrelated
>>> sections from both files.
>>>
>>
>> Ok.
>>
>
> Oops there is no copy command in git.
>
>
>>> You are duplicating some code from the type inferencer. We might want to
>>> clean that up at some point. However, given that flow control analysis will
>>> allow us to improve the type inferencer, I think it's best to keep this code
>>> in the FCA part.
>>>
>>
>> Yes, I think it could replace MarkAssignments transform later.
>> Unreachable code could be delete there too.
>>
>>>
 I've update stuff:
  - algo for finding definitions
  - warnings for uninitialized and may be uninitialised use
  - few test cases
>>>
>>> That looks very nice so far. Any idea how well it scales?
>>>
>>
>> "Usually iterative algorithm takes no more then 5 iterations"
>>
>> For ExprNodes.py max number is 15 while avg is about 3
>>
>> About execution time:
>>
>> ExprNodes.py compilation with c/f enabled takes 10.120 ms, w/o 9.325,
>> ~10% slow down.
>> -O flag could be introduced but I don't think that's a good idea.
>>
>> Should later try to execute cython compiled code.
>>
>>>
 Trying to compile ParseTreeTransforms.py I've found this for example:

 warning: Cython/Compiler/ParseTreeTransforms.py:1182:27: Variable
 'template' may be used uninitialized

     def create_Property(self, entry):
         if entry.visibility == 'public':
             if entry.type.is_pyobject:
                 template = self.basic_pyobject_property
             else:
                 template = self.basic_property
         elif entry.visibility == 'readonly':
             template = self.basic_property_ro
         property = template.substitute({
                 u"ATTR": ExprNodes.AttributeNode(pos=entry.pos,

 obj=ExprNodes.NameNode(pos=entry.pos, name="self"),
                                                  attribute=entry.name),
             }, pos=entry.pos).stats[0]
>>>
>>> Ok, I guess that code generally works, but it's better to get rid of the
>>> code smell.
>>>
>>
>> Might be used warning should be disabled by default, because algorithm
>> isn't smart enough:
>>
>> a = 1
>> if (a): b = 1
>> if (a): print b
>>
>> See also:
>> http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wuninitialized-325
>>


I've updated things:

 - Now it passes all the tests (some tests are run with
remove_unreachable=False)
 - Control flow graph support all the control statements (hope so)
 - Analyse PyClassDefNode (now only inner, need some more work)
 - Now we have the following optional warnings:
  1. uninitialized variable use [default on]
  2. possibly uninitialized use [-Wextra]
  3. unreachable code [default on] (this needs rework, unreachable
code detection could be better handled inside CreateControlFlowGraph)
  4. unused variable [-Wextra]
  5. unused argument [-X warn.unused_arg=True]
  6. unused result [-X warn_unused_result=True]

 - Optional dot graph output:

 $ cython foo.pyx -X control_flow.dot_output=foo.dot
 $ dot -Tpng -o foo.png foo.dot

-- 
vitja.
___
cython-devel mailing list
cython-devel@python.org
http://mail.python.org/mailman/listinfo/cython-devel