Skip to content

NodeCountMapper doesn't count nodes inside functions #513

@majosm

Description

@majosm

Comparing my concatenation changes with main reminded me that NodeCountMapper doesn't currently accumulate node counts from functions into the overall count. It should be given a map_function_definition method that does something similar to CallSiteCountMapper:

@memoize_method
def map_function_definition(self, /, expr: FunctionDefinition,
*args: Any, **kwargs: Any) -> None:
if not self.visit(expr):
return
new_mapper = self.clone_for_callee(expr)
for subexpr in expr.returns.values():
new_mapper(subexpr, *args, **kwargs)
self.count += new_mapper.count
self.post_visit(expr, *args, **kwargs)

I wonder if the default map_function_definition implementation in CachedWalkMapper should be disabled, to avoid bugs like this? Maybe a default implementation could be provided in a separate method, e.g. _basic_map_function_definition, for convenience in mappers that don't need special behavior.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions