From 61a901a495d3b2c0ff72fe704aac14ca37b7aae1 Mon Sep 17 00:00:00 2001 From: Kavindu Sachinthe Date: Sun, 23 Aug 2026 14:38:26 +0530 Subject: [PATCH] Guard against recursive protocol member checking in __getattr__ self validation Fixes #4632 When accessing an attribute on a class instance where the attribute does not exist statically, Pyrefly falls back to __getattr__ / __getattribute__. When __getattr__ has a protocol-annotated self parameter (e.g. def __getattr__(self: Awaitable, name: str): ...), Pyrefly validates receiver compatibility via structural protocol subtyping (C <: Awaitable). During structural protocol subtyping, Pyrefly checks whether C provides each member of Awaitable (such as __await__). Because C does not statically define __await__, looking up __await__ on C falls back to C.__getattr__ again, triggering another receiver self validation against Awaitable. Because each fallback invocation initiates a new subset query across query boundaries, local cycle detection in Subset alone does not observe the outer in-progress protocol-member check. This leads to unbounded recursion and a stack overflow abort. To address this: - Added protocol_member_guard_stack: RefCell> and coinductive_assumptions_used: Cell to ThreadState in answers_solver.rs. - Key the guard on (got_type, protocol_class_type, attr_name), preserving type arguments (e.g. P[int] vs P[str]). - Scoped the coinductive guard strictly to the dynamic Attribute::GetAttr fallback in attr.rs so that standard attribute lookups incur zero overhead. - When recursion is detected, mark coinductive_assumptions_used = true so speculative results depending on coinductive hypotheses are not persisted in protocol_cache. - Updated test_getattr_self_awaitable_recursion signature and added test_getattr_self_protocol_generic_cache_order. --- pyrefly/lib/alt/answers_solver.rs | 39 +++++++++++++++++++++++++++++++ pyrefly/lib/alt/attr.rs | 24 ++++++++++++------- pyrefly/lib/solver/subset.rs | 18 ++++++++++---- pyrefly/lib/solver/type_order.rs | 8 +++++++ pyrefly/lib/test/attributes.rs | 36 ++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 12 deletions(-) diff --git a/pyrefly/lib/alt/answers_solver.rs b/pyrefly/lib/alt/answers_solver.rs index ef701fa7e3..e14ad60db4 100644 --- a/pyrefly/lib/alt/answers_solver.rs +++ b/pyrefly/lib/alt/answers_solver.rs @@ -32,6 +32,7 @@ use pyrefly_graph::calculation::ProposalResult; use pyrefly_graph::index::Idx; use pyrefly_python::module_name::ModuleName; use pyrefly_python::module_path::ModulePath; +use pyrefly_types::class::ClassType; use pyrefly_types::heap::TypeHeap; use pyrefly_types::quantified::AnchorIndex; use pyrefly_types::quantified::Quantified; @@ -1769,6 +1770,13 @@ pub struct ThreadState { /// check is currently in progress, used as a coinductive guard against /// self-referential protocols. See `filter_overloads_by_self_type`. overload_self_filter_stack: RefCell>, + /// `(got_type, protocol_class_type, attr_name)` tuples whose protocol-member check is + /// currently in progress, used as a coinductive guard against recursive protocol checks + /// (e.g. `__getattr__` annotated with a protocol). + protocol_member_guard_stack: RefCell>, + /// Tracks whether any coinductive assumption was used during solving + /// (e.g. recursive protocol member resolution via dynamic fallback). + coinductive_assumptions_used: Cell, } impl ThreadState { @@ -1782,6 +1790,8 @@ impl ThreadState { trace_sink: RefCell::new(None), trace_sink_stack: RefCell::new(Vec::new()), overload_self_filter_stack: RefCell::new(FxHashSet::default()), + protocol_member_guard_stack: RefCell::new(FxHashSet::default()), + coinductive_assumptions_used: Cell::new(false), } } @@ -2230,6 +2240,35 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { .remove(key); } + /// Mark a protocol member check `(got, protocol_class_type, attr_name)` as in progress. + /// Returns `true` if it was already active, signaling a coinductive cycle. + pub(crate) fn enter_protocol_member_check(&self, key: (Type, ClassType, Name)) -> bool { + let is_cycle = !self + .thread_state + .protocol_member_guard_stack + .borrow_mut() + .insert(key); + if is_cycle { + self.thread_state.coinductive_assumptions_used.set(true); + } + is_cycle + } + + pub(crate) fn exit_protocol_member_check(&self, key: &(Type, ClassType, Name)) { + self.thread_state + .protocol_member_guard_stack + .borrow_mut() + .remove(key); + } + + pub(crate) fn coinductive_assumptions_used(&self) -> bool { + self.thread_state.coinductive_assumptions_used.get() + } + + pub(crate) fn set_coinductive_assumptions_used(&self, value: bool) { + self.thread_state.coinductive_assumptions_used.set(value); + } + /// Given the target idx of a ForwardToFirstUse binding, find the NameAssign's /// def_idx for partial answer lookup. /// diff --git a/pyrefly/lib/alt/attr.rs b/pyrefly/lib/alt/attr.rs index d4e39c2492..d9497a2b9a 100644 --- a/pyrefly/lib/alt/attr.rs +++ b/pyrefly/lib/alt/attr.rs @@ -1430,14 +1430,22 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { attr_name.clone(), )); } - // Filter overloaded got-side methods by self-type so the subset - // solver doesn't match an overload whose `self:` is incompatible - // with the receiver. - let filtered = self.filter_got_overloads_for_protocol(got_attr); - let got_attr = filtered.as_ref().unwrap_or(got_attr); - self.is_attribute_subset(got_attr, &want, &mut |got, want| { - is_subset(got, want) - }) + if matches!(got_attr, Attribute::GetAttr(..)) { + let guard_key = (got.clone(), protocol.clone(), attr_name.clone()); + if self.enter_protocol_member_check(guard_key.clone()) { + continue; + } + let res = self.is_attribute_subset(got_attr, &want, is_subset); + self.exit_protocol_member_check(&guard_key); + res + } else { + // Filter overloaded got-side methods by self-type so the subset + // solver doesn't match an overload whose `self:` is incompatible + // with the receiver. + let filtered = self.filter_got_overloads_for_protocol(got_attr); + let got_attr = filtered.as_ref().unwrap_or(got_attr); + self.is_attribute_subset(got_attr, &want, is_subset) + } .map_err(|err| { SubsetError::IncompatibleAttribute(Box::new(( protocol.name().clone(), diff --git a/pyrefly/lib/solver/subset.rs b/pyrefly/lib/solver/subset.rs index 220ad0fe9a..3a17b719b2 100644 --- a/pyrefly/lib/solver/subset.rs +++ b/pyrefly/lib/solver/subset.rs @@ -677,8 +677,10 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> { // Save coinductive state so we can detect if any coinductive assumptions // were used during this protocol check. - let prev_coinductive = self.coinductive_assumptions_used; + let prev_coinductive = + self.coinductive_assumptions_used || self.type_order.coinductive_assumptions_used(); self.coinductive_assumptions_used = false; + self.type_order.set_coinductive_assumptions_used(false); // For class-level coinductive reasoning: if the `got` type's type arguments // contain Vars, we're likely in a recursive pattern (e.g., checking method return @@ -713,7 +715,8 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> { // 2. No coinductive assumptions were used during this check // (otherwise the result may be contingent on an assumption // that could be invalidated by rollback) - let used_coinductive = self.coinductive_assumptions_used; + let used_coinductive = + self.coinductive_assumptions_used || self.type_order.coinductive_assumptions_used(); if has_no_vars && !used_coinductive { self.solver .store_protocol_cache(&got, &want, &res, self.type_order); @@ -721,6 +724,8 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> { // Restore: propagate any coinductive usage upward self.coinductive_assumptions_used = prev_coinductive || used_coinductive; + self.type_order + .set_coinductive_assumptions_used(prev_coinductive || used_coinductive); res } @@ -1254,15 +1259,20 @@ impl<'solver, 'subset, Ans: LookupAnswer> Subset<'solver, 'subset, Ans> { // Save coinductive state so we can detect if any coinductive assumptions // were used during field comparisons (e.g., a field with a protocol type // that recursively references this TypedDict). - let prev_coinductive = self.coinductive_assumptions_used; + let prev_coinductive = + self.coinductive_assumptions_used || self.type_order.coinductive_assumptions_used(); self.coinductive_assumptions_used = false; + self.type_order.set_coinductive_assumptions_used(false); let res = self.is_subset_typed_dict_inner(got, want); - let used_coinductive = self.coinductive_assumptions_used; + let used_coinductive = + self.coinductive_assumptions_used || self.type_order.coinductive_assumptions_used(); if cacheable && !used_coinductive { self.solver .store_typed_dict_cache(got, want, &res, self.type_order); } self.coinductive_assumptions_used = prev_coinductive || used_coinductive; + self.type_order + .set_coinductive_assumptions_used(prev_coinductive || used_coinductive); res } diff --git a/pyrefly/lib/solver/type_order.rs b/pyrefly/lib/solver/type_order.rs index b35fdd2aa9..194b0c4e6f 100644 --- a/pyrefly/lib/solver/type_order.rs +++ b/pyrefly/lib/solver/type_order.rs @@ -130,6 +130,14 @@ impl<'solver, Ans: LookupAnswer> TypeOrder<'solver, Ans> { .is_protocol_subset_at_attr(got, protocol, name, is_subset) } + pub fn coinductive_assumptions_used(self) -> bool { + self.0.coinductive_assumptions_used() + } + + pub fn set_coinductive_assumptions_used(self, value: bool) { + self.0.set_coinductive_assumptions_used(value) + } + pub fn as_tuple_type(self, cls: &ClassType) -> Option { self.0.as_tuple(cls).map(Type::Tuple) } diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 6c8e105729..f7dffd0450 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -3148,3 +3148,39 @@ del f.real_method.test # E: has no attribute `test` name: str = f.real_method.__name__ "#, ); + +testcase!( + test_getattr_self_awaitable_recursion, + r#" +from collections.abc import Awaitable + +class C: + def __getattr__(self: Awaitable, name: str): + pass + +C().a # E: Argument `C` is not assignable to parameter `self` with type `Awaitable[Unknown]` in function `C.__getattr__` +"#, +); + +testcase!( + test_getattr_self_protocol_generic_cache_order, + r#" +from typing import Protocol, TypeVar + +T = TypeVar("T", covariant=True) + +class P(Protocol[T]): + @property + def x(self) -> T: ... + +class C: + def __getattr__(self: "P[str]", name: str) -> int: + return 0 + +def f_int(x: P[int]) -> None: ... +def f_str(x: P[str]) -> None: ... + +f_int(C()) +f_str(C()) # E: Argument `C` is not assignable to parameter `x` with type `P[str]` in function `f_str` +"#, +);