Skip to content

Fix RecursionError in infer_call_result()#2432

Merged
jacobtylerwalls merged 4 commits intomainfrom
fix-recursion-error-call-result
May 14, 2024
Merged

Fix RecursionError in infer_call_result()#2432
jacobtylerwalls merged 4 commits intomainfrom
fix-recursion-error-call-result

Conversation

@jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented May 12, 2024

Type of Changes

Type
✓ 🐛 Bug fix

Description

Closes pylint-dev/pylint#9139

@jacobtylerwalls jacobtylerwalls added this to the 3.2.1 milestone May 12, 2024
@codecov
Copy link

codecov bot commented May 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.78%. Comparing base (0ccc2e2) to head (5dfa9e2).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2432   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files          94       94           
  Lines       11098    11102    +4     
=======================================
+ Hits        10297    10301    +4     
  Misses        801      801
Flag Coverage Δ
linux 92.59% <100.00%> (+<0.01%) ⬆️
pypy 92.78% <100.00%> (+<0.01%) ⬆️
windows 92.68% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
astroid/bases.py 88.32% <100.00%> (+0.12%) ⬆️

Pierre-Sassoulas
Pierre-Sassoulas reviewed May 13, 2024
View reviewed changes
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see nothing wrong here, but I would lie if I say I understand what was happening here :D would you mind explaining (very slowly) ?

@jacobtylerwalls
Copy link
Member Author

jacobtylerwalls commented May 13, 2024 •
edited
Loading

Of course! I noticed that the recursion error reports in pylint seemed to have a line from infer_call_result() in common in the tracebacks, so I looked to see if there was an addressable issue there. I found that pytorch was doing something elaborate with __call__ that was causing astroid to get stuck in a loop, evaluating the result of a call to __call__.

I edited the test to show this more clearly. Inferring the result of A.__call__() requires... inferring the result of A(), which requires inferring the result of A.__call__(), ...

If you comment out the code change and run the test, you'll see the recursive calls in the pytest output:

astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):
astroid/bases.py:334: in infer_call_result
    for res in node.infer_call_result(caller, context):

The point of the patch is to notice that the two things being checked are the same class and bail out.

@jacobtylerwalls
Copy link
Member Author

jacobtylerwalls commented May 13, 2024

(The self and node are both Instances of A, and the ._proxied on each is the ClassDef for A they have in common.)

Pierre-Sassoulas
Pierre-Sassoulas previously approved these changes May 13, 2024
View reviewed changes
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for explaining, make sense !

@jacobtylerwalls jacobtylerwalls added the pylint-tested PRs that don't cause major regressions with pylint label May 13, 2024
@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented May 14, 2024

(Sorry forgot to approve)

@jacobtylerwalls jacobtylerwalls merged commit d1c37a9 into main May 14, 2024
@jacobtylerwalls jacobtylerwalls deleted the fix-recursion-error-call-result branch May 14, 2024 13:39
github-actions bot pushed a commit that referenced this pull request May 14, 2024
@jacobtylerwalls @github-actions
Fix RecursionError in infer_call_result() (#2432)
1d7d9ac 
(cherry picked from commit d1c37a9)
@github-actions github-actions bot mentioned this pull request May 14, 2024
Merged
jacobtylerwalls added a commit that referenced this pull request May 14, 2024
@github-actions @jacobtylerwalls
Fix RecursionError in infer_call_result() (#2432) (#2436)
a7ff092 
(cherry picked from commit d1c37a9)

Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
@Pierre-Sassoulas Pierre-Sassoulas mentioned this pull request May 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Pierre-Sassoulas Pierre-Sassoulas Pierre-Sassoulas approved these changes

Assignees

No one assigned

Labels

pylint-tested PRs that don't cause major regressions with pylint

Projects

None yet

Milestone

3.2.1

Development

Successfully merging this pull request may close these issues.

RecursionError escapes during inference

2 participants

@jacobtylerwalls @Pierre-Sassoulas