From e15419396cefab6d0733d70e2f96465b0780d7c3 Mon Sep 17 00:00:00 2001 From: Mike Perlov Date: Tue, 4 Feb 2025 15:40:07 -0500 Subject: [PATCH] [red-knot] Fix Stack overflow in Type::bool (#15843) ## Summary This PR adds `Type::call_bound` method for calls that should follow descriptor protocol calling convention. The PR is intentionally shallow in scope and only fixes #15672 Couple of obvious things that weren't done: * Switch to `call_bound` everywhere it should be used * Address the fact, that red_knot resolves `__bool__ = bool` as a Union, which includes `Type::Dynamic` and hence fails to infer that the truthiness is always false for such a class (I've added a todo comment in mdtests) * Doesn't try to invent a new type for descriptors, although I have a gut feeling it may be more convenient in the end, instead of doing method lookup each time like I did in `call_bound` ## Test Plan * extended mdtests with 2 examples from the issue * cargo neatest run --- .../mdtest/type_properties/truthiness.md | 30 +++++++++ .../resources/mdtest/unary/not.md | 9 --- crates/red_knot_python_semantic/src/types.rs | 63 +++++++++++++++---- 3 files changed, 81 insertions(+), 21 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/type_properties/truthiness.md b/crates/red_knot_python_semantic/resources/mdtest/type_properties/truthiness.md index d2a7f63fd1..54f5858c98 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/type_properties/truthiness.md +++ b/crates/red_knot_python_semantic/resources/mdtest/type_properties/truthiness.md @@ -1,5 +1,7 @@ # Truthiness +## Literals + ```py from typing_extensions import Literal, LiteralString from knot_extensions import AlwaysFalsy, AlwaysTruthy @@ -45,3 +47,31 @@ def _( reveal_type(bool(c)) # revealed: bool reveal_type(bool(d)) # revealed: bool ``` + +## Instances + +Checks that we don't get into a cycle if someone sets their `__bool__` method to the `bool` builtin: + +### __bool__ is bool + +```py +class BoolIsBool: + __bool__ = bool + +reveal_type(bool(BoolIsBool())) # revealed: bool +``` + +### Conditional __bool__ method + +```py +def flag() -> bool: + return True + +class Boom: + if flag(): + __bool__ = bool + else: + __bool__ = int + +reveal_type(bool(Boom())) # revealed: bool +``` diff --git a/crates/red_knot_python_semantic/resources/mdtest/unary/not.md b/crates/red_knot_python_semantic/resources/mdtest/unary/not.md index 37ddfbcc65..e53d29fb10 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/unary/not.md +++ b/crates/red_knot_python_semantic/resources/mdtest/unary/not.md @@ -141,15 +141,6 @@ class AlwaysFalse: # revealed: Literal[True] reveal_type(not AlwaysFalse()) -# We don't get into a cycle if someone sets their `__bool__` method to the `bool` builtin: -class BoolIsBool: - # TODO: The `type[bool]` declaration here is a workaround to avoid running into - # https://github.com/astral-sh/ruff/issues/15672 - __bool__: type[bool] = bool - -# revealed: bool -reveal_type(not BoolIsBool()) - # At runtime, no `__bool__` and no `__len__` means truthy, but we can't rely on that, because # a subclass could add a `__bool__` method. class NoBoolMethod: ... diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 296ea3b64c..7f52e3ae21 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -670,6 +670,10 @@ impl<'db> Type<'db> { matches!(self, Type::ClassLiteral(..)) } + pub const fn is_instance(&self) -> bool { + matches!(self, Type::Instance(..)) + } + pub fn module_literal(db: &'db dyn Db, importing_file: File, submodule: Module) -> Self { Self::ModuleLiteral(ModuleLiteralType::new(db, importing_file, submodule)) } @@ -1843,19 +1847,8 @@ impl<'db> Type<'db> { return Truthiness::Ambiguous; }; - // Check if the class has `__bool__ = bool` and avoid infinite recursion, since - // `Type::call` on `bool` will call `Type::bool` on the argument. - if bool_method - .into_class_literal() - .is_some_and(|ClassLiteralType { class }| { - class.is_known(db, KnownClass::Bool) - }) - { - return Truthiness::Ambiguous; - } - if let Some(Type::BooleanLiteral(bool_val)) = bool_method - .call(db, &CallArguments::positional([*instance_ty])) + .call_bound(db, instance_ty, &CallArguments::positional([])) .return_type(db) { bool_val.into() @@ -2148,6 +2141,52 @@ impl<'db> Type<'db> { } } + /// Return the outcome of calling an class/instance attribute of this type + /// using descriptor protocol. + /// + /// `receiver_ty` must be `Type::Instance(_)` or `Type::ClassLiteral`. + /// + /// TODO: handle `super()` objects properly + #[must_use] + fn call_bound( + self, + db: &'db dyn Db, + receiver_ty: &Type<'db>, + arguments: &CallArguments<'_, 'db>, + ) -> CallOutcome<'db> { + debug_assert!(receiver_ty.is_instance() || receiver_ty.is_class_literal()); + + match self { + Type::FunctionLiteral(..) => { + // Functions are always descriptors, so this would effectively call + // the function with the instance as the first argument + self.call(db, &arguments.with_self(*receiver_ty)) + } + + Type::Instance(_) | Type::ClassLiteral(_) => { + // TODO descriptor protocol. For now, assume non-descriptor and call without `self` argument. + self.call(db, arguments) + } + + Type::Union(union) => CallOutcome::union( + self, + union + .elements(db) + .iter() + .map(|elem| elem.call_bound(db, receiver_ty, arguments)), + ), + + Type::Intersection(_) => CallOutcome::callable(CallBinding::from_return_type( + todo_type!("Type::Intersection.call_bound()"), + )), + + // Cases that duplicate, and thus must be kept in sync with, `Type::call()` + Type::Dynamic(_) => CallOutcome::callable(CallBinding::from_return_type(self)), + + _ => CallOutcome::not_callable(self), + } + } + /// Look up a dunder method on the meta type of `self` and call it. fn call_dunder( self,