From e91ace73418a6cdb2219922dc0d43a805aae1261 Mon Sep 17 00:00:00 2001 From: Junjie Mao Date: Wed, 21 Jul 2021 09:46:54 +0800 Subject: [PATCH] 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 --- .../board_inspector/acpiparser/aml/parser.py | 4 +- .../board_inspector/acpiparser/aml/tree.py | 56 +++++++++++++------ .../acpiparser/aml/visitors.py | 21 ++++--- .../board_inspector/acpiparser/dsdt.py | 4 +- 4 files changed, 54 insertions(+), 31 deletions(-) diff --git a/misc/config_tools/board_inspector/acpiparser/aml/parser.py b/misc/config_tools/board_inspector/acpiparser/aml/parser.py index 5cce3f1b4..6fcbf153b 100644 --- a/misc/config_tools/board_inspector/acpiparser/aml/parser.py +++ b/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", diff --git a/misc/config_tools/board_inspector/acpiparser/aml/tree.py b/misc/config_tools/board_inspector/acpiparser/aml/tree.py index 80df5c614..8aa049f2c 100644 --- a/misc/config_tools/board_inspector/acpiparser/aml/tree.py +++ b/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): diff --git a/misc/config_tools/board_inspector/acpiparser/aml/visitors.py b/misc/config_tools/board_inspector/acpiparser/aml/visitors.py index d455169ba..1e9808e25 100644 --- a/misc/config_tools/board_inspector/acpiparser/aml/visitors.py +++ b/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): diff --git a/misc/config_tools/board_inspector/acpiparser/dsdt.py b/misc/config_tools/board_inspector/acpiparser/dsdt.py index fbc282bbd..bc7e45646 100644 --- a/misc/config_tools/board_inspector/acpiparser/dsdt.py +++ b/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