Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a bound to the inference tips cache (and fix regressions from #2158) #2181

Merged
merged 5 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions astroid/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,18 @@ def restore_path(self) -> Iterator[None]:
yield
self.path = path

def is_empty(self) -> bool:
return (
not self.path
and not self.nodes_inferred
and not self.callcontext
and not self.boundnode
and not self.lookupname
and not self.callcontext
and not self.extra_context
and not self.constraints
)

def __str__(self) -> str:
state = (
f"{field}={pprint.pformat(getattr(self, field), width=80 - len(field))}"
Expand Down
31 changes: 24 additions & 7 deletions astroid/inference_tip.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from __future__ import annotations

from collections import OrderedDict
from collections.abc import Generator
from typing import Any, TypeVar

Expand All @@ -18,9 +19,9 @@
TransformFn,
)

_cache: dict[
_cache: OrderedDict[
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are on 3.7+ right? Dicts are ordered by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

>>> d = {}
>>> d.popitem(last=False)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: dict.popitem() takes no keyword arguments

Copy link
Member Author

Choose a reason for hiding this comment

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

The one thing OrderedDict is still good at :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

TIL πŸ˜„

tuple[InferFn[Any], NodeNG, InferenceContext | None], list[InferenceResult]
] = {}
] = OrderedDict()

_CURRENTLY_INFERRING: set[tuple[InferFn[Any], NodeNG]] = set()

Expand All @@ -44,7 +45,11 @@ def inner(
if partial_cache_key in _CURRENTLY_INFERRING:
# If through recursion we end up trying to infer the same
# func + node we raise here.
_CURRENTLY_INFERRING.remove(partial_cache_key)
raise UseInferenceDefault
if context is not None and context.is_empty():
# Fresh, empty contexts will defeat the cache.
context = None
try:
yield from _cache[func, node, context]
return
Expand All @@ -55,11 +60,23 @@ def inner(
# with slightly different contexts while still passing the simple
# test cases included with this commit.
_CURRENTLY_INFERRING.add(partial_cache_key)
result = _cache[func, node, context] = list(func(node, context, **kwargs))
# Remove recursion guard.
_CURRENTLY_INFERRING.remove(partial_cache_key)

yield from result
try:
# May raise UseInferenceDefault
result = _cache[func, node, context] = list(
func(node, context, **kwargs)
)
finally:
# Remove recursion guard.
try:
_CURRENTLY_INFERRING.remove(partial_cache_key)
except KeyError:
pass # Recursion may beat us to the punch.

if len(_cache) > 64:
_cache.popitem(last=False)

# https://github.com/pylint-dev/pylint/issues/8686
yield from result # pylint: disable=used-before-assignment

return inner

Expand Down
5 changes: 4 additions & 1 deletion tests/test_inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from astroid.arguments import CallSite
from astroid.bases import BoundMethod, Instance, UnboundMethod, UnionType
from astroid.builder import AstroidBuilder, _extract_single_node, extract_node, parse
from astroid.const import PY39_PLUS, PY310_PLUS
from astroid.const import IS_PYPY, PY39_PLUS, PY310_PLUS
from astroid.context import CallContext, InferenceContext
from astroid.exceptions import (
AstroidTypeError,
Expand Down Expand Up @@ -6976,6 +6976,9 @@ def test_imported_module_var_inferable3() -> None:
assert i_w_val.as_string() == "['w', 'v']"


@pytest.mark.skipif(
IS_PYPY, reason="Test run with coverage on PyPy sometimes raises a RecursionError"
)
def test_recursion_on_inference_tip() -> None:
"""Regression test for recursion in inference tip.

Expand Down
Loading