Clean up visitor usage.

All visitors are subclasses of ASTVisitor. You invoke them using v.Dispatch() instead of walk(). We no longer use the silly mechanism of walk() adding methods. Also update golden checksums. Four differences.
oilshell · Mar 19, 2018 · 58c729f6ad56c4ffbb8e2ee409ddcb3d0ba5c0b5 · 58c729f
1 parent 5586374
commit 58c729f6ad56c4ffbb8e2ee409ddcb3d0ba5c0b5
diff --git a/opy/_regtest/dis-md5.golden.txt b/opy/_regtest/dis-md5.golden.txt
@@ -100,17 +100,17 @@
  4157 08e3878abd23a8093d252c7044df727b _tmp/regtest/core/braces_test.pyc
  4194 f9d417154a0bf0da679fb957bc3eeadc _tmp/regtest/core/reader.pyc
  4238 09a4df5015792a7770ee7eace0145892 _tmp/regtest/osh/arith_parse_test.pyc
+  4273 200f27a5d3bfb96400806da917025a37 _tmp/regtest/opy/compiler2/misc.pyc
  4276 75a15eecc1e82fb6ac0e52c14e62744d _tmp/regtest/osh/meta.pyc
-  4280 2b8de5aaffcfae89d942e9417932294c _tmp/regtest/opy/compiler2/misc.pyc
  4330 0b4f35f0d70efef473f2e0d80b1adf48 _tmp/regtest/build/app_deps.pyc
  4371 9a488d36ee8a24b3bd483831ad8c2db0 _tmp/regtest/__future__.pyc
  4473 a73808c9bb4cef1c5179d9283c5a5736 _tmp/regtest/tools/deps.pyc
  4532 5e287e71d722f946d6143534badb9798 _tmp/regtest/core/legacy_test.pyc
  4618 866a47cb6e9d0800a8081478081c113f _tmp/regtest/asdl/visitor.pyc
  4632 b08545054821d40c7572a735daddbc4c _tmp/regtest/osh/bool_parse_test.pyc
  4645 1900cb9e8fe71d0a72e5e3e4374f4500 _tmp/regtest/core/id_kind_test.pyc
+  4651 c20ea304f2be7b5564b5cc74447c168c _tmp/regtest/opy/compiler2/visitor.pyc
  4653 ae0eb21dabefb51d94ce1752f3af2e17 _tmp/regtest/core/process_test.pyc
-  4659 902e5d679038e216ce598a203d2806a0 _tmp/regtest/opy/compiler2/visitor.pyc
  4817 858dba6b07633e3501b2aea59416c706 _tmp/regtest/core/id_kind_gen.pyc
  4897 3dac8304d0cbcb247c6f573899b7d5c0 _tmp/regtest/asdl/gen_python.pyc
  5062 b805c171f95636e88a4f4a664c995b82 _tmp/regtest/core/args_test.pyc
@@ -202,7 +202,7 @@
 28978 581863c0d28c4f23d0eaaaab3a525f33 _tmp/regtest/test/sh_spec.pyc
 29159 58e8951b48374c0368253e32c9550075 _tmp/regtest/_abcoll.pyc
 29440 5ae214959a39478444778fa9800f4dd6 _tmp/regtest/collections.pyc
- 30277 ded6882e1a9b49987f487a89baf4a124 _tmp/regtest/opy/compiler2/pyassem.pyc
+ 30243 9a54545f2f70fc1b265676e07f4f258c _tmp/regtest/opy/compiler2/pyassem.pyc
 30811 02d4a38a06b87bc1875e62ffe18bb88a _tmp/regtest/core/builtin.pyc
 32295 29b6c155971a78b46cae1c83500f7efb _tmp/regtest/osh/word_parse.pyc
 32295 3f24a5980d6761b920977cfa7856a65d _tmp/regtest/opy/pytree.pyc
@@ -217,6 +217,6 @@
 56695 dc4402957b8a978b829a9dd196550908 _tmp/regtest/opy/compiler2/transformer.pyc
 57165 2c07e6823dbcee196553599c9d6ea8d4 _tmp/regtest/locale.pyc
 59976 7ea8b29a98a017ee6aba911d78c3d599 _tmp/regtest/optparse.pyc
