From 6bdd92d1006ae73bd5747d7c3714e10de81f1818 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 14 May 2023 19:46:50 -0400 Subject: [PATCH 1/5] Remove cache entries in all exit paths Follow-up to 0740a0dd5e9cb48bb1a400aded498e4db1fcfca9. --- astroid/inference_tip.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/astroid/inference_tip.py b/astroid/inference_tip.py index 44a7fcf15..94c914e65 100644 --- a/astroid/inference_tip.py +++ b/astroid/inference_tip.py @@ -44,6 +44,7 @@ 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 try: yield from _cache[func, node, context] @@ -55,9 +56,15 @@ 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) + 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. yield from result From c807c03c48aea8e458a7db3df7f7b35e1cdffc6b Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 14 May 2023 20:12:02 -0400 Subject: [PATCH 2/5] Add `InferenceContext.is_empty()` --- astroid/context.py | 12 ++++++++++++ astroid/inference_tip.py | 3 +++ 2 files changed, 15 insertions(+) diff --git a/astroid/context.py b/astroid/context.py index a151ca626..cccc81c07 100644 --- a/astroid/context.py +++ b/astroid/context.py @@ -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))}" diff --git a/astroid/inference_tip.py b/astroid/inference_tip.py index 94c914e65..2e472437c 100644 --- a/astroid/inference_tip.py +++ b/astroid/inference_tip.py @@ -46,6 +46,9 @@ def inner( # 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 From 4757af207d96fa5ee50a453d452e35af083d49a4 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 14 May 2023 21:43:14 -0400 Subject: [PATCH 3/5] Add a bound to the inference tips cache Small bounds still yield about equal hits and misses. Further work could determine if storing only the last result is optimal. --- astroid/inference_tip.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/astroid/inference_tip.py b/astroid/inference_tip.py index 2e472437c..b2ac1198a 100644 --- a/astroid/inference_tip.py +++ b/astroid/inference_tip.py @@ -6,6 +6,7 @@ from __future__ import annotations +from collections import OrderedDict from collections.abc import Generator from typing import Any, TypeVar @@ -18,9 +19,9 @@ TransformFn, ) -_cache: dict[ +_cache: OrderedDict[ tuple[InferFn[Any], NodeNG, InferenceContext | None], list[InferenceResult] -] = {} +] = OrderedDict() _CURRENTLY_INFERRING: set[tuple[InferFn[Any], NodeNG]] = set() @@ -61,7 +62,9 @@ def inner( _CURRENTLY_INFERRING.add(partial_cache_key) try: # May raise UseInferenceDefault - result = _cache[func, node, context] = list(func(node, context, **kwargs)) + result = _cache[func, node, context] = list( + func(node, context, **kwargs) + ) finally: # Remove recursion guard. try: @@ -69,6 +72,9 @@ def inner( except KeyError: pass # Recursion may beat us to the punch. + if len(_cache) > 64: + _cache.popitem(last=False) + yield from result return inner From 59221837058252bad1344186040a1e475f045c4a Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 14 May 2023 22:24:49 -0400 Subject: [PATCH 4/5] Add disable --- astroid/inference_tip.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/astroid/inference_tip.py b/astroid/inference_tip.py index b2ac1198a..9eda5b4fc 100644 --- a/astroid/inference_tip.py +++ b/astroid/inference_tip.py @@ -75,7 +75,8 @@ def inner( if len(_cache) > 64: _cache.popitem(last=False) - yield from result + # https://github.com/pylint-dev/pylint/issues/8686 + yield from result # pylint: disable=used-before-assignment return inner From 5a33300db2c4927d5397d41b8e6f78348c6b9316 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 15 May 2023 08:30:49 -0400 Subject: [PATCH 5/5] Skip recursion test on PyPy Reapplied from c1e4c95. --- tests/test_inference.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_inference.py b/tests/test_inference.py index 3de2c17b0..29bf56ac2 100644 --- a/tests/test_inference.py +++ b/tests/test_inference.py @@ -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, @@ -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.