board_inspector: refactor tree visitors and transformers

Tree visitors usually have a fixed direction (either top-down or bottom-up)
and invoking a visitor with a wrong direction typically leads to unintended
behavior. However, the current implementation exposes both `visit_topdown`
and `visit_bottomup` methods to users, allowing callers to invoke the
visitors in an undesigned way. The same issue exists in the implementation
of transformers.

This patch refactors the base classes of visitors and transformers so that
they require an explicit direction from their sub-classes to
initialize. Callers of the visitors and transformers are now expected to
invoke the `visit` or `transform` methods without assuming the correct
direction of the visitor or transformer. The original `visit_topdown` or
`visit_bottomup` methods (or their transformer counterparts) are now
used to initialize the `visit` method and can be used by the subclasses in
case those subclasses visits the tree in a customized manner.

Tracked-On: #6287
Signed-off-by: Junjie Mao <junjie.mao@intel.com>

misc/config_tools/board_inspector/acpiparser/aml/parser.py

@@ -8,7 +8,7 @@ import logging
 from . import grammar
 from .context import *
 from .exception import *
-from .tree import Tree, Transformer
+from .tree import Tree, Transformer, Direction
 
 class Factory:
     @staticmethod
@@ -406,7 +406,7 @@ class OptionFactory(Factory):
 
 class DeferredExpansion(Transformer):
     def __init__(self, context):