- 64835 eb41bafad353397b0c5646707e1e0603 _tmp/regtest/opy/compiler2/pycodegen.pyc
+ 64817 1b56dfdd2dd91031be7e312bc8d3d7a7 _tmp/regtest/opy/compiler2/pycodegen.pyc
 65309 f51234057020da7d30eafc509482a21a _tmp/regtest/logging/__init__.pyc
 81561 fc68aabb67bc1ca842d3bd2fff1c746d _tmp/regtest/opy/compiler2/ast.pyc
diff --git a/opy/compiler2/future.py b/opy/compiler2/future.py
@@ -1,6 +1,7 @@
 """Parser for future statements"""

 from . import ast
+from .visitor import ASTVisitor


 def is_future(stmt):
@@ -13,13 +14,14 @@ def is_future(stmt):
        return 0


-class FutureParser(object):
+class FutureParser(ASTVisitor):

    features = ("nested_scopes", "generators", "division",
                "absolute_import", "with_statement", "print_function",
                "unicode_literals")

    def __init__(self):
+        ASTVisitor.__init__(self)
        self.found = {}  # set

    def visitModule(self, node):
@@ -45,7 +47,7 @@ def get_features(self):
        return self.found.keys()


-class BadFutureParser(object):
+class BadFutureParser(ASTVisitor):
    """Check for invalid future statements"""

    def visitFrom(self, node):
diff --git a/opy/compiler2/pyassem.py b/opy/compiler2/pyassem.py
@@ -281,9 +281,9 @@ def __repr__(self):
            return "<block id=%d>" % (self.bid)

    def __str__(self):
-        insts = map(str, self.insts)
-        return "<block %s %d:\n%s>" % (self.label, self.bid,
-                                       '\n'.join(insts))
+        return "<block %s %d:\n%s>" % (
+            self.label, self.bid,
+            '\n'.join(str(inst) for inst in self.insts))

    def emit(self, inst):
        op = inst[0]
@@ -297,9 +297,9 @@ def addOutEdge(self, block):

    def addNext(self, block):
        self.next.append(block)
-        assert len(self.next) == 1, map(str, self.next)
+        assert len(self.next) == 1, [str(b) for b in self.next]
        block.prev.append(self)
-        assert len(block.prev) == 1, map(str, block.prev)
+        assert len(block.prev) == 1, [str(b) for b in block.prev]

    _uncond_transfer = ('RETURN_VALUE', 'RAISE_VARARGS',
                        'JUMP_ABSOLUTE', 'JUMP_FORWARD', 'CONTINUE_LOOP',
@@ -621,7 +621,7 @@ def Bytecode(self):
        return ''.join(self.code)

    def LineNumberTable(self):
-        return ''.join(map(chr, self.lnotab))
+        return ''.join(chr(c) for c in self.lnotab)


 class StackDepthTracker(object):
diff --git a/opy/compiler2/pycodegen.py b/opy/compiler2/pycodegen.py
@@ -2,7 +2,7 @@
 import struct

 from . import ast, syntax
-from .visitor import walk
+from .visitor import ASTVisitor
 from . import pyassem, misc, future, symbols
 from .consts import (
    SC_LOCAL, SC_GLOBAL_IMPLICIT, SC_GLOBAL_EXPLICIT, SC_FREE, SC_CELL)
@@ -63,7 +63,7 @@ def compile(as_tree, filename, mode):

    # NOTE: This currently does nothing!
    v = syntax.SyntaxErrorChecker()
-    walk(as_tree, v)
+    v.Dispatch(as_tree)

    if mode == "single":
        graph = pyassem.PyFlowGraph("<interactive>", as_tree.filename)
@@ -76,8 +76,8 @@ def compile(as_tree, filename, mode):
        # TODO: Does this need to be made more efficient?
        p1 = future.FutureParser()
        p2 = future.BadFutureParser()
-        walk(as_tree, p1)
-        walk(as_tree, p2)
+        p1.Dispatch(as_tree)
+        p2.Dispatch(as_tree)

        gen = TopLevelCodeGenerator(graph, p1.get_features())
    elif mode == "eval":
@@ -88,9 +88,9 @@ def compile(as_tree, filename, mode):
                         "'eval' or 'single'")

    # This has a side effect of populating the gen.graph data structure?
