fix(complexity): count what the visitor claims to count#6
Open
GrigoryEvko wants to merge 2 commits into
Open
Conversation
…cFor total_nodes was computed as the sum of 7 specific visitor counts, severely under-reporting (`def f(): x=1+2; return x` returned 2 vs the real 13). Use len(ast.walk(tree)) instead — the value was already computed above for entropy. Add visit_Match (each case branch is a discrete condition, mirroring if/elif) and visit_AsyncFor (counted as a loop, mirroring for/while). Without these, modern Python code reported condition_count=0 / loop_count=0. One pre-existing test asserted the buggy sum-of-counts as expected output and is updated.
Comprehensions (list/set/dict/generator) are loops in disguise: each `for` generator contributes a loop and each `if` clause contributes a condition. `IfExp` (ternary `a if b else c`) is a branch decision. `Lambda` is an anonymous function definition. Each `except` handler in `try` / `except*` is a branch path. Verified against an independent ast.walk-based reference on 9 small samples and 7 production files from torch + transformers (4k-10k LOC each); all five counts match across both implementations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NumericalComplexityVisitorhad three correctness gaps: (a)total_nodeswas a sum of 7 specific visitor counts and under-reported by ~6× — uselen(ast.walk(tree))instead; (b)visit_Matchandvisit_AsyncForwere missing, so programs usingmatchorasync forreportedcondition_count=0/loop_count=0; (c) comprehensions,IfExp,Lambda,try/except, andexcept*were not counted at all.Reproducer against current main:
Fix: use
len(ast.walk(tree))fortotal_nodes; addvisit_Match(each case is a condition),visit_AsyncFor(loop), comprehension visitors (each generator = loop, eachifclause = condition),visit_IfExp/visit_Lambda/visit_Try/visit_TryStar. Verified against an independentast.walk-based reference on 9 small samples and 7 large production files from torch + transformers (4k–10k LOC); all five counts match across both implementations. One pre-existing test asserted the buggy sum-of-counts as expected output and is updated.