[Cython] Some small phase refactorings
Dag Sverre Seljebotn
dagss at student.matnat.uio.no
Sat May 3 10:27:16 CEST 2008
Thanks for the feedback.
> - children_attrs = ["body"]
> + child_attrs = ["body"]
>
>
> ### This looks like a separate fix to me.
Indeed (sorry).
>
> + 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?
Note that this code will almost certainly be moved again and rewritten at
some later point (they can't really belong to ModuleNode); but more
refactoring must happen before they can be moved to their proper location
and they serve a good purpose where they are for now though.
They have to be classes as they are transform objects with member methods,
some state etc. etc. But thinking about it one could have functions like
this in Analysis.py:
def analyse_function_body_declarations(tree, **opts):
return AnalyseFunctionBodyDeclarations()(tree, **opts)
if that helps. The reason I started using the __call__ is that I think in
time one can treat these as functions, like this:
pipeline = [
f,
g,
AnalyseFunctionBodyDeclarations(),
Coercions(),
]
for transform in pipeline:
tree = transform(tree)
(ie, basically saying that "pipeline = f <ring> g <ring> h"...)
> ### 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).
If not, then the classical visitor pattern might put you at ease?:
class FuncDefNode:
def accept(self, visitor):
visitor.visit_FuncDefNode(self)
However, that's a lot of extra trivial code to add (this would have to be
added to _all_ classes), and when one is using Python anyway I'd like to
avoid pretending I'm writing Java... :-)
I hope we can start using Python 2.4, then I'll implement a
decorator/metaclass solution instead.
In conclusion, I'd like to mention that I really think the important thing
here is to consider the "grand, large-scale" features of the patch. I
didn't polish the details in any way, because I think that what is
important here is the changes they make possible in the application
structure. How the visitors look like can be changed entirely in
Visitor.py and Analysis.py without interfering with existing code; while
the phase refactoring is going to intrude everywhere and make changes all
over the place, so the form the phase refactoring will take is the
important point.
(OTOH; I guess it is a good time to think about the details as well so
that when the 1000 line coercion refactoring patch should be written one
knows what to do in the details...)
(I've asked myself as to whether it is all worth it BTW. It is a heavy and
non-fun task. But I'm still convinced there's absolutely no way around it
if the feature-set of Cython is to grow significantly in any way. And
realistically, it can be done in two or three days with for instance me,
you and Robert working together... So this might be too early to talk
about it; but I end up working on it anyway because it is effectively a
blocker for me and I cannot get anywhere with my GSoC stuff until it is
done :-) )
Dag Sverre
More information about the Cython-dev
mailing list