-    # The multiple inheritance implements some weird double-dispatch.
+    # The multiple inheritance implements some weird double-Dispatch.

-    walk(as_tree, gen)
+    gen.Dispatch(as_tree)

    # Not sure why I need this, copied from InteractiveCodeGenerator
    if mode == "single":
@@ -99,9 +99,10 @@ def compile(as_tree, filename, mode):
    return graph.MakeCodeObject()


-class LocalNameFinder(object):
+class LocalNameFinder(ASTVisitor):
    """Find local names in scope"""
    def __init__(self, names=None):
+        ASTVisitor.__init__(self)
        self.names = names or set()
        self.globals = set()

@@ -158,7 +159,7 @@ def top(self):
        return self[-1]


-class CodeGenerator(object):
+class CodeGenerator(ASTVisitor):
    """Defines basic code generator for Python bytecode

    This class is an abstract base class.  Concrete subclasses must
@@ -169,6 +170,7 @@ class CodeGenerator(object):
    class_name = None  # provide default for instance variable

    def __init__(self, graph, futures):
+        ASTVisitor.__init__(self)
        self.graph = graph
        self.futures = futures  # passed down to child CodeGenerator instances

@@ -202,7 +204,7 @@ def mangle(self, name):

    def parseSymbols(self, tree):
        s = symbols.SymbolVisitor()
-        walk(tree, s)
+        s.Dispatch(tree)
        return s.scopes

    # Next five methods handle name access
@@ -285,7 +287,7 @@ def visitModule(self, node):
            self.storeName('__doc__')

        lnf = LocalNameFinder()
-        walk(node.node, lnf)
+        lnf.Dispatch(node.node)

        self.locals.push(lnf.getLocals())
        self.visit(node.node)
@@ -350,7 +352,7 @@ def _funcOrLambda(self, node, gen, ndecorators, isLambda=False):
            gen.setDocstring(node.doc)
        gen.FindLocals()

-        walk(node.code, gen)
+        gen.Dispatch(node.code)
        gen.Finish(isLambda=isLambda)

        self.set_lineno(node)
@@ -367,7 +369,7 @@ def visitClass(self, node):

        gen.Start()
        gen.FindLocals()
-        walk(node.code, gen)
+        gen.Dispatch(node.code)
        gen.Finish()

        self.set_lineno(node)
@@ -663,7 +665,7 @@ def visitGenExpr(self, node):

        gen.Start()
        gen.FindLocals()
-        walk(node.code, gen)
+        gen.Dispatch(node.code)
        gen.Finish(isLambda=isLambda)

        self.set_lineno(node)
@@ -1309,7 +1311,7 @@ def FindLocals(self):
        func = self.func 

        lnf = LocalNameFinder(set(self.func.argnames))
-        walk(func.code, lnf)
+        lnf.Dispatch(func.code)
        self.locals.push(lnf.getLocals())

        if func.varargs:
@@ -1365,7 +1367,7 @@ def Start(self):

    def FindLocals(self):
        lnf = LocalNameFinder()
-        walk(self.klass.code, lnf)
+        lnf.Dispatch(self.klass.code)
        self.locals.push(lnf.getLocals())

        self.graph.setFlag(CO_NEWLOCALS)
@@ -1381,11 +1383,14 @@ def Finish(self):
 def findOp(node):
    """Find the op (DELETE, LOAD, STORE) in an AssTuple tree"""
    v = OpFinder()
-    walk(node, v)
+    v.Dispatch(node)
    return v.op

-class OpFinder(object):
+
+class OpFinder(ASTVisitor):
+
    def __init__(self):
+        ASTVisitor.__init__(self)
        self.op = None

    def visitAssName(self, node):
diff --git a/opy/compiler2/symbols.py b/opy/compiler2/symbols.py
@@ -5,6 +5,7 @@
 from . import misc
 from .consts import SC_LOCAL, SC_GLOBAL_IMPLICIT, SC_GLOBAL_EXPLICIT, \
    SC_FREE, SC_CELL, SC_UNKNOWN
+from .visitor import ASTVisitor

 import sys
 import types
@@ -200,8 +201,9 @@ class ClassScope(Scope):
 gGenExprCounter = 1


-class SymbolVisitor(object):
+class SymbolVisitor(ASTVisitor):
    def __init__(self):
+        ASTVisitor.__init__(self)
        self.scopes = {}  # The "return value" of walk()
        self.klass = None

diff --git a/opy/compiler2/syntax.py b/opy/compiler2/syntax.py
@@ -9,7 +9,11 @@
 errors.
 """

