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

[red-knot] T | object == object #16088

Merged
merged 2 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,31 @@ def noreturn(u1: int | NoReturn, u2: int | NoReturn | str) -> None:
reveal_type(u2) # revealed: int | str
```

## `object` subsumes everything

Unions with `object` can be simplified to `object`:

```py
from typing_extensions import Never, Any

def _(
u1: int | object,
u2: object | int,
u3: Any | object,
u4: object | Any,
u5: object | Never,
u6: Never | object,
u7: int | str | object | bytes | Any,
) -> None:
reveal_type(u1) # revealed: object
reveal_type(u2) # revealed: object
reveal_type(u3) # revealed: object
reveal_type(u4) # revealed: object
reveal_type(u5) # revealed: object
reveal_type(u6) # revealed: object
reveal_type(u7) # revealed: object
```

## Flattening of nested unions

```py
Expand Down Expand Up @@ -121,7 +146,7 @@ Simplifications still apply when `Unknown` is present.
from knot_extensions import Unknown

def _(u1: str | Unknown | int | object):
reveal_type(u1) # revealed: Unknown | object
reveal_type(u1) # revealed: object
```

## Union of intersections
Expand Down
26 changes: 18 additions & 8 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,10 @@ impl<'db> Type<'db> {
Self::Dynamic(DynamicType::Unknown)
}

pub fn object(db: &'db dyn Db) -> Self {
KnownClass::Object.to_instance(db)
}

pub const fn is_unknown(&self) -> bool {
matches!(self, Type::Dynamic(DynamicType::Unknown))
}
Expand All @@ -639,6 +643,11 @@ impl<'db> Type<'db> {
matches!(self, Type::Never)
}

pub fn is_object(&self, db: &'db dyn Db) -> bool {
self.into_instance()
.is_some_and(|instance| instance.class.is_object(db))
}

pub const fn is_todo(&self) -> bool {
matches!(self, Type::Dynamic(DynamicType::Todo(_)))
}
Expand Down Expand Up @@ -895,7 +904,7 @@ impl<'db> Type<'db> {
// `object` is the only type that can be known to be a supertype of any intersection,
// even an intersection with no positive elements
(Type::Intersection(_), Type::Instance(InstanceType { class }))
if class.is_known(db, KnownClass::Object) =>
if class.is_object(db) =>
{
true
}
Expand Down Expand Up @@ -949,7 +958,7 @@ impl<'db> Type<'db> {
(left, Type::AlwaysTruthy) => left.bool(db).is_always_true(),
// Currently, the only supertype of `AlwaysFalsy` and `AlwaysTruthy` is the universal set (object instance).
(Type::AlwaysFalsy | Type::AlwaysTruthy, _) => {
target.is_equivalent_to(db, KnownClass::Object.to_instance(db))
target.is_equivalent_to(db, Type::object(db))
}

// All `StringLiteral` types are a subtype of `LiteralString`.
Expand Down Expand Up @@ -1088,11 +1097,7 @@ impl<'db> Type<'db> {

// All types are assignable to `object`.
// TODO this special case might be removable once the below cases are comprehensive
(_, Type::Instance(InstanceType { class }))
if class.is_known(db, KnownClass::Object) =>
{
true
}
(_, Type::Instance(InstanceType { class })) if class.is_object(db) => true,

// A union is assignable to a type T iff every element of the union is assignable to T.
(Type::Union(union), ty) => union
Expand Down Expand Up @@ -1801,7 +1806,7 @@ impl<'db> Type<'db> {
// TODO should be `Callable[[], Literal[True/False]]`
todo_type!("`__bool__` for `AlwaysTruthy`/`AlwaysFalsy` Type variants").into()
}
_ => KnownClass::Object.to_instance(db).member(db, name),
_ => Type::object(db).member(db, name),
},
}
}
Expand Down Expand Up @@ -3853,6 +3858,11 @@ impl<'db> Class<'db> {
self.known(db) == Some(known_class)
}

/// Return `true` if this class represents the builtin class `object`
pub fn is_object(self, db: &'db dyn Db) -> bool {
self.is_known(db, KnownClass::Object)
}

/// Return an iterator over the inferred types of this class's *explicit* bases.
///
/// Note that any class (except for `object`) that has no explicit
Expand Down
29 changes: 21 additions & 8 deletions crates/red_knot_python_semantic/src/types/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ impl<'db> UnionBuilder<'db> {
}
}

/// Collapse the union to a single type: `object`.
fn collapse_to_object(mut self) -> Self {
self.elements.clear();
self.elements.push(Type::object(self.db));
self
}
Comment on lines +46 to +51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also thought about extending the UnionBuilder to keep track of the fact that it represented object. It could then short-circuit any .add(…) into a no-op. Not sure if it's worth the extra complexity though, as we probably don't union with object a lot in reality.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the sort of thing IMO best left to testing with actual benchmarks once we ideally have a broader ecosystem check that we can also benchmark.


/// Adds a type to this union.
pub(crate) fn add(mut self, ty: Type<'db>) -> Self {
match ty {
Expand All @@ -53,7 +60,12 @@ impl<'db> UnionBuilder<'db> {
self = self.add(*element);
}
}
// Adding `Never` to a union is a no-op.
Type::Never => {}
// Adding `object` to a union results in `object`.
ty if ty.is_object(self.db) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could also be

Suggested change
ty if ty.is_object(self.db) => {
Type::Instance(instance) if instance.class.is_object(self.db) => {

but I somehow doubt that it would be much faster(?)

Copy link
Contributor

Choose a reason for hiding this comment

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

I wondered the same thing; I think it could be unpacked even further to match against class.known of the InstanceType and eliminate the if entirely? But in the absence of evidence that that is faster, it just looks more verbose.

return self.collapse_to_object();
}
_ => {
let bool_pair = if let Type::BooleanLiteral(b) = ty {
Some(Type::BooleanLiteral(!b))
Expand All @@ -76,7 +88,10 @@ impl<'db> UnionBuilder<'db> {
break;
}

if ty.is_same_gradual_form(*element) || ty.is_subtype_of(self.db, *element) {
if ty.is_same_gradual_form(*element)
|| ty.is_subtype_of(self.db, *element)
|| element.is_object(self.db)
{
return self;
} else if element.is_subtype_of(self.db, ty) {
to_remove.push(index);
Expand All @@ -88,9 +103,7 @@ impl<'db> UnionBuilder<'db> {
// `element | ty` must be `object` (object has no other supertypes). This means we can simplify
// the whole union to just `object`, since all other potential elements would also be subtypes of
// `object`.
self.elements.clear();
self.elements.push(KnownClass::Object.to_instance(self.db));
return self;
return self.collapse_to_object();
}
}
match to_remove[..] {
Expand Down Expand Up @@ -416,7 +429,7 @@ impl<'db> InnerIntersectionBuilder<'db> {
Type::Never => {
// Adding ~Never to an intersection is a no-op.
}
Type::Instance(instance) if instance.class.is_known(db, KnownClass::Object) => {
Type::Instance(instance) if instance.class.is_object(db) => {
// Adding ~object to an intersection results in Never.
*self = Self::default();
self.positive.insert(Type::Never);
Expand Down Expand Up @@ -481,7 +494,7 @@ impl<'db> InnerIntersectionBuilder<'db> {

fn build(mut self, db: &'db dyn Db) -> Type<'db> {
match (self.positive.len(), self.negative.len()) {
(0, 0) => KnownClass::Object.to_instance(db),
(0, 0) => Type::object(db),
(1, 0) => self.positive[0],
_ => {
self.positive.shrink_to_fit();
Expand Down Expand Up @@ -534,7 +547,7 @@ mod tests {
let db = setup_db();

let intersection = IntersectionBuilder::new(&db).build();
assert_eq!(intersection, KnownClass::Object.to_instance(&db));
assert_eq!(intersection, Type::object(&db));
}

#[test_case(Type::BooleanLiteral(true))]
Expand All @@ -548,7 +561,7 @@ mod tests {
// We add t_object in various orders (in first or second position) in
// the tests below to ensure that the boolean simplification eliminates
// everything from the intersection, not just `bool`.
let t_object = KnownClass::Object.to_instance(&db);
let t_object = Type::object(&db);
let t_bool = KnownClass::Bool.to_instance(&db);

let ty = IntersectionBuilder::new(&db)
Expand Down
6 changes: 2 additions & 4 deletions crates/red_knot_python_semantic/src/types/mro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::ops::Deref;
use rustc_hash::FxHashSet;

use crate::types::class_base::ClassBase;
use crate::types::{Class, KnownClass, Type};
use crate::types::{Class, Type};
use crate::Db;

/// The inferred method resolution order of a given class.
Expand Down Expand Up @@ -52,9 +52,7 @@ impl<'db> Mro<'db> {
match class_bases {
// `builtins.object` is the special case:
// the only class in Python that has an MRO with length <2
[] if class.is_known(db, KnownClass::Object) => {
Ok(Self::from([ClassBase::Class(class)]))
}
[] if class.is_object(db) => Ok(Self::from([ClassBase::Class(class)])),

// All other classes in Python have an MRO with length >=2.
// Even if a class has no explicit base classes,
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/src/types/narrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,13 @@ fn merge_constraints_or<'db>(
*entry.get_mut() = UnionBuilder::new(db).add(*entry.get()).add(*value).build();
}
Entry::Vacant(entry) => {
entry.insert(KnownClass::Object.to_instance(db));
entry.insert(Type::object(db));
}
}
}
for (key, value) in into.iter_mut() {
if !from.contains_key(key) {
*value = KnownClass::Object.to_instance(db);
*value = Type::object(db);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/src/types/property_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ fn union<'db>(db: &'db TestDb, tys: impl IntoIterator<Item = Type<'db>>) -> Type

mod stable {
use super::union;
use crate::types::{KnownClass, Type};
use crate::types::Type;

// Reflexivity: `T` is equivalent to itself.
type_property_test!(
Expand Down Expand Up @@ -419,13 +419,13 @@ mod stable {
// All types should be assignable to `object`
type_property_test!(
all_types_assignable_to_object, db,
forall types t. t.is_assignable_to(db, KnownClass::Object.to_instance(db))
forall types t. t.is_assignable_to(db, Type::object(db))
);

// And for fully static types, they should also be subtypes of `object`
type_property_test!(
all_fully_static_types_subtype_of_object, db,
forall types t. t.is_fully_static(db) => t.is_subtype_of(db, KnownClass::Object.to_instance(db))
forall types t. t.is_fully_static(db) => t.is_subtype_of(db, Type::object(db))
);

// Never should be assignable to every type
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/types/signatures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ mod tests {
},
Parameter {
name: Some(Name::new_static("args")),
annotated_ty: Some(KnownClass::Object.to_instance(&db)),
annotated_ty: Some(Type::object(&db)),
kind: ParameterKind::Variadic,
},
Parameter {
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/types/subclass_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl<'db> SubclassOfType<'db> {
ClassBase::Class(class) => {
if class.is_final(db) {
Type::ClassLiteral(ClassLiteralType { class })
} else if class.is_known(db, KnownClass::Object) {
} else if class.is_object(db) {
KnownClass::Type.to_instance(db)
} else {
Type::SubclassOf(Self { subclass_of })
Expand Down
Loading