From b85be6297e958e514c64af3836c6d405ef24b36e Mon Sep 17 00:00:00 2001 From: David Peter Date: Wed, 16 Oct 2024 11:30:03 +0200 Subject: [PATCH] [red knot] Minor follow-up tasks regarding singleton types (#13769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Do not treat empty tuples as singletons after discussion [1] - Improve comment regarding intersection types - Resolve unnecessary TODO in Markdown test [1] https://discuss.python.org/t/should-we-specify-in-the-language-reference-that-the-empty-tuple-is-a-singleton/67957 ## Test Plan — --- .../mdtest/narrow/conditionals_is_not.md | 7 ++-- crates/red_knot_python_semantic/src/types.rs | 34 +++++++++---------- .../src/types/narrow.rs | 2 +- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals_is_not.md b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals_is_not.md index 481e70e6d38dc..b1c75d053c1d3 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals_is_not.md +++ b/crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals_is_not.md @@ -31,10 +31,9 @@ non-singleton class may occupy different addresses in memory even if they compare equal. ```py -x = [1] -y = [1] +x = 345 +y = 345 if x is not y: - # TODO: should include type parameter: list[int] - reveal_type(x) # revealed: list + reveal_type(x) # revealed: Literal[345] ``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 447212d384f9e..bbcc055825a07 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -467,7 +467,7 @@ impl<'db> Type<'db> { /// /// Note: This function aims to have no false positives, but might return `false` /// for more complicated types that are actually singletons. - pub(crate) fn is_singleton(self, db: &'db dyn Db) -> bool { + pub(crate) fn is_singleton(self) -> bool { match self { Type::Any | Type::Never @@ -485,14 +485,13 @@ impl<'db> Type<'db> { false } Type::None | Type::BooleanLiteral(_) | Type::Function(..) | Type::Class(..) | Type::Module(..) => true, - Type::Tuple(tuple) => { - // We deliberately deviate from the language specification [1] here and claim - // that the empty tuple type is a singleton type. The reasoning is that `()` - // is often used as a sentinel value in user code. Declaring the empty tuple to - // be of singleton type allows us to narrow types in `is not ()` conditionals. - // - // [1] https://docs.python.org/3/reference/expressions.html#parenthesized-forms - tuple.elements(db).is_empty() + Type::Tuple(..) => { + // The empty tuple is a singleton on CPython and PyPy, but not on other Python + // implementations such as GraalPy. Its *use* as a singleton is discouraged and + // should not be relied on for type narrowing, so we do not treat it as one. + // See: + // https://docs.python.org/3/reference/expressions.html#parenthesized-forms + false } Type::Union(..) => { // A single-element union, where the sole element was a singleton, would itself @@ -501,13 +500,12 @@ impl<'db> Type<'db> { false } Type::Intersection(..) => { - // Intersection types are hard to analyze. The following types are technically - // all singleton types, but it is not straightforward to compute this. Again, - // we simply return false. + // Here, we assume that all intersection types that are singletons would have + // been reduced to a different form via [`IntersectionBuilder::build`] by now. + // For example: // - // bool & ~Literal[False]` - // None & (None | int) - // (A | B) & (B | C) with A, B, C disjunct and B a singleton + // bool & ~Literal[False] = Literal[True] + // None & (None | int) = None | None & int = None // false } @@ -1682,23 +1680,23 @@ mod tests { #[test_case(Ty::None)] #[test_case(Ty::BoolLiteral(true))] #[test_case(Ty::BoolLiteral(false))] - #[test_case(Ty::Tuple(vec![]))] fn is_singleton(from: Ty) { let db = setup_db(); - assert!(from.into_type(&db).is_singleton(&db)); + assert!(from.into_type(&db).is_singleton()); } #[test_case(Ty::Never)] #[test_case(Ty::IntLiteral(345))] #[test_case(Ty::BuiltinInstance("str"))] #[test_case(Ty::Union(vec![Ty::IntLiteral(1), Ty::IntLiteral(2)]))] + #[test_case(Ty::Tuple(vec![]))] #[test_case(Ty::Tuple(vec![Ty::None]))] #[test_case(Ty::Tuple(vec![Ty::None, Ty::BoolLiteral(true)]))] fn is_not_singleton(from: Ty) { let db = setup_db(); - assert!(!from.into_type(&db).is_singleton(&db)); + assert!(!from.into_type(&db).is_singleton()); } #[test_case(Ty::IntLiteral(1); "is_int_literal_truthy")] diff --git a/crates/red_knot_python_semantic/src/types/narrow.rs b/crates/red_knot_python_semantic/src/types/narrow.rs index 0f84b9c84ac2d..16577e3ddf48e 100644 --- a/crates/red_knot_python_semantic/src/types/narrow.rs +++ b/crates/red_knot_python_semantic/src/types/narrow.rs @@ -157,7 +157,7 @@ impl<'db> NarrowingConstraintsBuilder<'db> { let comp_ty = inference.expression_ty(comparator.scoped_ast_id(self.db, scope)); match op { ast::CmpOp::IsNot => { - if comp_ty.is_singleton(self.db) { + if comp_ty.is_singleton() { let ty = IntersectionBuilder::new(self.db) .add_negative(comp_ty) .build();