-class SyntaxErrorChecker(object):
+
+from .visitor import ASTVisitor
+
+
+class SyntaxErrorChecker(ASTVisitor):
    """A visitor to find syntax errors in the AST."""

    def __init__(self, multi=None):
@@ -19,6 +23,7 @@ def __init__(self, multi=None):
        for each error rather than raising a SyntaxError for the
        first.
        """
+        ASTVisitor.__init__(self)
        self.multi = multi
        self.errors = 0

diff --git a/opy/compiler2/visitor.py b/opy/compiler2/visitor.py
@@ -31,38 +31,27 @@ class ASTVisitor(object):
    VERBOSE = 0

    def __init__(self):
-        self.node = None
-        self._cache = {}
+        self._method_cache = {}

-    def default(self, node, *args):
+    def _Default(self, node, *args):
+        """If a visitClassName method isn't provided, visit children."""
        for child in node.getChildNodes():
-            self.dispatch(child, *args)
+            self.Dispatch(child, *args)

-    def dispatch(self, node, *args):
-        self.node = node
+    def Dispatch(self, node, *args):
        klass = node.__class__
-        meth = self._cache.get(klass, None)
+
+        # TODO: Shouldn't it be keyed by string rather than class instance?
+        # This would probably change the bytecode order.
+        meth = self._method_cache.get(klass, None)
        if meth is None:
            className = klass.__name__
-            meth = getattr(self.visitor, 'visit' + className, self.default)
-            self._cache[klass] = meth
-##        if self.VERBOSE > 0:
-##            className = klass.__name__
-##            if self.VERBOSE == 1:
-##                if meth == 0:
-##                    print "dispatch", className
-##            else:
-##                print "dispatch", className, (meth and meth.__name__ or '')
+            meth = getattr(self, 'visit' + className, self._Default)
+            self._method_cache[klass] = meth
        return meth(node, *args)

-    def preorder(self, tree, visitor, *args):
-        """Do preorder walk of tree using visitor"""
-        self.visitor = visitor
-
-        # TODO: Remove this!  I don't like mutating the class.
-
-        visitor.visit = self.dispatch
-        self.dispatch(tree, *args) # XXX *args make sense?
+    # Subclasses call self.visit().  TODO: Rename?
+    visit = Dispatch


 class ExampleASTVisitor(ASTVisitor):
@@ -74,15 +63,15 @@ class ExampleASTVisitor(ASTVisitor):
    """
    examples = {}

-    def dispatch(self, node, *args):
+    def Dispatch(self, node, *args):
        self.node = node
-        meth = self._cache.get(node.__class__, None)
+        meth = self._method_cache.get(node.__class__, None)
        className = node.__class__.__name__
        if meth is None:
            meth = getattr(self.visitor, 'visit' + className, 0)
-            self._cache[node.__class__] = meth
+            self._method_cache[node.__class__] = meth
        if self.VERBOSE > 1:
-            print("dispatch", className, (meth and meth.__name__ or ''))
+            print("Dispatch", className, (meth and meth.__name__ or ''))
        if meth:
            meth(node, *args)
        elif self.VERBOSE > 0:
@@ -96,9 +85,4 @@ def dispatch(self, node, *args):
                    if attr[0] != '_':
                        print("\t", "%-12.12s" % attr, getattr(node, attr))
                print()
-            return self.default(node, *args)
-
-
-def walk(tree, visitor):
-    walker = ASTVisitor()
-    walker.preorder(tree, visitor)
+            return self._Default(node, *args)