[Cython] Some small phase refactorings

Stefan Behnel stefan_ml at behnel.de
Sat May 3 07:53:30 CEST 2008


Hi,

since no-one replied so far (and since I think public code-review is
important), here I go. Lines starting with ### are mine.

In general, I'm +0 for the change and -0 for the patch. I think, if we use
transformations, there should be a good way to subscribe them to what they
work on. We brought up the idea of a path language, but maybe a subscription
to an AST class and just calling the transform to let it return either the
unchanged input or a modified AST subtree might already be enough.


diff -r 3c924a0594ba Cython/Compiler/ModuleNode.py
--- a/Cython/Compiler/ModuleNode.py	Fri May 02 10:22:20 2008 +0200
+++ b/Cython/Compiler/ModuleNode.py	Fri May 02 11:42:17 2008 +0200
@@ -33,7 +33,7 @@ class ModuleNode(Nodes.Node, Nodes.Block
     #  module_temp_cname    string
     #  full_module_name     string

-    children_attrs = ["body"]
+    child_attrs = ["body"]

     def analyse_declarations(self, env):
         if Options.embed_pos_in_docstring:

### This looks like a separate fix to me.


+        import Cython.Compiler.Transforms.Analysis as Analysis
+        Analysis.CreateFunctionScope()(self, env=env)
+        Analysis.AnalyseControlFlow()(self, env=env)
+        Analysis.AnalyseFunctionBodyDeclarations()(self, env=env)
+        Analysis.AnalyseFunctionBodyExpressions()(self, env=env)
+        options.transforms.run('after_function_analysis', self, global_env=env)
+

### These look like functions, they should follow PEP 8 naming. Then again,
### why aren't they functions?


+    def get_visitfunc(self, prefix, cls):
+        mname = prefix + cls.__name__
+        m = self.visitmethods[prefix].get(mname)
+        if m is None:
+            # Must resolve, try entire hierarchy
+            for cls in inspect.getmro(cls):
+                m = getattr(self, prefix + cls.__name__, None)
+                if m is not None:
+                    break
+            if m is None: raise RuntimeError("Not a Node descendant: " + node)
+            self.visitmethods[prefix][mname] = m
+        return m

### I'm not convinced of this one. I understand why you do it, but I believe
### that using the class itself rather than dispatching on a string would help
### here (I saw your decorator proposal - still thinking about it, but might
### be worth doing).


@@ -110,7 +174,6 @@ class TransformSet(dict):
     def run(self, name, node, **options):
         assert name in self
         for transform in self[name]:
-            transform.initialize(phase=name, **options)
-            transform.process_node(node, "(root)")
+            transform(node, phase=name, **options)

### I like this part.



More information about the Cython-dev mailing list