-        super().__init__()
+        super().__init__(Direction.TOPDOWN)
         self.context = context
 
         nodes = ["DefScope", "DefDevice", "DefMethod", "DefPowerRes", "DefProcessor", "DefThermalZone",

misc/config_tools/board_inspector/acpiparser/aml/tree.py

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 #
 
+from enum import Enum
 from copy import copy
 
 from . import grammar
@@ -44,38 +45,54 @@ class Tree:
                     setattr(self, elem, self.children[i])
                     i += 1
 
-class Visitor:
-    def __init__(self):
-        self.depth = 0
+class Direction(Enum):
+    TOPDOWN = 1
+    BOTTOMUP = 2
+    CUSTOMIZED = 3
 
-    def __visit(self, tree):
+class Visitor:
+    def __init__(self, direction):
+        self.depth = 0
+        if direction == Direction.TOPDOWN:
+            self.visit = self._visit_topdown
+        elif direction == Direction.BOTTOMUP:
+            self.visit = self._visit_bottomup
+
+    def __visit_node(self, tree):
         fn = getattr(self, tree.label, None)
         if not fn:
             fn = getattr(self, "default", None)
         if fn:
             fn(tree)
 
-    def visit_topdown(self, tree):
-        self.__visit(tree)
+    def _visit_topdown(self, tree):
+        self.__visit_node(tree)
         self.depth += 1
         for child in tree.children:
             if isinstance(child, Tree):
-                self.visit_topdown(child)
+                self.visit(child)
         self.depth -= 1
 
-    def visit_bottomup(self, tree):
+    def _visit_bottomup(self, tree):
         self.depth += 1
         for child in tree.children:
             if isinstance(child, Tree):
-                self.visit_bottomup(child)
+                self.visit(child)
         self.depth -= 1
-        self.__visit(tree)
+        self.__visit_node(tree)
+
+    def visit(self, tree):
+        raise NotImplementedError
 
 class Transformer:
-    def __init__(self):
+    def __init__(self, direction):
         self.depth = 0
+        if direction == Direction.TOPDOWN:
+            self.transform = self._transform_topdown
+        elif direction == Direction.BOTTOMUP:
+            self.transform = self._transform_bottomup
 
-    def __transform(self, tree):
+    def __transform_node(self, tree):
         fn = getattr(self, tree.label, None)
         if not fn:
             fn = getattr(self, "default", None)
@@ -84,22 +101,25 @@ class Transformer:
         else:
             return tree
 
-    def transform_topdown(self, tree):
-        new_tree = self.__transform(tree)
+    def _transform_topdown(self, tree):
+        new_tree = self.__transform_node(tree)
         self.depth += 1
         for i, child in enumerate(tree.children):
             if isinstance(child, Tree):
-                tree.children[i] = self.transform_topdown(child)
+                tree.children[i] = self.transform(child)
         self.depth -= 1
         return new_tree
 
-    def transform_bottomup(self, tree):
+    def _transform_bottomup(self, tree):
         self.depth += 1
         for i, child in enumerate(tree.children):
             if isinstance(child, Tree):
-                tree.children[i] = self.transform_bottomup(child)
+                tree.children[i] = self.transform(child)
         self.depth -= 1
-        return self.__transform(tree)
+        return self.__transform_node(tree)
+
+    def transform(self, tree):
+        raise NotImplementedError
 
 class Interpreter:
     def __init__(self, context):

misc/config_tools/board_inspector/acpiparser/aml/visitors.py

@@ -5,7 +5,7 @@
 
 import sys
 
-from .tree import Visitor
+from .tree import Visitor, Direction
 from . import grammar
 
 class PrintLayoutVisitor(Visitor):
@@ -13,6 +13,9 @@ class PrintLayoutVisitor(Visitor):
     def __is_printable(s):
         return all(ord(c) >= 0x20 and ord(c) < 0xFF for c in s)
 
+    def __init__(self):
+        super().__init__(Direction.TOPDOWN)
+
     def default(self, tree):
         indent = "  " * self.depth
         print(f"{indent}{tree.label}", end="")
@@ -30,7 +33,7 @@ class PrintLayoutVisitor(Visitor):
 
 class ConditionallyUnregisterSymbolVisitor(Visitor):
     def __init__(self, interpreter):
-        super().__init__()
+        super().__init__(Direction.CUSTOMIZED)
         self.context = interpreter.context
         self.interpreter = interpreter
         self.conditionally_hidden = False
@@ -48,7 +51,7 @@ class ConditionallyUnregisterSymbolVisitor(Visitor):
                 self.context.unregister_object(realpath)
         return f
 
-    def visit_topdown(self, tree):
+    def visit(self, tree):
         if not self.conditionally_hidden and tree.label == "DefIfElse":
             self.context.change_scope(tree.scope)
             cond = self.interpreter.interpret(tree.children[1]).get()
@@ -57,25 +60,25 @@ class ConditionallyUnregisterSymbolVisitor(Visitor):
             self.depth += 1
             if cond:
                 if hasattr(tree, "TermList"):
-                    self.visit_topdown(tree.TermList)
+                    self.visit(tree.TermList)
                 if hasattr(tree, "DefElse") and tree.DefElse:
                     self.conditionally_hidden = True
-                    self.visit_topdown(tree.DefElse)
+                    self.visit(tree.DefElse)
                     self.conditionally_hidden = False
             else:
                 if hasattr(tree, "TermList"):
                     self.conditionally_hidden = True
-                    self.visit_topdown(tree.TermList)
+                    self.visit(tree.TermList)
                     self.conditionally_hidden = False
                 if hasattr(tree, "DefElse") and tree.DefElse:
-                    self.visit_topdown(tree.DefElse)
+                    self.visit(tree.DefElse)
             self.depth -= 1
         elif tree.label not in ["DefMethod"]:
-            super().visit_topdown(tree)
+            self._visit_topdown(tree)
 
 class GenerateBinaryVisitor(Visitor):
     def __init__(self):
-        super().__init__()
+        super().__init__(Direction.BOTTOMUP)
 
     @staticmethod
     def __format_length(length):

misc/config_tools/board_inspector/acpiparser/dsdt.py

@@ -27,7 +27,7 @@ def DSDT(val):
             context.switch_stream(t)
             tree = Tree()
             AMLCode.parse(context, tree)
-            tree = DeferredExpansion(context).transform_topdown(tree)
+            tree = DeferredExpansion(context).transform(tree)
             context.trees[os.path.basename(t)] = tree
     except Exception as e:
         context.current_stream.dump()
@@ -36,5 +36,5 @@ def DSDT(val):
     context.skip_external_on_lookup()
     visitor = ConditionallyUnregisterSymbolVisitor(ConcreteInterpreter(context))
     for tree in context.trees.values():
-        visitor.visit_topdown(tree)
+        visitor.visit(tree)
     return context