From 2563960fafccdb26e1be7a797626f1fd25bd32b9 Mon Sep 17 00:00:00 2001 From: Nick Drozd Date: Fri, 9 Feb 2018 20:46:09 -0600 Subject: [PATCH 1/3] Add type-specific get_children get_children is elegant and flexible and slow. def get_children(self): for field in self._astroid_fields: attr = getattr(self, field) if attr is None: continue if isinstance(attr, (list, tuple)): for elt in attr: yield elt else: yield attr It iterates over a list, dynamically accesses attributes, does null checks, and does type checking. This function gets called a lot, and all that extra work is a real drag on performance. In most cases there isn't any need to do any of these checks. Take an Assign node for instance: def get_children(self): for elt in self.targets: yield elt yield self.value It's known in advance that Assign nodes have a list of targets and a value, so just yield those without checking anything. --- astroid/node_classes.py | 230 ++++++++++++++++++++++++++++++++++++++-- astroid/scoped_nodes.py | 51 +++++++++ 2 files changed, 273 insertions(+), 8 deletions(-) diff --git a/astroid/node_classes.py b/astroid/node_classes.py index 59b2349c59..64dd343804 100644 --- a/astroid/node_classes.py +++ b/astroid/node_classes.py @@ -965,6 +965,9 @@ def pytype(self): :rtype: str """ + def get_children(self): + yield from self.elts + class LookupMixIn(object): """Mixin to look up a name in the right scope.""" @@ -1170,6 +1173,9 @@ def __init__(self, name=None, lineno=None, col_offset=None, parent=None): super(AssignName, self).__init__(lineno, col_offset, parent) + def get_children(self): + yield from () + class DelName(LookupMixIn, mixins.ParentAssignTypeMixin, NodeNG): """Variation of :class:`ast.Delete` represention deletion of a name. @@ -1207,6 +1213,9 @@ def __init__(self, name=None, lineno=None, col_offset=None, parent=None): super(DelName, self).__init__(lineno, col_offset, parent) + def get_children(self): + yield from () + class Name(LookupMixIn, NodeNG): """Class representing an :class:`ast.Name` node. @@ -1247,6 +1256,9 @@ def __init__(self, name=None, lineno=None, col_offset=None, parent=None): super(Name, self).__init__(lineno, col_offset, parent) + def get_children(self): + yield from () + class Arguments(mixins.AssignTypeMixin, NodeNG): """Class representing an :class:`ast.arguments` node. @@ -1489,16 +1501,28 @@ def find_argname(self, argname, rec=False): return None, None def get_children(self): - """Get the child nodes below this node. + yield from self.args or () - This skips over `None` elements in :attr:`kw_defaults`. + yield from self.defaults + yield from self.kwonlyargs - :returns: The children. - :rtype: iterable(NodeNG) - """ - for child in super(Arguments, self).get_children(): - if child is not None: - yield child + if self.varargannotation is not None: + yield self.varargannotation + + if self.kwargannotation is not None: + yield self.kwargannotation + + for elt in self.kw_defaults: + if elt is not None: + yield elt + + for elt in self.annotations: + if elt is not None: + yield elt + + for elt in self.kwonlyargs_annotations: + if elt is not None: + yield elt def _find_arg(argname, args, rec=False): @@ -1587,6 +1611,9 @@ def postinit(self, expr=None): """ self.expr = expr + def get_children(self): + yield self.expr + class Assert(Statement): """Class representing an :class:`ast.Assert` node. @@ -1621,6 +1648,12 @@ def postinit(self, test=None, fail=None): self.fail = fail self.test = test + def get_children(self): + yield self.test + + if self.fail is not None: + yield self.fail + class Assign(mixins.AssignTypeMixin, Statement): """Class representing an :class:`ast.Assign` node. @@ -1656,6 +1689,11 @@ def postinit(self, targets=None, value=None): self.targets = targets self.value = value + def get_children(self): + yield from self.targets + + yield self.value + class AnnAssign(mixins.AssignTypeMixin, Statement): """Class representing an :class:`ast.AnnAssign` node. @@ -1711,6 +1749,13 @@ def postinit(self, target, annotation, simple, value=None): self.value = value self.simple = simple + def get_children(self): + yield self.target + yield self.annotation + + if self.value is not None: + yield self.value + class AugAssign(mixins.AssignTypeMixin, Statement): """Class representing an :class:`ast.AugAssign` node. @@ -1792,6 +1837,10 @@ def type_errors(self, context=None): except exceptions.InferenceError: return [] + def get_children(self): + yield self.target + yield self.value + class Repr(NodeNG): """Class representing an :class:`ast.Repr` node. @@ -1896,6 +1945,10 @@ def type_errors(self, context=None): except exceptions.InferenceError: return [] + def get_children(self): + yield self.left + yield self.right + class BoolOp(NodeNG): """Class representing an :class:`ast.BoolOp` node. @@ -1945,6 +1998,9 @@ def postinit(self, values=None): """ self.values = values + def get_children(self): + yield from self.values + class Break(Statement): """Class representing an :class:`ast.Break` node. @@ -1954,6 +2010,9 @@ class Break(Statement): """ + def get_children(self): + yield from () + class Call(NodeNG): """Class representing an :class:`ast.Call` node. @@ -2015,6 +2074,13 @@ def kwargs(self): keywords = self.keywords or [] return [keyword for keyword in keywords if keyword.arg is None] + def get_children(self): + yield self.func + + yield from self.args + + yield from self.keywords or () + class Compare(NodeNG): """Class representing an :class:`ast.Compare` node. @@ -2183,6 +2249,12 @@ def _get_filtered_stmts(self, lookup_node, node, stmts, mystmt): return stmts, False + def get_children(self): + yield self.target + yield self.iter + + yield from self.ifs + class Const(NodeNG, bases.Instance): """Class representing any constant including num, str, bool, None, bytes. @@ -2295,6 +2367,9 @@ def bool_value(self): """ return bool(self.value) + def get_children(self): + yield from () + class Continue(Statement): """Class representing an :class:`ast.Continue` node. @@ -2304,6 +2379,9 @@ class Continue(Statement): """ + def get_children(self): + yield from () + class Decorators(NodeNG): """A node representing a list of decorators. @@ -2345,6 +2423,9 @@ def scope(self): # skip the function node to go directly to the upper level scope return self.parent.parent.scope() + def get_children(self): + yield from self.nodes + class DelAttr(mixins.ParentAssignTypeMixin, NodeNG): """Variation of :class:`ast.Delete` representing deletion of an attribute. @@ -2394,6 +2475,9 @@ def postinit(self, expr=None): """ self.expr = expr + def get_children(self): + yield self.expr + class Delete(mixins.AssignTypeMixin, Statement): """Class representing an :class:`ast.Delete` node. @@ -2419,6 +2503,9 @@ def postinit(self, targets=None): """ self.targets = targets + def get_children(self): + yield from self.targets + class Dict(NodeNG, bases.Instance): """Class representing an :class:`ast.Dict` node. @@ -2579,6 +2666,9 @@ def postinit(self, value=None): """ self.value = value + def get_children(self): + yield self.value + class Ellipsis(NodeNG): # pylint: disable=redefined-builtin """Class representing an :class:`ast.Ellipsis` node. @@ -2599,12 +2689,18 @@ def bool_value(self): """ return True + def get_children(self): + yield from () + class EmptyNode(NodeNG): """Holds an arbitrary object in the :attr:`LocalsDictNodeNG.locals`.""" object = None + def get_children(self): + yield from () + class ExceptHandler(mixins.AssignTypeMixin, Statement): """Class representing an :class:`ast.ExceptHandler`. node. @@ -2639,6 +2735,15 @@ class ExceptHandler(mixins.AssignTypeMixin, Statement): :type: list(NodeNG) or None """ + def get_children(self): + if self.type is not None: + yield self.type + + if self.name is not None: + yield self.name + + yield from self.body + # pylint: disable=redefined-builtin; had to use the same name as builtin ast module. def postinit(self, type=None, name=None, body=None): """Do some setup after initialisation. @@ -2820,6 +2925,13 @@ def blockstart_tolineno(self): """ return self.iter.tolineno + def get_children(self): + yield self.target + yield self.iter + + yield from self.body + yield from self.orelse + class AsyncFor(For): """Class representing an :class:`ast.AsyncFor` node. @@ -2871,6 +2983,9 @@ def postinit(self, value=None): """ self.value = value + def get_children(self): + yield self.value + class ImportFrom(mixins.ImportFromMixin, Statement): """Class representing an :class:`ast.ImportFrom` node. @@ -2931,6 +3046,9 @@ def __init__(self, fromname, names, level=0, lineno=None, super(ImportFrom, self).__init__(lineno, col_offset, parent) + def get_children(self): + yield from () + class Attribute(NodeNG): """Class representing an :class:`ast.Attribute` node.""" @@ -2973,6 +3091,9 @@ def postinit(self, expr=None): """ self.expr = expr + def get_children(self): + yield self.expr + class Global(Statement): """Class representing an :class:`ast.Global` node. @@ -3009,6 +3130,9 @@ def __init__(self, names, lineno=None, col_offset=None, parent=None): def _infer_name(self, frame, name): return name + def get_children(self): + yield from () + class If(mixins.BlockRangeMixIn, Statement): """Class representing an :class:`ast.If` node. @@ -3075,6 +3199,12 @@ def block_range(self, lineno): return self._elsed_block_range(lineno, self.orelse, self.body[0].fromlineno - 1) + def get_children(self): + yield self.test + + yield from self.body + yield from self.orelse + class IfExp(NodeNG): """Class representing an :class:`ast.IfExp` node. @@ -3116,6 +3246,12 @@ def postinit(self, test=None, body=None, orelse=None): self.body = body self.orelse = orelse + def get_children(self): + yield self.test + yield self.body + yield self.orelse + + class Import(mixins.ImportFromMixin, Statement): """Class representing an :class:`ast.Import` node. @@ -3151,6 +3287,9 @@ def __init__(self, names=None, lineno=None, col_offset=None, parent=None): super(Import, self).__init__(lineno, col_offset, parent) + def get_children(self): + yield from () + class Index(NodeNG): """Class representing an :class:`ast.Index` node. @@ -3178,6 +3317,9 @@ def postinit(self, value=None): """ self.value = value + def get_children(self): + yield self.value + class Keyword(NodeNG): """Class representing an :class:`ast.keyword` node. @@ -3227,6 +3369,9 @@ def postinit(self, value=None): """ self.value = value + def get_children(self): + yield self.value + class List(_BaseContainer): """Class representing an :class:`ast.List` node. @@ -3318,6 +3463,9 @@ def __init__(self, names, lineno=None, col_offset=None, parent=None): def _infer_name(self, frame, name): return name + def get_children(self): + yield from () + class Pass(Statement): """Class representing an :class:`ast.Pass` node. @@ -3327,6 +3475,9 @@ class Pass(Statement): """ + def get_children(self): + yield from () + class Print(Statement): """Class representing an :class:`ast.Print` node. @@ -3428,6 +3579,13 @@ def raises_not_implemented(self): return True return False + def get_children(self): + if self.exc is not None: + yield self.exc + + if self.cause is not None: + yield self.cause + class Return(Statement): """Class representing an :class:`ast.Return` node. @@ -3451,6 +3609,10 @@ def postinit(self, value=None): """ self.value = value + def get_children(self): + if self.value is not None: + yield self.value + class Set(_BaseContainer): """Class representing an :class:`ast.Set` node. @@ -3554,6 +3716,16 @@ def igetattr(self, attrname, context=None): def getattr(self, attrname, context=None): return self._proxied.getattr(attrname, context) + def get_children(self): + if self.lower is not None: + yield self.lower + + if self.step is not None: + yield self.step + + if self.upper is not None: + yield self.upper + class Starred(mixins.ParentAssignTypeMixin, NodeNG): """Class representing an :class:`ast.Starred` node. @@ -3602,6 +3774,9 @@ def postinit(self, value=None): """ self.value = value + def get_children(self): + yield self.value + class Subscript(NodeNG): """Class representing an :class:`ast.Subscript` node. @@ -3660,6 +3835,10 @@ def postinit(self, value=None, slice=None): self.value = value self.slice = slice + def get_children(self): + yield self.value + yield self.slice + class TryExcept(mixins.BlockRangeMixIn, Statement): """Class representing an :class:`ast.TryExcept` node. @@ -3729,6 +3908,12 @@ def block_range(self, lineno): last = exhandler.body[0].fromlineno - 1 return self._elsed_block_range(lineno, self.orelse, last) + def get_children(self): + yield from self.body + + yield from self.handlers or () + yield from self.orelse or () + class TryFinally(mixins.BlockRangeMixIn, Statement): """Class representing an :class:`ast.TryFinally` node. @@ -3785,6 +3970,10 @@ def block_range(self, lineno): return child.block_range(lineno) return self._elsed_block_range(lineno, self.finalbody) + def get_children(self): + yield from self.body + yield from self.finalbody + class Tuple(_BaseContainer): """Class representing an :class:`ast.Tuple` node. @@ -3903,6 +4092,9 @@ def type_errors(self, context=None): except exceptions.InferenceError: return [] + def get_children(self): + yield self.operand + class While(mixins.BlockRangeMixIn, Statement): """Class representing an :class:`ast.While` node. @@ -3967,6 +4159,12 @@ def block_range(self, lineno): """ return self. _elsed_block_range(lineno, self.orelse) + def get_children(self): + yield self.test + + yield from self.body + yield from self.orelse + class With(mixins.BlockRangeMixIn, mixins.AssignTypeMixin, Statement): """Class representing an :class:`ast.With` node. @@ -4051,6 +4249,10 @@ def postinit(self, value=None): """ self.value = value + def get_children(self): + if self.value is not None: + yield self.value + class YieldFrom(Yield): """Class representing an :class:`ast.YieldFrom` node.""" @@ -4059,6 +4261,9 @@ class YieldFrom(Yield): class DictUnpack(NodeNG): """Represents the unpacking of dicts into dicts using :pep:`448`.""" + def get_children(self): + yield from () + class FormattedValue(NodeNG): """Class representing an :class:`ast.FormattedValue` node. @@ -4110,6 +4315,12 @@ def postinit(self, value, conversion=None, format_spec=None): self.conversion = conversion self.format_spec = format_spec + def get_children(self): + yield self.value + + if self.format_spec is not None: + yield self.format_spec + class JoinedStr(NodeNG): """Represents a list of string expressions to be joined. @@ -4134,6 +4345,9 @@ def postinit(self, values=None): """ self.values = values + def get_children(self): + yield from self.values + class Unknown(mixins.AssignTypeMixin, NodeNG): """This node represents a node in a constructed AST where diff --git a/astroid/scoped_nodes.py b/astroid/scoped_nodes.py index 40970671b3..59dad233ff 100644 --- a/astroid/scoped_nodes.py +++ b/astroid/scoped_nodes.py @@ -696,6 +696,10 @@ def bool_value(self): """ return True + def get_children(self): + for elt in self.body: + yield elt + class ComprehensionScope(LocalsDictNodeNG): """Scoping for different types of comprehensions.""" @@ -777,6 +781,11 @@ def bool_value(self): """ return True + def get_children(self): + yield self.elt + + yield from self.generators + class DictComp(ComprehensionScope): """Class representing an :class:`ast.DictComp` node. @@ -851,6 +860,12 @@ def bool_value(self): """ return util.Uninferable + def get_children(self): + yield self.key + yield self.value + + yield from self.generators + class SetComp(ComprehensionScope): """Class representing an :class:`ast.SetComp` node. @@ -916,6 +931,11 @@ def bool_value(self): """ return util.Uninferable + def get_children(self): + yield self.elt + + yield from self.generators + class _ListComp(node_classes.NodeNG): """Class representing an :class:`ast.ListComp` node. @@ -957,6 +977,11 @@ def bool_value(self): """ return util.Uninferable + def get_children(self): + yield self.elt + + yield from self.generators + class ListComp(_ListComp, ComprehensionScope): """Class representing an :class:`ast.ListComp` node. @@ -1174,6 +1199,10 @@ def bool_value(self): """ return True + def get_children(self): + yield self.args + yield self.body + class FunctionDef(node_classes.Statement, Lambda): """Class representing an :class:`ast.FunctionDef`. @@ -1554,6 +1583,18 @@ def bool_value(self): """ return True + def get_children(self): + if self.decorators is not None: + yield self.decorators + + yield self.args + + if self.returns is not None: + yield self.returns + + for elt in self.body: + yield elt + class AsyncFunctionDef(FunctionDef): """Class representing an :class:`ast.FunctionDef` node. @@ -2649,6 +2690,16 @@ def bool_value(self): """ return True + def get_children(self): + for elt in self.body: + yield elt + + for elt in self.bases: + yield elt + + if self.decorators is not None: + yield self.decorators + # Backwards-compatibility aliases Class = util.proxy_alias('Class', ClassDef) From bee7dc58ad675d9e1c69b396765be305115cc06e Mon Sep 17 00:00:00 2001 From: Nick Drozd Date: Sat, 10 Feb 2018 18:46:30 -0600 Subject: [PATCH 2/3] Move nodes_of_class null check out of inner loop The check was being repeated unnecessarily in a tight loop. --- astroid/node_classes.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/astroid/node_classes.py b/astroid/node_classes.py index 64dd343804..836626f699 100644 --- a/astroid/node_classes.py +++ b/astroid/node_classes.py @@ -627,8 +627,16 @@ def nodes_of_class(self, klass, skip_klass=None): """ if isinstance(self, klass): yield self + + if skip_klass is None: + for child_node in self.get_children(): + for matching in child_node.nodes_of_class(klass, skip_klass): + yield matching + + return + for child_node in self.get_children(): - if skip_klass is not None and isinstance(child_node, skip_klass): + if isinstance(child_node, skip_klass): continue for matching in child_node.nodes_of_class(klass, skip_klass): yield matching From a954bc7a77482b9c7a970e330a3d214a27272c94 Mon Sep 17 00:00:00 2001 From: Nick Drozd Date: Fri, 9 Feb 2018 18:34:08 -0600 Subject: [PATCH 3/3] Add type-specific nodes_of_class nodes_of_class is a very flexible method, which is great for use in client code (e.g. Pylint). However, that flexibility requires a great deal of runtime type checking: def nodes_of_class(self, klass, skip_klass=None): if isinstance(self, klass): yield self if skip_klass is None: for child_node in self.get_children(): for matching in child_node.nodes_of_class(klass, skip_klass): yield matching return for child_node in self.get_children(): if isinstance(child_node, skip_klass): continue for matching in child_node.nodes_of_class(klass, skip_klass): yield matching First, the node has to check its own type to see whether it's of the desired class. Then the skip_klass flag has to be checked to see whether anything needs to be skipped. If so, the type of every yielded node has to be check to see if it should be skipped. This is fine for calling code whose arguments can't be known in advance ("Give me all the Assign and ClassDef nodes, but skip all the BinOps, YieldFroms, and Globals."), but in Astroid itself, every call to this function can be known in advance. There's no need to do any type checking if all the nodes know how to respond to certain requests. Take get_assign_nodes for example. The Assign nodes know that they should yield themselves and then yield their Assign children. Other nodes know in advance that they aren't Assign nodes, so they don't need to check their own type, just immediately yield their Assign children. Overly specific functions like get_yield_nodes_skip_lambdas certainly aren't very elegant, but the tradeoff is to take advantage of knowing how the library code works to improve speed. --- astroid/node_classes.py | 61 +++++++++++++++++++++++++++++++++++++++-- astroid/scoped_nodes.py | 9 +++--- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/astroid/node_classes.py b/astroid/node_classes.py index 836626f699..99dbe37161 100644 --- a/astroid/node_classes.py +++ b/astroid/node_classes.py @@ -220,6 +220,7 @@ class NodeNG(object): :type: bool """ + is_lambda = False # Attributes below are set by the builder module or by raw factories lineno = None """The line that this node appears on in the source code. @@ -641,6 +642,30 @@ def nodes_of_class(self, klass, skip_klass=None): for matching in child_node.nodes_of_class(klass, skip_klass): yield matching + def _get_assign_nodes(self): + for child_node in self.get_children(): + for matching in child_node._get_assign_nodes(): + yield matching + + def _get_name_nodes(self): + for child_node in self.get_children(): + for matching in child_node._get_name_nodes(): + yield matching + + def _get_return_nodes_skip_functions(self): + for child_node in self.get_children(): + if child_node.is_function: + continue + for matching in child_node._get_return_nodes_skip_functions(): + yield matching + + def _get_yield_nodes_skip_lambdas(self): + for child_node in self.get_children(): + if child_node.is_lambda: + continue + for matching in child_node._get_yield_nodes_skip_lambdas(): + yield matching + def _infer_name(self, frame, name): # overridden for ImportFrom, Import, Global, TryExcept and Arguments return None @@ -1267,6 +1292,13 @@ def __init__(self, name=None, lineno=None, col_offset=None, parent=None): def get_children(self): yield from () + def _get_name_nodes(self): + yield self + + for child_node in self.get_children(): + for matching in child_node._get_name_nodes(): + yield matching + class Arguments(mixins.AssignTypeMixin, NodeNG): """Class representing an :class:`ast.arguments` node. @@ -1702,6 +1734,13 @@ def get_children(self): yield self.value + def _get_assign_nodes(self): + yield self + + for child_node in self.get_children(): + for matching in child_node._get_assign_nodes(): + yield matching + class AnnAssign(mixins.AssignTypeMixin, Statement): """Class representing an :class:`ast.AnnAssign` node. @@ -2792,7 +2831,7 @@ def catch(self, exceptions): # pylint: disable=redefined-outer-name """ if self.type is None or exceptions is None: return True - for node in self.type.nodes_of_class(Name): + for node in self.type._get_name_nodes(): if node.name in exceptions: return True return False @@ -3582,7 +3621,7 @@ def raises_not_implemented(self): """ if not self.exc: return False - for name in self.exc.nodes_of_class(Name): + for name in self.exc._get_name_nodes(): if name.name == 'NotImplementedError': return True return False @@ -3621,6 +3660,15 @@ def get_children(self): if self.value is not None: yield self.value + def _get_return_nodes_skip_functions(self): + yield self + + for child_node in self.get_children(): + if child_node.is_function: + continue + for matching in child_node._get_return_nodes_skip_functions(): + yield matching + class Set(_BaseContainer): """Class representing an :class:`ast.Set` node. @@ -4261,6 +4309,15 @@ def get_children(self): if self.value is not None: yield self.value + def _get_yield_nodes_skip_lambdas(self): + yield self + + for child_node in self.get_children(): + if child_node.is_function_or_lambda: + continue + for matching in child_node._get_yield_nodes_skip_lambdas(): + yield matching + class YieldFrom(Yield): """Class representing an :class:`ast.YieldFrom` node.""" diff --git a/astroid/scoped_nodes.py b/astroid/scoped_nodes.py index 59dad233ff..8486b8edac 100644 --- a/astroid/scoped_nodes.py +++ b/astroid/scoped_nodes.py @@ -1037,6 +1037,7 @@ class Lambda(mixins.FilterStmtsMixin, LocalsDictNodeNG): _astroid_fields = ('args', 'body',) _other_other_fields = ('locals',) name = '' + is_lambda = True # function's type, 'function' | 'method' | 'staticmethod' | 'classmethod' @property @@ -1313,7 +1314,7 @@ def extra_decorators(self): return [] decorators = [] - for assign in frame.nodes_of_class(node_classes.Assign): + for assign in frame._get_assign_nodes(): if (isinstance(assign.value, node_classes.Call) and isinstance(assign.value.func, node_classes.Name)): for assign_node in assign.targets: @@ -1530,9 +1531,7 @@ def is_generator(self): :returns: True is this is a generator function, False otherwise. :rtype: bool """ - yield_nodes = (node_classes.Yield, node_classes.YieldFrom) - return next(self.nodes_of_class(yield_nodes, - skip_klass=(FunctionDef, Lambda)), False) + return next(self._get_yield_nodes_skip_lambdas(), False) def infer_call_result(self, caller=None, context=None): """Infer what the function returns when called. @@ -1563,7 +1562,7 @@ def infer_call_result(self, caller=None, context=None): c._metaclass = metaclass yield c return - returns = self.nodes_of_class(node_classes.Return, skip_klass=FunctionDef) + returns = self._get_return_nodes_skip_functions() for returnnode in returns: if returnnode.value is None: yield node_classes.Const(None)