From e56c382fb6f340ee0d2ee362c6b0d2c9bcf24c37 Mon Sep 17 00:00:00 2001 From: KotlinIsland <65446343+kotlinisland@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:30:00 +1000 Subject: [PATCH 1/5] fix a false invalid-base, two cycle panics and a subscript crash in ty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit three independent type-inference defects the native backend work ran into while checking real projects. a parameter-shape base spelled as a parenthesised tuple was inferred as a runtime tuple value and reported `invalid-base`, so ty rejected code the transpiler accepts and emits as `class C(tuple[A, B])`. a `src/` holding files named after stdlib modules made `by check` panic and take every other file in the run with it. a panic that cancels the whole run is worse than any diagnostic being wrong, and both of these reproduced on the baseline. the first is `known_class_to_instance`. a first-party module that shadows a stdlib one makes a known class resolve into code whose own inference asks for that same known class again; specializing what was found reopens the recursion a level lower, through `generic_context` and `explicit_bases`. the sibling query that performs the lookup half already recovers, so this one gets the same treatment rather than a new policy. the recovery answers `Unknown` for one fixpoint iteration inside the cycle and nothing outside it. the second is `inferred_return_type`'s recovery running a salsa query of its own, which salsa forbids. the query was reached through the union builder asking a literal for its fallback *instance* — which means resolving the class's module and reading the symbol out of it. it now asks the element it already has which class it is instead, keeping "which class is this literal's fallback" apart from "what type does that class make". where the lookup used to fail, which is exactly the shadowed case, the old comparison matched nothing and kept a redundant literal beside its own instance, so the new form is strictly more precise. `object & ~int`, which is what `if not isinstance(x, int)` narrows to, carries no positive element of its own. both the subscript store and the delete path iterated those elements alone, so they visited nothing at all there. for the store that was a crash: `infer_loud` was never reached, so a `MultiInferenceGuard` was dropped unfinalised and its `debug_assert` aborted the run. it was also losing real work — the key and the assigned value were never inferred with diagnostics, so `x[missing_key] = missing_value` silently dropped two `unresolved-reference` errors as well. for the delete it was quieter: nothing was reported at all where `object` has no `__delitem__`. an intersection with only negatives still has a positive bound, because everything is an `object`. the read side already fell back to it through `positive_elements_or_object`, which exists for this — so this is a known pattern applied in the two places that had missed it, not a new rule. Co-Authored-By: Claude Opus 5 --- .../mdtest/basedpython_tuple_base.md | 65 ++++++++++++++++ .../resources/mdtest/cycle.md | 77 +++++++++++++++++++ .../resources/mdtest/del.md | 15 ++++ ..._narr\342\200\246_(e7d27f4362b537ae).snap" | 53 +++++++++++++ .../subscript/assignment_diagnostics.md | 15 ++++ .../src/types/class/known.rs | 13 +++- .../src/types/infer/builder/class.rs | 67 +++++++++++++++- .../src/types/infer/builder/subscript.rs | 26 +++++-- .../types/infer/builder/type_expression.rs | 13 +++- .../ty_python_semantic/src/types/instance.rs | 17 ++++ .../ty_python_semantic/src/types/literal.rs | 70 ++++++++++++++--- .../src/types/set_theoretic/builder.rs | 23 +++--- 12 files changed, 421 insertions(+), 33 deletions(-) create mode 100644 crates/ty_python_semantic/resources/mdtest/basedpython_tuple_base.md create mode 100644 "crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_An_intersection_narr\342\200\246_(e7d27f4362b537ae).snap" diff --git a/crates/ty_python_semantic/resources/mdtest/basedpython_tuple_base.md b/crates/ty_python_semantic/resources/mdtest/basedpython_tuple_base.md new file mode 100644 index 0000000000..7abf93566c --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/basedpython_tuple_base.md @@ -0,0 +1,65 @@ +# basedpython: a tuple type as a class base + +`(*: T)` is basedpython for `tuple[T, ...]`, and `(A, B)` is basedpython for `tuple[A, B]`. Both are +type-only forms — no runtime value is ever spelled that way — so in a class base list they name the +tuple type the class inherits from, exactly as the transpiler lowers them. + +```toml +[environment] +python-version = "3.12" +``` + +## a variadic tuple base is a `tuple` subclass + +```by +class Row((*: int)): + pass + +def f(row: Row): + reveal_type(row[0]) # revealed: int + for cell in row: + reveal_type(cell) # revealed: int +``` + +## a variadic tuple base does not make the class assignable to everything + +The base has to resolve. When it does not, the class carries `Unknown` in its MRO and silently +satisfies every annotation. + +```by +class Row((*: int)): + pass + +def f(row: Row): + # error: [invalid-assignment] "Object of type `Row` is not assignable to `int`" + x: int = row + y: (*: int) = row +``` + +## `sys.flags` is not an `int` + +`sys.flags` is declared in the typeshed with exactly this base, so it is the case that first showed +the hole. + +```py +import sys + +# error: [invalid-assignment] "Object of type `_flags` is not assignable to `int`" +x: int = sys.flags +reveal_type(sys.flags.debug) # revealed: int +``` + +## a variadic tuple base composes with a nominal base + +```by +class Mixin: + def describe(self) -> str: + return "row" + +class Row(Mixin, (*: str)): + pass + +def f(row: Row): + reveal_type(row.describe()) # revealed: str + reveal_type(row[0]) # revealed: str +``` diff --git a/crates/ty_python_semantic/resources/mdtest/cycle.md b/crates/ty_python_semantic/resources/mdtest/cycle.md index 32885c9d18..65e2bc8d36 100644 --- a/crates/ty_python_semantic/resources/mdtest/cycle.md +++ b/crates/ty_python_semantic/resources/mdtest/cycle.md @@ -1187,3 +1187,80 @@ def run(flag: int): t = 0.6 reveal_type(t) # revealed: 0.1 | 0.2 | 0.3 | 0.4 | 0.5 | 0.6 ``` + +## a first-party module that shadows the home of a known class + +The `collections.abc` ABCs really live in `_collections_abc`, so a project carrying a file of that +name is what `Sequence` resolves to — and `str` is declared in terms of `Sequence` in the typeshed. +Working out what an instance of a known class is therefore runs through the shadowing file, whose +own classes ask for that known class back before it has one. + +The metaclass here is load-bearing: a metaclass that is not itself a class is the thing that sends +working out `Base`'s metaclass through `str`, which is where the two directions meet. + +`_collections_abc.py`: + +```py +def meta(name, bases, namespace): ... + +class Base(metaclass=meta): ... +class Sequence(Base): ... +``` + +Neither direction can finish before the other, and while that is being untangled the known class +answers `Unknown` — the same thing it answers when it cannot be found at all. Once the recursion has +settled it is the class the typeshed declares again, so nothing outside the cycle pays for it: + +```py +reveal_type("a".upper()) # revealed: LiteralString +reveal_type([1, 2, 3]) # revealed: list[int] +``` + +## the class a literal falls back to, while the module it lives in is still being untangled + +A project can carry files named after several stdlib modules that import one another, and then its +own `typing`, `warnings` and `linecache` are in a cycle with each other. `typing` is where the +typeshed reaches for the pieces `builtins` is written in terms of, so while that cycle is being +untangled, *which class `str` is* has no answer yet either. + +`linecache.py`: + +```py +def getline(lineno): + if lineno: + return lines[lineno - 1] # error: [unresolved-reference] + return "" +``` + +`warnings.py`: + +```py +import linecache + +line = linecache.getline(1) + +def _deprecated(*, remove, _version=sys.version_info): # error: [unresolved-reference] + pass +``` + +`typing.py`: + +```py +import collections +import warnings + +warnings._deprecated(remove=1) + +Sequence = collections.abc.Sequence +``` + +`getline` says nothing about what it returns, so its return type is recovered from the cycle, and +recovering it rebuilds the union of everything the body hands back. Adding `Literal[""]` to a union +that already holds a `str` means deciding whether the two say the same thing — and asking that by +building the `str` this program means would re-enter, from inside the recovery, the very cycle being +recovered from. The class the existing type is already carrying answers it without going anywhere: + +```py +reveal_type("a".upper()) # revealed: LiteralString +reveal_type([1, 2, 3]) # revealed: list[int] +``` diff --git a/crates/ty_python_semantic/resources/mdtest/del.md b/crates/ty_python_semantic/resources/mdtest/del.md index 6fad0dad95..cdd01e254e 100644 --- a/crates/ty_python_semantic/resources/mdtest/del.md +++ b/crates/ty_python_semantic/resources/mdtest/del.md @@ -513,3 +513,18 @@ error[invalid-argument-type]: Cannot delete unknown key "non_existent" from Type 25 | del mixed["non_existent"] | ^^^^^^^^^^^^^^ ``` + +### An intersection narrowed down to nothing but negatives + +`~int` carries no positive element of its own, but it still has a positive bound: everything is an +`object`, and `object` has no `__delitem__`. Without that bound the deletion was checked against no +element at all and went unreported. Unlike the assignment case, the key is inferred either way — +only the deletion diagnostic was lost. + +```py +def _(x: object) -> None: + if not isinstance(x, int): + # error: [not-subscriptable] + # error: [unresolved-reference] + del x[missing_key] +``` diff --git "a/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_An_intersection_narr\342\200\246_(e7d27f4362b537ae).snap" "b/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_An_intersection_narr\342\200\246_(e7d27f4362b537ae).snap" new file mode 100644 index 0000000000..f50d207eab --- /dev/null +++ "b/crates/ty_python_semantic/resources/mdtest/snapshots/assignment_diagnosti\342\200\246_-_Subscript_assignment\342\200\246_-_An_intersection_narr\342\200\246_(e7d27f4362b537ae).snap" @@ -0,0 +1,53 @@ +--- +source: crates/mdtest/src/lib.rs +expression: snapshot +--- + +--- +mdtest name: assignment_diagnostics.md - Subscript assignment diagnostics - An intersection narrowed down to nothing but negatives +mdtest path: crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md +--- + +# Python source files + +## mdtest_snippet.py + +``` +1 | def _(x: object) -> None: +2 | if not isinstance(x, int): +3 | # error: [invalid-assignment] +4 | # error: [unresolved-reference] +5 | # error: [unresolved-reference] +6 | x[missing_key] = missing_value +``` + +# Diagnostics + +``` +error[invalid-assignment]: Cannot assign to a subscript on an object of type `object` + --> src/mdtest_snippet.py:6:9 + | +6 | x[missing_key] = missing_value + | ^^^^^^^^^^^^^^ +info: The full type of the subscripted object is `~int` +info: `object` does not have a `__setitem__` method. + +``` + +``` +error[unresolved-reference]: Name `missing_key` used when not defined + --> src/mdtest_snippet.py:6:11 + | +6 | x[missing_key] = missing_value + | ^^^^^^^^^^^ + +``` + +``` +error[unresolved-reference]: Name `missing_value` used when not defined + --> src/mdtest_snippet.py:6:26 + | +6 | x[missing_key] = missing_value + | ^^^^^^^^^^^^^ + +``` diff --git a/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md b/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md index 5e20895a2c..8cbd5abcea 100644 --- a/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md +++ b/crates/ty_python_semantic/resources/mdtest/subscript/assignment_diagnostics.md @@ -151,3 +151,18 @@ def _(config: dict[str, int] | dict[str, str]) -> None: # error: [invalid-assignment] config["retries"] = 3.0 ``` + +## An intersection narrowed down to nothing but negatives + +`~int` carries no positive element of its own, but it still has a positive bound: everything is an +`object`, and `object` has no `__setitem__`. Checking against that bound is also what infers the key +and the assigned value, so the names in them are resolved rather than skipped. + +```py +def _(x: object) -> None: + if not isinstance(x, int): + # error: [invalid-assignment] + # error: [unresolved-reference] + # error: [unresolved-reference] + x[missing_key] = missing_value +``` diff --git a/crates/ty_python_semantic/src/types/class/known.rs b/crates/ty_python_semantic/src/types/class/known.rs index 3db39dabaa..8401e498e7 100644 --- a/crates/ty_python_semantic/src/types/class/known.rs +++ b/crates/ty_python_semantic/src/types/class/known.rs @@ -1188,7 +1188,18 @@ impl KnownClass { "Use `Type::heterogeneous_tuple` or `Type::homogeneous_tuple` to create `tuple` instances" ); - #[salsa::tracked(returns(copy), heap_size=ruff_memory_usage::heap_size)] + // the lookup this delegates to already recovers from cycles by answering "no such + // class", and it has to: a first-party module that shadows a stdlib one (a project + // with its own `_collections_abc.py`, say) makes a known class resolve into code + // whose own inference asks for that same known class again. specializing the class + // we found reopens exactly that recursion one level further down — through + // `generic_context` and `explicit_bases` — so this query needs the same recovery, + // or salsa aborts the whole run + #[salsa::tracked( + returns(copy), + cycle_initial=|_, _, _| Type::unknown(), + heap_size=ruff_memory_usage::heap_size, + )] fn known_class_to_instance<'db>( db: &'db dyn Db, argument: KnownClassArgument<'db>, diff --git a/crates/ty_python_semantic/src/types/infer/builder/class.rs b/crates/ty_python_semantic/src/types/infer/builder/class.rs index 94cf157dde..3b4df1d10b 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/class.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/class.rs @@ -130,7 +130,11 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let mut is_typed_dict = false; for base in class.bases() { - let ty = if let ast::Expr::Starred(starred) = base { + let ty = if let Some(ty) = + self.infer_parameter_shape_class_base(base, defer_class_args) + { + ty + } else if let ast::Expr::Starred(starred) = base { let ty = self.infer_expression(&starred.value, TypeContext::default()); self.store_expression_type(base, ty); ty @@ -580,13 +584,66 @@ impl<'db> TypeInferenceBuilder<'db, '_> { let previous_typevar_binding_context = self.typevar_binding_context.replace(definition); for base in class_node.bases() { - self.infer_expression(base, TypeContext::default()); + if self + .infer_parameter_shape_class_base(base, /* deferred = */ false) + .is_none() + { + self.infer_expression(base, TypeContext::default()); + } } self.typevar_binding_context = previous_typevar_binding_context; } } } + /// basedpython: resolve a class base written in the callable-parameter tuple form + /// + /// `(*: int)` is how basedpython spells `tuple[int, ...]`, and it is what the reverse + /// transpiler writes into the `.byi` typeshed — `class _flags(_UninstantiableStructseq, + /// (*: int))` is `sys.flags`. the form is type-only: no runtime value is ever spelled + /// that way. inferring it as a value therefore reads its `*: int` element as an unpack + /// in value position and yields `Unknown`, which as a base makes the class assignable + /// to every type. so resolve the base as a type expression, exactly as the transpiler + /// lowers it, and hand back the class the resulting instance is an instance of, which + /// is the shape a base list needs. + /// + /// returns `None` for every base this does not apply to, leaving the caller's ordinary + /// value inference to run + fn infer_parameter_shape_class_base( + &mut self, + base: &ast::Expr, + deferred: bool, + ) -> Option> { + if !self.is_basedpython_file() { + return None; + } + let ast::Expr::Tuple(tuple) = base else { + return None; + }; + if !tuple.has_parameter_shape() { + return None; + } + let previous_deferred_state = if deferred { + Some(std::mem::replace( + &mut self.deferred_state, + DeferredExpressionState::Deferred, + )) + } else { + None + }; + let ty = self.infer_type_expression_unstored(base); + if let Some(previous) = previous_deferred_state { + self.deferred_state = previous; + } + // a base that does not denote a class is recorded as it is, so the ordinary + // `invalid-base` report fires instead of the type silently going missing + let base_ty = ty + .nominal_class(self.db(), self.program_environment()) + .map_or(ty, Type::from); + self.store_expression_type(base, base_ty); + Some(base_ty) + } + pub(super) fn infer_class_deferred( &mut self, definition: Definition<'db>, @@ -603,6 +660,12 @@ impl<'db> TypeInferenceBuilder<'db, '_> { .inference_flags .replace(InferenceFlags::IN_TYPE_EXPRESSION, true); for base in class.bases() { + if self + .infer_parameter_shape_class_base(base, defer_class_args) + .is_some() + { + continue; + } if defer_class_args { self.infer_expression_with_state( base, diff --git a/crates/ty_python_semantic/src/types/infer/builder/subscript.rs b/crates/ty_python_semantic/src/types/infer/builder/subscript.rs index 41eaa55f24..9edae2edef 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/subscript.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/subscript.rs @@ -2433,13 +2433,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let mut infer_slice_ty = MultiInferenceGuard::new(infer_slice_ty); let mut infer_rhs_value = MultiInferenceGuard::new(infer_rhs_value); + // an intersection with nothing but negative elements — `object & ~int`, from + // `if not isinstance(x, int)` — still has a positive bound, because everything + // is an `object`. iterating the positive elements alone would visit nothing at + // all here, which leaves the slice and the assigned value never inferred out + // loud. the read side of a subscript takes the same fallback let mut check_positive_elements = |emit_diagnostic_and_short_circuit| { let mut valid = false; - for element_ty in intersection.positive(db) { + for element_ty in intersection.positive_elements_or_object(db) { valid |= self.validate_subscript_assignment_impl( target, full_object_ty.or(Some(object_ty)), - *element_ty, + element_ty, &mut |builder, tcx| infer_slice_ty.infer_silent(builder, tcx), rhs_value_node, &mut |builder, tcx| infer_rhs_value.infer_silent(builder, tcx), @@ -2841,22 +2846,27 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } Type::Intersection(intersection) => { - // Check if any positive element supports deletion - let positive = intersection.positive(db); + // an intersection with nothing but negative elements — `object & ~int`, from + // `if not isinstance(x, int)` — still has a positive bound, because everything + // is an `object`. iterating the positive elements alone visits nothing at all + // there, so the deletion went unchecked and nothing was reported. the store and + // read sides of a subscript both take the same fallback let mut any_valid = false; - for element_ty in positive { - if self.can_delete_subscript(*element_ty, slice_ty) { + let mut first = None; + for element_ty in intersection.positive_elements_or_object(db) { + first.get_or_insert(element_ty); + if self.can_delete_subscript(element_ty, slice_ty) { any_valid = true; break; } } // If none are valid, emit a diagnostic for the first failing element - if !any_valid && let Some(element_ty) = positive.first() { + if !any_valid && let Some(element_ty) = first { self.validate_subscript_deletion_impl( target, full_object_ty.or(Some(object_ty)), - *element_ty, + element_ty, slice_ty, ); } diff --git a/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs b/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs index 4b63f8529e..13e82810e5 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs @@ -51,6 +51,18 @@ impl<'db> TypeInferenceBuilder<'db, '_> { /// Infer the type of a type expression. pub(super) fn infer_type_expression(&mut self, expression: &ast::Expr) -> Type<'db> { + let ty = self.infer_type_expression_unstored(expression); + self.store_expression_type(expression, ty); + ty + } + + /// Infer the type of a type expression in type-expression context, without recording the + /// result against `expression`. + /// + /// A caller that stores a *different* type against the expression than the one the type + /// expression denotes needs this, because `store_expression_type` accepts one entry per + /// node. + pub(super) fn infer_type_expression_unstored(&mut self, expression: &ast::Expr) -> Type<'db> { let previous_deferred_state = self.deferred_state; let was_in_type_expression = self .inference_flags() @@ -89,7 +101,6 @@ impl<'db> TypeInferenceBuilder<'db, '_> { InferenceFlags::IN_TYPE_EXPRESSION, previously_in_type_expression, ); - self.store_expression_type(expression, ty); ty } diff --git a/crates/ty_python_semantic/src/types/instance.rs b/crates/ty_python_semantic/src/types/instance.rs index d0d5243a1b..e2b346832b 100644 --- a/crates/ty_python_semantic/src/types/instance.rs +++ b/crates/ty_python_semantic/src/types/instance.rs @@ -351,6 +351,23 @@ impl<'db> NominalInstanceType<'db> { file_to_module(db, class.program_file(db).resolver_file(db)).map(|module| module.name(db)) } + /// The class this is an instance of, when the instance is already carrying it. + /// + /// `object`, `sys.version_info` and an exact tuple name their class rather than hold it, + /// and finding the class a name stands for means resolving the module it lives in and + /// reading the symbol out — a salsa query. A cycle recovery function may not run a query + /// it was not already inside, on pain of salsa aborting the whole run, so a caller that + /// runs inside one asks this instead and reads `None` as "not without looking it up". + pub(crate) fn class_without_lookup(&self, db: &'db dyn Db) -> Option> { + match self.0 { + NominalInstanceInner::NonTuple(class) => Some(class.class(db)), + NominalInstanceInner::Regex(regex) => Some(regex.class(db)), + NominalInstanceInner::ExactTuple(_) + | NominalInstanceInner::SysVersionInfo + | NominalInstanceInner::Object => None, + } + } + pub(crate) fn class(&self, db: &'db dyn Db, env: &ProgramEnvironment<'db>) -> ClassType<'db> { match self.0 { NominalInstanceInner::ExactTuple(tuple) => tuple.to_class_type(db), diff --git a/crates/ty_python_semantic/src/types/literal.rs b/crates/ty_python_semantic/src/types/literal.rs index fa8e4e1e22..71e70515f7 100644 --- a/crates/ty_python_semantic/src/types/literal.rs +++ b/crates/ty_python_semantic/src/types/literal.rs @@ -282,23 +282,73 @@ impl<'db> LiteralValueType<'db> { } } + /// Which class this literal's values are instances of, named rather than looked up. + pub(crate) fn fallback(self, db: &'db dyn Db) -> LiteralFallback<'db> { + match self.kind() { + LiteralValueTypeKind::String(_) + | LiteralValueTypeKind::LiteralString + | LiteralValueTypeKind::Template(_) => LiteralFallback::Known(KnownClass::Str), + LiteralValueTypeKind::Bool(_) => LiteralFallback::Known(KnownClass::Bool), + LiteralValueTypeKind::Int(_) => LiteralFallback::Known(KnownClass::Int), + LiteralValueTypeKind::Bytes(_) => LiteralFallback::Known(KnownClass::Bytes), + LiteralValueTypeKind::Enum(literal) => LiteralFallback::Enum(literal.enum_class(db)), + LiteralValueTypeKind::Float(_) => LiteralFallback::Known(KnownClass::Float), + LiteralValueTypeKind::Complex(_) => LiteralFallback::Known(KnownClass::Complex), + } + } + pub(crate) fn fallback_instance( self, db: &'db dyn Db, env: &ProgramEnvironment<'db>, ) -> Type<'db> { - match self.kind() { - LiteralValueTypeKind::String(_) - | LiteralValueTypeKind::LiteralString - | LiteralValueTypeKind::Template(_) => KnownClass::Str.to_instance(db, env), - LiteralValueTypeKind::Bool(_) => KnownClass::Bool.to_instance(db, env), - LiteralValueTypeKind::Int(_) => KnownClass::Int.to_instance(db, env), - LiteralValueTypeKind::Bytes(_) => KnownClass::Bytes.to_instance(db, env), - LiteralValueTypeKind::Enum(literal) => literal.enum_class_instance(db, env), - LiteralValueTypeKind::Float(_) => KnownClass::Float.to_instance(db, env), - LiteralValueTypeKind::Complex(_) => KnownClass::Complex.to_instance(db, env), + match self.fallback(db) { + LiteralFallback::Known(known) => known.to_instance(db, env), + LiteralFallback::Enum(class) => class.to_non_generic_instance(db, env), } } + + /// Whether `ty` is the very type [`Self::fallback_instance`] would build. + /// + /// For a builtin this is answered from the class `ty` is already carrying, without + /// building the fallback: naming `str` is free, but *finding* the `str` a program means + /// is a salsa query, and cycle recovery may not run one it was not already inside. + /// Reading the class off the type in hand also gives the right answer where building it + /// cannot — a project whose own module shadows the one a builtin lives in makes the + /// lookup fail, and a failed lookup matches nothing at all. + pub(crate) fn is_fallback_instance( + self, + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + ty: Type<'db>, + ) -> bool { + match self.fallback(db) { + LiteralFallback::Known(known) => match ty { + Type::NominalInstance(instance) => instance + .class_without_lookup(db) + .is_some_and(|class| class.is_known(db, known)), + _ => false, + }, + // an enum literal carries the enum it belongs to, so there is nothing to look up + LiteralFallback::Enum(class) => class.to_non_generic_instance(db, env) == ty, + } + } +} + +/// The class a literal type's values are instances of. +/// +/// Which class it is and what type that class makes are kept apart on purpose. The first is +/// a property of the literal; the second means resolving the module the class lives in and +/// reading the symbol out of it, which is a salsa query — and a query is exactly what a +/// cycle recovery function must not run, or salsa aborts the run rather than let the +/// recovery pull a half-finished query of its own into the cycle. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum LiteralFallback<'db> { + /// A builtin — `str`, `int`, `bytes`, `bool`, `float`, `complex` — which has to be found + /// before its instance type can be built. + Known(KnownClass), + /// The enum a member belongs to, which the literal is already holding. + Enum(ClassLiteral<'db>), } impl From for LiteralValueTypeKind<'_> { diff --git a/crates/ty_python_semantic/src/types/set_theoretic/builder.rs b/crates/ty_python_semantic/src/types/set_theoretic/builder.rs index 5072fa8a1a..ee2704b60e 100644 --- a/crates/ty_python_semantic/src/types/set_theoretic/builder.rs +++ b/crates/ty_python_semantic/src/types/set_theoretic/builder.rs @@ -781,8 +781,8 @@ impl<'db> UnionBuilder<'db> { } UnionElement::Type(existing) if cycle_recovery - && literal.fallback_instance(db, &self.env) - == *existing => + && literal + .is_fallback_instance(db, &self.env, *existing) => { return; } @@ -836,8 +836,8 @@ impl<'db> UnionBuilder<'db> { } UnionElement::Type(existing) if cycle_recovery - && literal.fallback_instance(db, &self.env) - == *existing => + && literal + .is_fallback_instance(db, &self.env, *existing) => { return; } @@ -893,8 +893,8 @@ impl<'db> UnionBuilder<'db> { } UnionElement::Type(existing) if cycle_recovery - && literal.fallback_instance(db, &self.env) - == *existing => + && literal + .is_fallback_instance(db, &self.env, *existing) => { return; } @@ -970,8 +970,8 @@ impl<'db> UnionBuilder<'db> { } UnionElement::Type(existing) if cycle_recovery - && literal.fallback_instance(db, &self.env) - == *existing => + && literal + .is_fallback_instance(db, &self.env, *existing) => { return; } @@ -1072,7 +1072,7 @@ impl<'db> UnionBuilder<'db> { } let db = self.db; - let fallback = literal.fallback_instance(db, &self.env); + let fallback = literal.fallback(db); let mut same_kind = SmallVec::<[usize; 8]>::new(); for (index, element) in self.elements.iter().enumerate() { let UnionElement::Type(existing) = element else { @@ -1081,11 +1081,11 @@ impl<'db> UnionBuilder<'db> { // Once widened, the instance type stands for every literal of its kind, this one // included. Outside recovery `push_type` reaches the same conclusion through the // ordinary redundancy check, which also applies the simplifications skipped here. - if self.cycle_recovery && *existing == fallback { + if self.cycle_recovery && literal.is_fallback_instance(db, &self.env, *existing) { return true; } if let Type::LiteralValue(existing_literal) = existing - && existing_literal.fallback_instance(db, &self.env) == fallback + && existing_literal.fallback(db) == fallback { same_kind.push(index); } @@ -1099,6 +1099,7 @@ impl<'db> UnionBuilder<'db> { for index in same_kind.into_iter().rev() { self.elements.remove(index); } + let fallback = literal.fallback_instance(db, &self.env); self.add_in_place_impl(fallback, seen_aliases); true } From 6f4130945529679d41e6f5224c451f6bdf99204e Mon Sep 17 00:00:00 2001 From: KotlinIsland <65446343+kotlinisland@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:32:16 +1000 Subject: [PATCH 2/5] answer correctly where the compiled module used to answer quietly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit seven defects, each one a case where an emitted module gave a different answer from the interpreted twin and said nothing about it. a silent wrong answer is the worst thing this compiler can do — a decline costs speed, a crash is at least visible, and these were neither. five of them shared one shape: a hand-maintained list that had to agree with another list, and did not. `__dict__` on an emitted instance is now a real `dict` over the whole state that every write updates, rather than a mapping that named only half of it. a decline protects the function it is raised in, not the class that function is handed — `render` declined correctly, the interpreted twin ran, and it still failed because the emitted class it was given had no `__dict__` to reach. `walk` now descends into `match` case bodies, and `written_names` and `local_representations` learned pattern captures alongside it; widening `walk` alone traded an honest decline for a wrong answer, so all three moved together. `dead-registers` no longer carries its own copy of every operand's shape: it goes through `Op::dest_mut`, `Op::loop_cursor_mut` and `Op::operands_mut`, so there is one list rather than two that had to agree, and `ArraySet` was the one that had drifted. the `warnings.warn` class-body gate is lifted — the two defects its first re-costing turned up are both fixed — and the weakref gate now asks whether an instance of ours could stand where the referent is read, so `ref(self.attr)` is left alone where `ref(self)` still refuses. the sixth is binding. a `PyCFunction` is not a descriptor, so `Cls.method = mod.fn` — which is what `functools.total_ordering` does — installed something that never received the receiver. with a default on that receiver it answered quietly rather than raising: `Thing().label()` gave `'bound'` interpreted and `'UNBOUND'` compiled, from ordinary code with nothing reported. cpython offers no way to make a `PyCFunction` bind, and a custom binding type loses `inspect.isroutine` and `copy`'s atomic handling, so neither of those was the answer. each exported module-level definition now gets a real `function` written into the interpreted twin, which forwards to the native and takes its name, docstring, defaults, annotations and `__wrapped__` from the definition standing under the same name. it takes `*args, **kwargs` deliberately: written with the exact parameter list it fails, because the transpiler rewrites a mutable default into a sentinel plus a body test, so the twin's `__defaults__` holds a sentinel the native has never heard of. a transparent forwarder makes no arity or default decision at all, so none of them can be got wrong. the cost is one forwarder frame on the single entry through a module's own name: measured at ~40ns, constant in the work done, because an intra-module call is `Op::CallNative` by symbol and never touches the module namespace. two more were found on the way — a module defining a function called `globals` made the installer fail on its own first line, and the forwarder's frame broke `warnings.warn(stacklevel=…)`, which now steps over it. the seventh is comparison. one `tp_richcompare` backs all six, so a class writing `__lt__` took the slot over from `object` for the other five as well, and two things followed. `object`'s richcompare is not empty — it is where `!=` gets its meaning, negate `__eq__` — and answering `NotImplemented` in its place threw that away: `Money(5) != Money(5)` was `True` compiled against `False` interpreted, from ordinary code with no decorator anywhere. an unwritten comparison now goes to the base's slot, where python would have answered it. and `PyType_Ready` publishes a wrapper under every name a filled slot backs, so the type advertised a `__le__`, `__gt__` and `__ge__` this body never wrote — which is why `functools.total_ordering` saw all four roots present and set none. those names come back off after the type is built; the slot is untouched, so what a comparison *does* is unchanged and only what the class *says* moves. the same goes for the pairs a binary number slot backs and for `__setitem__` alongside `__delitem__`. with that gone, the class-statement decline for a decorated class is no longer needed and `@total_ordering` on a class compiles where it used to fall back entirely — a decline recovered rather than replaced. removing it exposed a cascade that was always there: a class whose decline is settled at the end of its lowering has a layout by then, and every class holding one declines too. `tracemalloc` went 50 compiled to 26 that way. the question is asked in `class_fields` now, where the layout is decided, so a declining class is an ordinary object to the rest of the module. the semantic-delta table is updated throughout for what moves. `type(f)` and the binding row go, since both now match python; `__code__`, `__wrapped__` and the forwarder's traceback frame arrive; and applying `@total_ordering` from another module raises at the `setattr`, since an emitted class is sealed — loud where it used to be quiet. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 + crates/by_build/src/lib.rs | 13 + crates/by_build/tests/differential.rs | 1331 +++++++++++++++-- crates/by_build/tests/end_to_end.rs | 74 + crates/by_codegen_c/src/lib.rs | 693 ++++++++- crates/by_ir/src/builder.rs | 26 + crates/by_ir/src/function.rs | 210 ++- crates/by_ir/src/ops.rs | 66 +- crates/by_ir/src/print.rs | 8 +- crates/by_ir/src/verify.rs | 17 +- crates/by_irbuild/src/closures.rs | 56 +- crates/by_irbuild/src/lib.rs | 765 ++++++++-- crates/by_irbuild/src/mapper.rs | 10 - crates/by_irbuild/src/shims.rs | 259 ++++ crates/by_irbuild/src/tests.rs | 462 +++++- crates/by_opt/src/borrow.rs | 147 +- crates/by_opt/src/coalesce.rs | 1 + crates/by_opt/src/copy_propagation.rs | 1 + crates/by_opt/src/dead_registers.rs | 496 +----- crates/by_opt/src/dict_find.rs | 1 + crates/by_opt/src/fold.rs | 1 + crates/by_opt/src/infallible.rs | 1 + crates/by_opt/src/lib.rs | 2 + crates/by_opt/src/refcount.rs | 1 + crates/by_opt/src/str_append.rs | 1 + crates/by_opt/src/str_item_compare.rs | 1 + crates/by_opt/src/unbox_counters.rs | 45 + crates/by_opt/src/unswitch.rs | 1 + crates/by_rt/include/by.h | 1060 ++++++++++++- crates/ty_python_semantic/src/lib.rs | 2 +- .../ty_python_semantic/src/semantic_model.rs | 15 + crates/ty_python_semantic/src/types.rs | 15 + .../development/compilation/plan.md | 68 +- .../development/compilation/runtime.md | 20 +- 34 files changed, 4961 insertions(+), 911 deletions(-) create mode 100644 crates/by_irbuild/src/shims.rs diff --git a/.gitignore b/.gitignore index f75e68c745..cc450645e1 100644 --- a/.gitignore +++ b/.gitignore @@ -227,3 +227,6 @@ cython_debug/ !crates/ruff_python_resolver/resources/test/airflow/venv/lib !crates/ruff_python_resolver/resources/test/airflow/venv/lib/python3.11/site-packages/_watchdog_fsevents.cpython-311-darwin.so !crates/ruff_python_resolver/resources/test/airflow/venv/lib/python3.11/site-packages/orjson/orjson.cpython-311-darwin.so + +# a patch backup, never source +*.orig diff --git a/crates/by_build/src/lib.rs b/crates/by_build/src/lib.rs index dcc8fb89b8..923b5da622 100644 --- a/crates/by_build/src/lib.rs +++ b/crates/by_build/src/lib.rs @@ -300,6 +300,18 @@ fn finish( // and the same program compiled, so that importing the artefact does not have to // parse it all over again. it is asked for after every rewrite above, because what // gets compiled has to be exactly what would otherwise be run + // a compiled module publishes a real `function` under each of its own function + // names rather than the native object, because a `PyCFunction` is not a + // descriptor and so never receives a receiver when it is installed on a class. + // the forwarders are python, and this is where python gets compiled — so they + // are written into the twin before it is handed to the interpreter + let twin = match by_irbuild::shims::shims(&module, &twin) { + Some(shims) => { + module.shims = Some(shims.install); + format!("{twin}{}", shims.source) + } + None => twin, + }; module.fallback_code = toolchain.and_then(|toolchain| toolchain.marshal(&twin)); module.fallback_source = Some(twin); Ok(module) @@ -509,6 +521,7 @@ mod tests { lines: None, fallback_source: None, fallback_code: None, + shims: None, }; let dir = std::env::temp_dir().join("by_build_refuses_test"); let _ = fs::remove_dir_all(&dir); diff --git a/crates/by_build/tests/differential.rs b/crates/by_build/tests/differential.rs index ab759798c2..f85b2856a3 100644 --- a/crates/by_build/tests/differential.rs +++ b/crates/by_build/tests/differential.rs @@ -212,6 +212,48 @@ def _blamed_module(module, fn, name=_no_name): finally: module.__dict__['__name__'] = saved +# a chain of interpreted frames for a warning to be blamed on, `depth` of them between +# the caller and the call +# +# a warning above the default stack level blames a frame further out than the function +# that wrote it, and the frames out there have to be *these*: one in the module under +# test would be a frame the interpreted leg pushes and the compiled leg does not, which +# is a question about the missing frame rather than about the walk +def _under(depth, fn, *args): + if depth > 0: + return _under(depth - 1, fn, *args) + return fn(*args) + +# a frame `warn` walks past rather than blames +# +# it steps over every frame whose file name holds both 'importlib' and '_bootstrap', so +# a warning raised under the import machinery is blamed on whoever asked for the import. +# the file name is the whole of that rule, so a function compiled under one stands in +# for the loader here +_machinery = {} +exec(compile('def hop(fn, *args):\\n return fn(*args)\\n', + 'importlib/_bootstrap.py', 'exec'), _machinery) +_walked_past = _machinery['hop'] + +# how often a repeated warning is shown, and what the blamed module recorded +# +# the registry that suppresses a repeat belongs to the frame the warning was blamed on, +# so above the default level it is *this* module's rather than the one under test's. a +# lowering that wrote the registry it knows instead of the one it walked to would keep +# printing +def _registry_of_the_blamed(fn, times=3): + globals().pop('__warningregistry__', None) + with warnings.catch_warnings(record=True) as seen: + warnings.resetwarnings() + warnings.simplefilter('default') + for _ in range(times): + fn() + shown = len(seen) + recorded = sorted(key[0] for key in globals().get('__warningregistry__', {}) + if isinstance(key, tuple)) + globals().pop('__warningregistry__', None) + return (shown, recorded) + # the registry carries a version, and changing the filters invalidates it: a warning # already shown is shown again rather than stayed silent about def _registry_after_a_filter_change(module): @@ -2449,7 +2491,7 @@ def raw_ender() -> bytes: #[test] fn the_compiled_build_is_the_one_that_answers_for_a_nul_literal() { // `agree` cannot say which build answered — a declined function answers the same. - // `builtin_function_or_method` is what says the compiled leg is under these calls + // the forwarder's own code object is what says the compiled leg is under these calls let Some((python, toolchain)) = environment() else { return; }; @@ -2474,9 +2516,10 @@ fn the_compiled_build_is_the_one_that_answers_for_a_nul_literal() { &python, &dir, "import by_diff_nulstr_which as m\n\ - print(type(m.whole).__name__, m.length(), m.equals_prefix(), ascii(m.whole()))\n", + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ + print(_leg(m.whole), m.length(), m.equals_prefix(), ascii(m.whole()))\n", ); - assert_eq!(out, "builtin_function_or_method 3 False 'a\\x00b'"); + assert_eq!(out, "native 3 False 'a\\x00b'"); } #[test] @@ -4195,14 +4238,15 @@ class Held: &python, &dir, "import by_diff_pathdecolive as m\n\ - print(type(m.cached).__name__, type(m.cached.__wrapped__).__name__)\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ + print(type(m.cached).__name__, _leg(m.cached.__wrapped__))\n\ print(type(m.Marks.area).__name__, type(m.Marks.sized).__name__,\n\ \x20 type(m.Held.read).__name__)\n\ print(m.cached(4), m.Marks.area.__isabstractmethod__, m.Held.tag)\n", ); assert_eq!( out, - "_lru_cache_wrapper builtin_function_or_method\n\ + "_lru_cache_wrapper native\n\ function method_descriptor method_descriptor\n\ 8 True seen" ); @@ -4361,17 +4405,18 @@ class Rooted: assert_eq!(out, "7 property method_descriptor 3"); } -/// a class whose type slots publish more than its body wrote keeps its decorator's -/// decline +/// a decorator that fills in the comparisons a class left out is handed the gaps it +/// expects /// -/// python reaches `<=` through `tp_richcompare`, one slot behind all six comparisons — -/// so an emitted type that writes `__lt__` publishes `__le__` as well, answering -/// `NotImplemented`. `functools.total_ordering` reads exactly that: it saw `__le__` -/// already there, filled in nothing, and `a <= b` raised where the interpreted class -/// answered `True`. that was a live wrong answer for the plain-name spelling before the -/// path spelling could reach it at all -#[test] -fn a_class_decorator_over_a_partly_filled_slot_declines() { +/// python reaches `<=` through `tp_richcompare`, one slot behind all six comparisons, and +/// publishes a wrapper for every name a filled slot backs — so an emitted type that +/// writes `__lt__` used to publish `__le__` as well. `functools.total_ordering` reads +/// exactly those names: it saw `__le__` already there, filled in nothing, and `a <= b` +/// raised where the interpreted class answered `True`. the class declined its whole +/// compilation to stay out of that. now that the type publishes only what the body wrote, +/// the decorator finds the three gaps, fills them, and the class compiles +#[test] +fn a_class_decorator_over_a_partly_filled_slot_fills_it_in() { let Some((python, toolchain)) = environment() else { return; }; @@ -4409,22 +4454,145 @@ class Ranked: return; } }; - assert!( - built - .declined - .iter() - .any(|declined| declined.reason.contains("publishes `__le__`")), - "declined: {:?}", - built.declined - ); + assert!(built.declined.is_empty(), "declined: {:?}", built.declined); + // `wrapper_descriptor` rather than `function` is the whole point: the class compiled + // and `tp_init` is a real slot, where it used to hand its whole definition back to + // the interpreter let out = run( &python, &dir, "import by_diff_partialslot as m\n\ - print(m.Ranked(1) <= m.Ranked(2), m.Ranked(3) > m.Ranked(2))\n\ + print(m.Ranked(1) <= m.Ranked(2), m.Ranked(3) > m.Ranked(2), m.Ranked(1) != m.Ranked(1))\n\ print(type(m.Ranked.__init__).__name__)\n", ); - assert_eq!(out, "True True\nfunction"); + assert_eq!(out, "True True False\nwrapper_descriptor"); +} + +/// the same decorator applied from *another* module refuses out loud +/// +/// nothing in this module says the class will be decorated, so its type is the sealed one +/// every undecorated class gets, and `total_ordering`'s `setattr` cannot land on it. what +/// matters is *when* that is said. while the type published all six comparisons the +/// decorator found nothing missing, set nothing, raised nothing — and the first `<=` +/// raised instead, a long way from the decoration that caused it. publishing only the +/// body's own names puts the refusal back where the decision is made +#[test] +fn a_class_decorator_applied_from_another_module_refuses_at_decoration() { + let Some((python, toolchain)) = environment() else { + return; + }; + let dir = diff_root().join("by_diff_outsideslot"); + let _ = std::fs::remove_dir_all(&dir); + let source = "\ +class Ordered: + def __init__(self, n: int) -> None: + self.n = n + + def __eq__(self, other: object) -> bool: + return isinstance(other, Ordered) and self.n == other.n + + def __lt__(self, other: object) -> bool: + return self.n < other.n +"; + let built = match build_source( + source, + "by_diff_outsideslot", + &toolchain, + &dir, + &Options { + language: by_irbuild::Language::Python, + ..Options::default() + }, + ) { + Ok(built) => built, + Err(error) => { + assert!(missing_toolchain(&error), "failed to build: {error:#}"); + eprintln!("skipping: no working C toolchain ({error})"); + return; + } + }; + assert!(built.declined.is_empty(), "declined: {:?}", built.declined); + let out = run( + &python, + &dir, + "import functools, by_diff_outsideslot as m\n\ + try:\n\ + \x20 functools.total_ordering(m.Ordered)\n\ + \x20 print('applied')\n\ + except TypeError as error:\n\ + \x20 print(type(error).__name__, 'immutable' in str(error))\n", + ); + assert_eq!(out, "TypeError True"); +} + +/// a class that declines for its decorator takes no other class down with it +/// +/// a written decorator has to be applied at module init rather than where the `class` +/// statement stands, so a class the module body goes on running below declines. that was +/// settled at the end of the class's lowering — by which point it had a layout, and every +/// other class had been lowered against it. `Holder` then declined too, for a layout that +/// was no longer there, and anything holding a `Holder` after it. +/// +/// nothing showed while a decorated class with a half-filled slot group declined earlier, +/// where the layout is decided, and got there first. with that decline gone, `tracemalloc` +/// fell from 50 compiled functions to 26 — 24 of them to this cascade, for a class that +/// was always going to decline +#[test] +fn a_class_declining_for_its_decorator_leaves_no_layout_behind() { + let Some((_, toolchain)) = environment() else { + return; + }; + let dir = diff_root().join("by_diff_decorlayout"); + let _ = std::fs::remove_dir_all(&dir); + // `Holder` names a base, so evaluating its header reaches the module — which is what + // leaves `Held` visible, undecorated, in the window a moved decorator opens + let source = "\ +from collections.abc import Sequence + + +def mark(cls): + return cls + + +@mark +class Held: + def __init__(self, n): + self._n = n + + +class Holder(Sequence): + def __init__(self, ns): + self._ns = ns + + def __len__(self): + return len(self._ns) + + def __getitem__(self, index): + return Held(self._ns[index]) +"; + let built = match build_source( + source, + "by_diff_decorlayout", + &toolchain, + &dir, + &Options { + language: by_irbuild::Language::Python, + ..Options::default() + }, + ) { + Ok(built) => built, + Err(error) => { + assert!(missing_toolchain(&error), "failed to build: {error:#}"); + eprintln!("skipping: no working C toolchain ({error})"); + return; + } + }; + let declined: Vec<_> = built + .declined + .iter() + .map(|declined| declined.name.as_str()) + .collect(); + assert_eq!(declined, ["Held"], "declined: {:?}", built.declined); } #[test] @@ -5349,9 +5517,9 @@ def replaces_itself() -> int: fn a_compiled_frame_is_what_reaches_the_module_namespace() { // the differential tests above compare two legs, and a leg that fell back to its // interpreted definition answers exactly as the interpreted leg does — so they - // cannot say *which* build wrote the global. this one can: a module-level function - // python calls through `PyModule_AddFunctions` is a `builtin_function_or_method`, - // and one that fell back is a `function` + // cannot say *which* build wrote the global. this one can: what a compiled module + // publishes is a forwarder onto the native, and its code object says so where a + // definition that fell back names the module's own file let Some((python, toolchain)) = environment() else { return; }; @@ -5395,12 +5563,13 @@ def init() -> None: &python, &dir, "import by_diff_globalidentity as m\n\ - print(type(m.init).__name__, m.C().x, m.inited)\n", + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ + print(_leg(m.init), m.C().x, m.inited)\n", ); // and `m.inited` read from out here is the module's own binding, which a register // write never touched. before there was an op for it, `C()` inside `init` saw the // old `False` and called `init` again until the stack ran out - assert_eq!(out, "builtin_function_or_method 1 True"); + assert_eq!(out, "native 1 True"); } #[test] @@ -5470,10 +5639,11 @@ def declines_and_reads() -> str: &python, &dir, "import by_diff_globaltwin as m\n\ - print(type(m.writes).__name__, type(m.declines_and_reads).__name__,\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ + print(_leg(m.writes), _leg(m.declines_and_reads),\n\ \x20 m.declines_and_reads(), m.writes(42), m.declines_and_reads(), m.flag)\n", ); - assert_eq!(out, "builtin_function_or_method function 0 42 42 42"); + assert_eq!(out, "native function 0 42 42 42"); } #[test] @@ -5720,19 +5890,26 @@ data class Point: return; } // a declared field is the *layout*: a descriptor on the type, read at an offset, - // never an entry in an instance dict. and `__dict__` itself is refused however the - // instance is built — a mapping naming only what the layout has no room for would - // be an empty answer where the interpreted class gives a full one, which is quiet - // and wrong where the refusal is at least loud + // never an entry in an instance dict. `__dict__` still names it, because python's + // `__dict__` is one mapping over the whole of an object's state — a mapping naming + // only what the layout has no room for would be an empty answer where the + // interpreted class gives a full one, which is quiet and wrong let out = run( &python, &dir, "import by_diff_layout as m\n\ p = m.Point(1, 2)\n\ print(type(vars(m.Point)['x']).__name__)\n\ - print(hasattr(p, '__dict__'))\n", + print(hasattr(p, '__dict__'), vars(p))\n\ + p.extra = 3\n\ + print(vars(p), p.__dict__['x'])\n", + ); + assert_eq!( + out, + "getset_descriptor\n\ + True {'x': 1, 'y': 2}\n\ + {'x': 1, 'y': 2, 'extra': 3} 1" ); - assert_eq!(out, "getset_descriptor\nFalse"); } /// the source both of the instance-dict tests build @@ -6303,6 +6480,213 @@ def took(obj, name): ); } +/// a class keeping its state in a layout, an attribute put on an instance from outside +/// it, and a name the class body binds beside a field its `__init__` writes on only one +/// path — which between them are every kind of entry an instance's `__dict__` can hold +const A_CLASS_WHOSE_DICT_IS_READ: &str = "\ +class Record: + tag = 'none' + + def __init__(self, msg): + self.msg = msg + if msg: + self.tag = 'set' + + def rename(self, msg): + self.msg = msg + + +def render(r): + return '%(msg)s/%(tag)s' % r.__dict__ + + +def state(r): + return sorted(r.__dict__.items(), key=str) +"; + +#[test] +fn an_emitted_instance_agrees_with_its_twin_about_its_dict() { + // an emitted instance keeps its attributes in two places — the class's own in the + // layout and anything put on it afterwards in the dict beside them — so the dict + // alone names the *extra* attributes and none of the real ones. answering `__dict__` + // with it would be an empty mapping where the interpreted class gives a full one, and + // the refusal that stood here instead broke a compiled `logging`: `Formatter.format` + // reads `record.__dict__`, and the read declines to a function that is then handed an + // object with no `__dict__` at all. a decline protects the function, not the class + agree_python_with_declines( + "instdict", + A_CLASS_WHOSE_DICT_IS_READ, + &[ + "m.render(m.Record('hello'))", + "m.state(m.Record('hello'))", + // the class body's value is not the instance's, so `__dict__` does not name it + "m.state(m.Record(''))", + "m.Record('a').__dict__['msg']", + "'msg' in m.Record('a').__dict__", + "'nothing' in m.Record('a').__dict__", + "len(vars(m.Record('a')))", + "sorted(vars(m.Record('a')).keys())", + "sorted(vars(m.Record('a')).values(), key=str)", + "vars(m.Record('a')).get('msg')", + "vars(m.Record('a')).get('nothing', 'fallback')", + "vars(m.Record('a')) == {'msg': 'a', 'tag': 'set'}", + "dict(vars(m.Record('a')))", + "{**vars(m.Record('a'))}", + "repr(vars(m.Record('a')))", + "'{msg}'.format_map(vars(m.Record('a')))", + "[k for k in vars(m.Record('a'))]", + // a name put on the instance from outside the class joins the same mapping + "(lambda r: [setattr(r, 'extra', 3), m.state(r), r.__dict__['extra']])(m.Record('a'))", + // and a write *through* the mapping reaches the layout, which is the whole + // point of it being a view rather than a copy + "(lambda r: [r.__dict__.__setitem__('msg', 'bye'), r.msg, m.state(r)])(m.Record('a'))", + "(lambda r: [r.__dict__.update({'msg': 'z', 'fresh': 1}), r.msg, r.fresh])(m.Record('a'))", + "(lambda r: [r.__dict__.pop('tag'), m.state(r), r.tag])(m.Record('a'))", + "(lambda r: [r.__dict__.setdefault('msg', 'no'), r.__dict__.setdefault('new', 5), r.new])(m.Record('a'))", + "(lambda r: [r.__dict__.__delitem__('tag'), m.state(r)])(m.Record('a'))", + // and replacing the whole mapping replaces the whole of the object's state + "(lambda r: [setattr(r, '__dict__', {'msg': 'fresh', 'other': 2}), m.state(r), r.msg, r.other])(m.Record('a'))", + "vars(m.Record('a')).copy()", + "type(vars(m.Record('a')).copy()).__name__", + // and it has to *be* a dict, not merely read like one. `isinstance(x, dict)` + // gates a great deal of library code, and every reader the C api offers reads + // the base's own storage and ignores an override — a mapping answering out of + // a side table serialises as `{}`, silently + "isinstance(vars(m.Record('a')), dict)", + "__import__('json').dumps(vars(m.Record('a')))", + "vars(m.Record('a')).keys()", + // a write through the mapping has to reach the storage as well as the + // object, or every reader that goes straight to the storage answers with + // what stood there before + "(lambda d: [d.__setitem__('msg', 'bye'), __import__('json').dumps(d)])(vars(m.Record('a')))", + "(lambda r: [r.__dict__.popitem(), m.state(r)])(m.Record('a'))", + "(lambda r: [r.__dict__.__ior__({'msg': 'or'}), r.msg])(m.Record('a'))", + // the type's own name is the one thing left that differs, so the refusal is + // asked for by its kind rather than by its wording + "[type(e).__name__ for e in [_capture(hash, vars(m.Record('a')))]]", + // and a mapping taken and *held* has to see what the object does next. + // answering with what stood there when it was handed out is a wrong answer + // nothing marks — no exception, no missing key, just the old value + "(lambda r: (lambda d: [setattr(r, 'msg', 'three'), d['msg'], d.get('msg'), sorted(d.items(), key=str)])(r.__dict__))(m.Record('two'))", + "(lambda r: (lambda d: [r.rename('three'), d['msg'], sorted(d.items(), key=str)])(r.__dict__))(m.Record('two'))", + "(lambda r: (lambda d: [setattr(r, 'extra', 3), sorted(d.items(), key=str)])(r.__dict__))(m.Record('two'))", + // including through every reader that goes straight to the storage + "(lambda r: (lambda d: [setattr(r, 'msg', 'three'), __import__('json').dumps(d)])(r.__dict__))(m.Record('two'))", + // asked twice it is the same mapping, which is what makes the one somebody + // holds the one the object goes on writing to + "(lambda r: r.__dict__ is r.__dict__)(m.Record('a'))", + "(lambda r: [setattr(r, 'extra', 3), r.__dict__ is vars(r)])(m.Record('a'))", + // a mapping outliving the object it stood for is an ordinary dict holding the + // state that stood at the end, and writing to it reaches nothing + "sorted((lambda r: r.__dict__)(m.Record('two')).items(), key=str)", + "(lambda d: [d.__setitem__('msg', 'after'), sorted(d.items(), key=str)])((lambda r: r.__dict__)(m.Record('two')))", + "(lambda d: [d.update({'late': 1}), sorted(d.items(), key=str)])((lambda r: r.__dict__)(m.Record('two')))", + // and a field the object gives up leaves the mapping with it + "(lambda r: (lambda d: [delattr(r, 'tag'), sorted(d.items(), key=str), 'tag' in d])(r.__dict__))(m.Record('a'))", + ], + ); +} + +#[test] +fn the_class_that_answered_the_dict_is_the_compiled_one() { + // the legs agree whichever class answered, so this is where the compiled one is + // pinned: with the codegen path off, `Record` is the interpreted definition and every + // assertion above passes for the wrong reason + let Some((python, toolchain)) = environment() else { + return; + }; + let dir = diff_root().join("by_diff_instdictlive"); + let _ = std::fs::remove_dir_all(&dir); + if build_source( + A_CLASS_WHOSE_DICT_IS_READ, + "by_diff_instdictlive", + &toolchain, + &dir, + &Options { + language: by_irbuild::Language::Python, + ..Options::default() + }, + ) + .is_err() + { + eprintln!("skipping: no working C toolchain"); + return; + } + let out = run( + &python, + &dir, + "import by_diff_instdictlive as m\n\ + r = m.Record('a')\n\ + print(type(m.Record.__init__).__name__, hasattr(r, '__dict__'))\n\ + print(type(r).__dict__['msg'].__class__.__name__)\n\ + print(m.render(r))\n", + ); + // `msg` reaching python through a `getset_descriptor` is what says the attribute is + // the *layout's* and not an entry in a dict — an interpreted `Record` has no such + // descriptor at all + assert_eq!( + out, + "wrapper_descriptor True\n\ + getset_descriptor\n\ + a/set" + ); +} + +#[test] +fn an_emitted_instance_agrees_with_its_twin_about_its_state() { + // `object.__getstate__` reads the dict word straight out of the instance, and on an + // emitted one that word holds the *extra* attributes and none of the class's own — so + // whatever asks an object for its state was handed half of it, and handed it quietly. + // that is the same hole as the `__dict__` one, through a door a decline never reaches + agree_python_with_declines( + "inststate", + A_CLASS_WHOSE_DICT_IS_READ, + &[ + "m.Record('hello').__getstate__()", + "m.Record('').__getstate__()", + "(lambda r: [setattr(r, 'extra', 3), r.__getstate__()])(m.Record('a'))", + ], + ); +} + +#[test] +fn a_formatter_reading_a_record_dict_agrees() { + // the shape that motivated the view, reduced to the two classes `logging` uses: a + // record whose state is its layout, and a formatter in the same module that reads + // `record.__dict__` off a parameter it knows nothing about. the read declines, so the + // formatter runs interpreted — against the *emitted* record, which is exactly the + // case a decline cannot cover on its own + agree_python_with_declines( + "recorddict", + "\ +class LogRecord: + def __init__(self, name, level, message): + self.name = name + self.levelname = level + self.message = message + + +class Style: + def __init__(self, fmt): + self.fmt = fmt + + def format(self, record): + return self.fmt % record.__dict__ + + +def emit(fmt, name, level, message): + return Style(fmt).format(LogRecord(name, level, message)) +", + &[ + "m.emit('%(levelname)s:%(name)s:%(message)s', 'root', 'WARNING', 'hello')", + "m.emit('%(message)s', 'x', 'INFO', 'quiet')", + // the extra a caller attaches to a record, which `logging` puts there through + // the mapping itself + "(lambda r: [r.__dict__.__setitem__('zz', 1), '%(name)s %(zz)s' % r.__dict__])(m.LogRecord('n', 'W', 'm'))", + ], + ); +} + #[test] fn a_native_class_instance_does_not_leak() { let Some((python, toolchain)) = environment() else { @@ -6427,9 +6811,11 @@ fn a_compiled_function_is_a_c_function_object() { let out = run( &python, &dir, - "import by_diff_cfunc as m\nprint(type(m.f).__name__)\n", + "import by_diff_cfunc as m\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ + print(type(m.f).__name__, _leg(m.f))\n", ); - assert_eq!(out, "builtin_function_or_method"); + assert_eq!(out, "function native"); } #[test] @@ -7369,14 +7755,12 @@ fn the_shadowed_calls_are_answered_by_compiled_bodies() { &python, &dir, "import by_diff_aloneshadowkind as m\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ print(type(m.Alone.double).__name__)\n\ print(type(m.Slotted.double).__name__)\n\ - print(type(m.alone).__name__)\n", - ); - assert_eq!( - out, - "method_descriptor\nmethod_descriptor\nbuiltin_function_or_method" + print(_leg(m.alone))\n", ); + assert_eq!(out, "method_descriptor\nmethod_descriptor\nnative"); } /// a builtin whose entry point wants the defining class, which no call site can supply @@ -7757,6 +8141,40 @@ class Loud(Quiet): ); } +#[test] +fn a_class_a_helper_takes_a_weak_reference_of_is_declined() { + // `logging.Handler.__init__`'s shape: it writes no `weakref.ref` at all, it calls + // `_addHandlerRef(self)` and the reference is taken a whole function away. nothing + // about `__init__`'s own body says it raises, so the refusal has to travel back along + // the call — and if it does not, constructing the class raises `TypeError` rather + // than answering + agree_python_with_declines( + "weakhelper", + "\ +import weakref + +registry = [] + + +def registers(thing): + registry.append(weakref.ref(thing)) + + +def forwards(thing): + registers(thing) + + +class Handler: + def __init__(self): + forwards(self) + + def alive(self): + return registry[-1]() is self +", + &["m.Handler().alive()", "len(m.registry)"], + ); +} + #[test] fn a_basedpython_default_that_is_not_an_immediate_is_re_evaluated_at_each_call() { // basedpython has no mutable-default gotcha: `mutable_defaults` rewrites such a @@ -9129,15 +9547,13 @@ async def finishing(v: object) -> object: &dir, "import asyncio\n\ import by_diff_sendslot_pin as m\n\ - print(type(m.counting).__name__)\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ + print(_leg(m.counting))\n\ print(type(m.counting(0)).__name__)\n\ print(asyncio.run(m.finishing((1, 2))))\n", ); // a declined function would be a plain `function` and its state a `generator` - assert_eq!( - out, "builtin_function_or_method\ncounting$gen\n(1, 2)", - "{out}" - ); + assert_eq!(out, "native\ncounting$gen\n(1, 2)", "{out}"); } /// a `StopIteration` the body *raised* leaves the frame as a `RuntimeError`, and one @@ -12287,16 +12703,17 @@ fn only_the_class_no_spec_can_build_is_left_interpreted() { &python, &dir, "import by_diff_perclassheld_t as m\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ print(type(m.Held.__dict__['read']).__name__,\n\ \x20 type(m.Kept.__dict__['note']).__name__,\n\ \x20 type(m.Deeper.__dict__['down']).__name__,\n\ - \x20 type(m.add).__name__)\n\ + \x20 _leg(m.add))\n\ # a spec has no code object to write one from, so its absence is the emitted type\n\ print('__firstlineno__' in vars(m.Held), '__firstlineno__' in vars(m.Kept))\n", ); assert_eq!( out, - "function method_descriptor method_descriptor builtin_function_or_method\n\ + "function method_descriptor method_descriptor native\n\ True False" ); } @@ -14863,9 +15280,9 @@ fn a_class_attribute_naming_a_module_function_keeps_the_definition_that_binds() // `PyCFunction` in a class dict is not a descriptor, so `Reducer().dump()` would call // `_dump` with no `self` at all. // - // `function` against `builtin_function_or_method` is the whole assertion. the class - // answers the same *value* either way, so nothing but the type of what sits in the - // slot says which definition is standing there + // `function` against the forwarder is the whole assertion. the class answers the + // same *value* either way, so nothing but what sits in the slot says which + // definition is standing there let Some((python, toolchain)) = environment() else { return; }; @@ -14904,8 +15321,9 @@ class Reducer: &python, &dir, "import by_diff_fntwinbind as m\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ print(m.Reducer().dump(), m.Reducer().kind())\n\ - print(type(m._dump).__name__, type(m.Reducer.__dict__['dump']).__name__)\n\ + print(_leg(m._dump), type(m.Reducer.__dict__['dump']).__name__)\n\ print(type(m.Reducer.kind).__name__)\n", ); // `method_descriptor` says the emitted type answered rather than a class that fell @@ -14913,7 +15331,7 @@ class Reducer: assert_eq!( out, "dumped reducer\n\ - builtin_function_or_method function\n\ + native function\n\ method_descriptor" ); } @@ -14979,8 +15397,9 @@ Option.__ge__ = lambda self, other: True &python, &dir, "import by_diff_fntwindunder as m\n\ + _leg = lambda f: 'native' if f.__code__.co_filename == '' else type(f).__name__\n\ print(repr(m.Option()), m.Option().kind())\n\ - print(type(m._repr).__name__, type(m.Option.__dict__['__repr__']).__name__)\n\ + print(_leg(m._repr), type(m.Option.__dict__['__repr__']).__name__)\n\ print(type(m.Option.kind).__name__)\n", ); // `function` on the last line is the class itself confirming it stayed interpreted, @@ -14988,7 +15407,7 @@ Option.__ge__ = lambda self, other: True assert_eq!( out, "