[ty] Make Divergent a top-level type variant#24252
Conversation
bf1cf64 to
6a610f1
Compare
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 86.61%. The percentage of expected errors that received a diagnostic held steady at 81.56%. The number of fully passing files held steady at 70/132. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
|
6a610f1 to
85f0d4e
Compare
carljm
left a comment
There was a problem hiding this comment.
Ecosystem results suggest that one case we missed here is skipping redundant-cast for divergent types.
I think it may be that both inline comments, plus the above, will be fixed if we make Type::is_dynamic return true for a non-materialized Type::Divergent.
| fn extend_with_type(&mut self, db: &'db dyn Db, ty: Type<'db>) { | ||
| match ty { | ||
| Type::Union(union) => { | ||
| fn is_dynamic(db: &dyn Db, ty: Type<'_>) -> bool { |
There was a problem hiding this comment.
Codex suggests we also need this helper to return true for Type::Divergent in order to maintain the prior behavior here. It says that this test passes on main but fails after this PR, without this change:
from ty_extensions import has_member, static_assert
class Base:
def flip(self) -> "Base":
return Base()
class Sub(Base):
pass
class C:
def __init__(self, x: Sub):
self.x = [x]
def replace_with(self, other: "C"):
self.x = [self.x[0].flip()]
static_assert(has_member(C(Sub()).x[0], "flip"))There was a problem hiding this comment.
Codex also found this check -- it should return true for Type::Divergent, or else the behavior of this snippet regresses (we get an invalid-key on the index on the last line):
from typing import TypedDict
A = list["A | None"]
class Movie(TypedDict):
name: str
def f(x: A, movie: Movie):
movie[x[0]]85f0d4e to
dce8f53
Compare
dce8f53 to
b8334c7
Compare
|
What When I first introduced However, in this PR, Theoretically, it would make the most sense to reinterpret |
|
@mtshiba I think it has never been the case (even from the first PR) that Perhaps ideally
I would say that recursive types do exist in Python's type system, given that the type spec and conformance suite require support for recursive type aliases. The question of how to represent them is not specified, however. We've discussed using a de Bruijn index representation to allow transformations of recursive types to not fall back to |
b8334c7 to
6ebadc8
Compare
* main: (35 commits) Store definition indexes as u32 (#24307) Avoid re-using symbol in RUF024 fix (#24316) [ty] Add materialization to `Divergent` type (#24255) [ty] Make `Divergent` a top-level type variant (#24252) [ty] Fix nested global and nonlocal lookups through forwarding scopes (#24279) Fetch the cargo-dist binary directly instead of using the installer (#24258) [ty] Fix panic on `list[Annotated[()]]` (#24303) Don't measure the AST deallocation time in parser benchmarks (#24301) Enable CodSpeed's memory benchmarks for simulation benchmarks (#24298) Upgrade imara-diff to 0.2.0 (#24299) [ty] Represent `InitVar` as a special form internally, not a class (#24248) `RUF067`: Allow dunder-named assignments in non-strict mode [`pyupgrade`] UP018 should detect more unnecessarily wrapped literals (UP018) (#24093) [ty] Remove unused `system.glob` method (#24300) [ty] Reject functional TypedDict with mismatched name (#24295) Update Rust crate arc-swap to v1.9.0 (#24292) [ty] Remove unused `@Todo(Functional TypedDicts)` (#24297) Update CodSpeedHQ/action action to v4.12.1 (#24290) Update taiki-e/install-action action to v2.69.6 (#24293) Update Rust crate toml to v1.0.7 (#24289) ...
@carljm What makes You might then wonder why the behavior of
I now find it more appealing to reinterpret Currently, we only have the Is this the same as what you have in mind?
Whether the recursive types mentioned here can be interpreted as recursive type aliases is debatable, as they are synthesized during type inference and are not "aliases" of anything. However, this kind of internal extension is present in ty, with examples like I would like to create a PoC to replace |
Summary
This PR follows #24245 (comment), making
Divergenta top-level type rather than aDynamicTypevariant.