From 48ea6cc7074a9bf15c297b2593c6061cd27374de Mon Sep 17 00:00:00 2001 From: KotlinIsland <65446343+kotlinisland@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:09:52 +1000 Subject: [PATCH] compile to a cpython extension module: the declines, the wrong answers, and a sixth rung MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the native backend's correctness pass. every change here was measured against a build of the commit before it, and the numbers are in scratch.tasks.md. - an artefact built for one interpreter refuses to run under another, rather than segfaulting inside a type construction - code that never runs proves nothing: ty types unreachable code as `Never`, which is assignable to everything, so it took the first representation going - a method's decorator ran twice, because the class body had already applied it - a parameter's register covers what its own body writes, not just its default - a default is a written type, so the interpreted twin enforces it too - a package body whose package cannot be named is declined, not renamed - a union taken apart keeps knowing it was defined in terms of itself - a finished frame writes its return down, so `am_send` answers without an exception: `coro` 1.07x -> 1.77x against cpython - `isoinstance.sh`, a sixth sweep rung, reads members off the instance it built — the first rung to look past construction, and five modules differ only there --- crates/by_build/src/lib.rs | 29 +- crates/by_build/src/toolchain.rs | 112 +- crates/by_build/tests/differential.rs | 541 +++++- crates/by_build/tests/end_to_end.rs | 326 +++- crates/by_codegen_c/src/lib.rs | 368 +++- crates/by_ir/src/function.rs | 35 + crates/by_ir/src/print.rs | 1 + crates/by_ir/src/verify.rs | 2 + crates/by_irbuild/src/lib.rs | 111 +- crates/by_irbuild/src/mapper.rs | 30 +- crates/by_irbuild/src/tests.rs | 114 ++ crates/by_opt/src/coalesce.rs | 1 + crates/by_opt/src/copy_propagation.rs | 1 + crates/by_opt/src/dead_registers.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/unswitch.rs | 1 + crates/by_rt/include/by.h | 311 +++- crates/by_rt/src/lib.rs | 1 + .../src/transforms/lazy_import.rs | 10 +- .../by_transforms/src/transforms/soundness.rs | 60 +- crates/by_transforms/src/type_info.rs | 22 +- crates/ty/src/by_commands.rs | 42 +- crates/ty/tests/by_e2e.rs | 132 ++ .../mdtest/basedpython_sound_types.md | 482 +++++- .../resources/mdtest/cycle.md | 19 + crates/ty_python_semantic/src/types.rs | 4 +- .../src/types/inferred_signature.rs | 1492 +++++++++++++++-- .../src/types/set_theoretic/builder.rs | 33 + .../ty_python_semantic/src/types/soundness.rs | 31 + .../development/compilation/index.md | 25 + .../development/compilation/runtime.md | 26 + .../development/compilation/technology.md | 29 + docs/basedpython/features/sound-types.md | 137 +- scripts/bg.sh | 33 +- scripts/native-sweeps/isoconstruct.sh | 4 +- scripts/native-sweeps/isoimport.sh | 2 +- scripts/native-sweeps/isoinstance.sh | 418 +++++ scripts/native-sweeps/isosubclass.sh | 4 +- scripts/native-sweeps/sweeplib.sh | 176 ++ 44 files changed, 4822 insertions(+), 351 deletions(-) create mode 100755 scripts/native-sweeps/isoinstance.sh diff --git a/crates/by_build/src/lib.rs b/crates/by_build/src/lib.rs index 8af73006c0..7b7f680ace 100644 --- a/crates/by_build/src/lib.rs +++ b/crates/by_build/src/lib.rs @@ -39,7 +39,7 @@ pub fn build_source( out_dir: &Path, options: &Options, ) -> Result { - let module = lower(source, module_name, options, toolchain.version)?; + let module = lower(source, module_name, options, Some(toolchain))?; let mut artifact = build_module(&module, toolchain, out_dir)?; artifact.annotation = write_annotation(&module, out_dir, options)?; Ok(Built { @@ -61,7 +61,7 @@ pub fn build_lowered( out_dir: &Path, options: &Options, ) -> Result { - let module = finish(module, source, options, toolchain.version)?; + let module = finish(module, source, options, Some(toolchain))?; let mut artifact = build_module(&module, toolchain, out_dir)?; artifact.annotation = write_annotation(&module, out_dir, options)?; Ok(Built { @@ -71,14 +71,18 @@ pub fn build_lowered( } /// as [`build_lowered`], but writing only the generated C +/// +/// the toolchain is still wanted, and for the same reason a real build wants it: it +/// is the interpreted twin's compiled form that a caller passing `None` gives up, +/// and the C then written is not the C a build would have written pub fn emit_lowered( module: ModuleIr, source: &str, + toolchain: Option<&Toolchain>, out_dir: &Path, options: &Options, - version: Option<(u8, u8)>, ) -> Result { - let module = finish(module, source, options, version)?; + let module = finish(module, source, options, toolchain)?; emit_verified(&module, out_dir, options) } @@ -89,11 +93,11 @@ pub fn emit_lowered( pub fn emit_source( source: &str, module_name: impl Into, + toolchain: Option<&Toolchain>, out_dir: &Path, options: &Options, - version: Option<(u8, u8)>, ) -> Result { - let module = lower(source, module_name, options, version)?; + let module = lower(source, module_name, options, toolchain)?; emit_verified(&module, out_dir, options) } @@ -206,13 +210,13 @@ fn lower( source: &str, module_name: impl Into, options: &Options, - version: Option<(u8, u8)>, + toolchain: Option<&Toolchain>, ) -> Result { finish( by_irbuild::module_from_source(source, module_name, options.language), source, options, - version, + toolchain, ) } @@ -224,7 +228,7 @@ fn finish( mut module: by_ir::function::ModuleIr, source: &str, options: &Options, - version: Option<(u8, u8)>, + toolchain: Option<&Toolchain>, ) -> Result { // the generated C points back at the `.by` it came from, so a compiler warning // or a debugger lands on source somebody wrote. a caller that knows the real @@ -280,7 +284,7 @@ fn finish( source.to_string() } else { let mut config = options.fallback.clone().unwrap_or_default(); - if let Some((major, minor)) = version + if let Some((major, minor)) = toolchain.and_then(|toolchain| toolchain.version) && let Ok(parsed) = format!("{major}.{minor}").parse() { config.min_version = parsed; @@ -293,6 +297,10 @@ fn finish( // too, over the twin's — once for each definition rather than once for the name let twin = by_irbuild::without_init_decorators(&twin, &module) .map_err(|error| anyhow::anyhow!("could not prepare the interpreted fallback: {error}"))?; + // 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 + module.fallback_code = toolchain.and_then(|toolchain| toolchain.marshal(&twin)); module.fallback_source = Some(twin); Ok(module) } @@ -500,6 +508,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let dir = std::env::temp_dir().join("by_build_refuses_test"); let _ = fs::remove_dir_all(&dir); diff --git a/crates/by_build/src/toolchain.rs b/crates/by_build/src/toolchain.rs index ea51242ed4..9d389397f7 100644 --- a/crates/by_build/src/toolchain.rs +++ b/crates/by_build/src/toolchain.rs @@ -5,11 +5,13 @@ //! compiler, the flags, and the include and library paths used to build it, and //! those are exactly the ones an extension has to match. +use std::io::Write; use std::path::PathBuf; -use std::process::Command; +use std::process::{Command, Stdio}; use anyhow::{Context, Result, bail}; use by_ir::ModuleName; +use by_ir::function::FallbackCode; use serde::Deserialize; /// everything needed to compile and link an extension for one interpreter @@ -83,6 +85,29 @@ print(json.dumps({ })) "; +/// compile a module body and hand back the marshalled code object +/// +/// this runs in the *target* interpreter for the same reason the probe above does: +/// a code object is only readable by the interpreter that wrote it, and this is the +/// one that will read it. the source arrives on stdin because a module body is +/// routinely a hundred kilobytes, which is past what an argument list will take on +/// some platforms +/// +/// `` is the filename, which is what `PyRun_String` calls a module body — so +/// a traceback out of the interpreted twin says exactly what it said before +const MARSHAL: &str = r" +import importlib.util, marshal, sys + +level = sys.flags.optimize +source = sys.stdin.buffer.read().decode('utf-8') +blob = marshal.dumps(compile(source, '', 'exec', dont_inherit=True, optimize=level)) +magic = int.from_bytes(importlib.util.MAGIC_NUMBER, 'little') +out = sys.stdout.buffer +out.write(('%d %d %d\n' % (magic, level, len(blob))).encode('ascii')) +out.write(blob) +out.flush() +"; + /// the probe's answers, exactly as the interpreter reported them /// /// every field defaults, because an interpreter that cannot answer one of these is @@ -181,6 +206,69 @@ impl Toolchain { pub fn extension_path(&self, module: &ModuleName) -> PathBuf { module.relative_path(&self.ext_suffix) } + + /// compile a module body in this interpreter, for the artefact to carry + /// + /// the answer is a cache and nothing depends on having it, so every way this can + /// fail — no such interpreter, a body it will not compile, an answer we cannot + /// read — reads as `None` and leaves the artefact running the source. that is + /// what it does today, so the worst outcome is the speed we already have + pub fn marshal(&self, source: &str) -> Option { + // the emitted C has to be a function of the source and nothing else — that is + // what lets a rebuild skip the C compiler, which is by far its slowest step. the + // one thing in a code object that could vary between two runs of one interpreter + // is a `set` or `frozenset` constant, which `x in {"a", "b"}` compiles to: it + // holds strings, whose hashes are seeded per process. cpython 3.13 and 3.14 both + // write such a constant in a fixed order regardless, so pinning the seed changes + // nothing measurable today — it is here so that this does not *depend* on their + // doing so. it cannot change the program either way: the set is rebuilt under the + // reading interpreter's own hashing + let mut child = Command::new(&self.python) + .args(["-c", MARSHAL]) + .env("PYTHONHASHSEED", "0") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .ok()?; + // the child is waited for whatever happens to the write, because a `Child` that + // is merely dropped is never reaped — and a whole-project build runs one of these + // per module + let written = child + .stdin + .take() + .is_some_and(|mut stdin| stdin.write_all(source.as_bytes()).is_ok()); + let output = child.wait_with_output().ok()?; + if !written || !output.status.success() { + return None; + } + parse_marshal(&output.stdout) + } +} + +/// read what [`MARSHAL`] wrote: one ascii header line, then the bytes it counted +fn parse_marshal(output: &[u8]) -> Option { + let split = output.iter().position(|byte| *byte == b'\n')?; + let header = std::str::from_utf8(&output[..split]).ok()?; + let mut fields = header.split(' '); + let magic: i64 = fields.next()?.parse().ok()?; + let optimize: i32 = fields.next()?.parse().ok()?; + let length: usize = fields.next()?.parse().ok()?; + if fields.next().is_some() { + return None; + } + let marshalled = output.get(split + 1..)?; + // a short read means the interpreter was interrupted partway through writing, and + // a long one means something else wrote to its stdout — either way these are not + // the bytes it counted, and half a code object is worse than none + if marshalled.len() != length || length == 0 { + return None; + } + Some(FallbackCode { + marshalled: marshalled.into(), + magic, + optimize, + }) } #[cfg(test)] @@ -294,6 +382,28 @@ mod tests { ); } + #[test] + fn a_marshal_result_parses_into_a_code_object_and_its_guards() { + // the payload is binary and may hold anything, newlines included — only the + // *first* one ends the header, and the count says where the rest stops + let code = parse_marshal(b"168627699 0 5\n\xc3\n\x00\xffz").unwrap(); + assert_eq!(code.magic, 168_627_699); + assert_eq!(code.optimize, 0); + assert_eq!(&*code.marshalled, b"\xc3\n\x00\xffz"); + } + + #[test] + fn a_marshal_result_that_does_not_match_its_own_count_is_refused() { + // a code object is worthless in halves, and an interpreter that printed + // something of its own before ours has not left us the bytes it counted + assert!(parse_marshal(b"168627699 0 9\n\xc3\xc3").is_none()); + assert!(parse_marshal(b"168627699 0 1\n\xc3\xc3").is_none()); + assert!(parse_marshal(b"168627699 0 0\n").is_none()); + assert!(parse_marshal(b"168627699 0\n\xc3").is_none()); + assert!(parse_marshal(b"168627699 0 1 4\n\xc3").is_none()); + assert!(parse_marshal(b"no header at all").is_none()); + } + #[test] fn position_independence_is_asked_for_only_where_it_is_a_choice() { assert_eq!( diff --git a/crates/by_build/tests/differential.rs b/crates/by_build/tests/differential.rs index 138a9c46b4..24a8685bb8 100644 --- a/crates/by_build/tests/differential.rs +++ b/crates/by_build/tests/differential.rs @@ -9,6 +9,15 @@ //! it is the strongest test in the suite, because it needs no expected values of //! its own — cpython supplies them. //! +//! the property is over the programs `by check` accepts, which is why every call +//! written here is one it does. a compiled function checks its arguments at the +//! boundary and an interpreted one does not, so a call the checker rejects can be +//! answered by the twin and refused by the extension — and cpython is then +//! supplying the behaviour of a program ty had already said was wrong, which is +//! not an expected value for anything. the opt-in `parameters` soundness gate is +//! what lets the twin enforce the same contract; see +//! docs/basedpython/development/compilation/index.md. +//! //! see docs/basedpython/development/compilation/plan.md#differential-testing #![expect( @@ -2498,6 +2507,112 @@ alias = one ); } +/// the source both method-decorator tests below compile +/// +/// `mark` hands back what it was given, so the binding is right however many times it ran +/// and `seen` is the one thing that can show a second run. `double` is the other half: it +/// wraps, so it says which application ended up installed +const MARKED_METHODS: &str = "\ +seen = [] + + +def mark(f): + seen.append(f.__name__) + return f + + +def double(f): + def wrapper(self) -> int: + return f(self) * 2 + return wrapper + + +class C: + @mark + def g(self) -> int: + return 1 + + @double + def doubled(self) -> int: + return 3 + + def calls_doubled(self) -> int: + return self.doubled() + + def plain(self) -> int: + return 2 +"; + +/// a method's decorator is evaluated once, and what it did on the way happened once +/// +/// a method's decorators run *inside* the class body, so the interpreted twin already +/// applied them before anything of the module was installed. module init then applied them +/// a second time to the native method: the value installed was right — the second +/// application is the one that wins — which is exactly what made it silent. `seen` read +/// `['g', 'g']` where python reads `['g']`, so a decorator that registers registered +/// twice +#[test] +fn a_method_decorator_runs_once() { + agree_python( + "methoddecoratoronce", + MARKED_METHODS, + &[ + "m.C().g()", + "m.C().doubled()", + // through another method too: an internal call must reach the same + // decorated method an external one does + "m.C().calls_doubled()", + "m.seen", + ], + ); +} + +/// and the decorated method is the *interpreted* one, which is the price of the above +/// +/// a decorator is handed whatever the class body gave it, and there is no way to hand it +/// the native method without calling it a second time. so a decorated method is no longer +/// native on the type — while an undecorated one is untouched, which is where a compiled +/// class's speed lives. `type(...)` is what can tell the two apart: a compiled type holds +/// a `method_descriptor` where an interpreted class holds a plain function +#[test] +fn a_decorated_method_is_the_interpreted_one_and_its_siblings_are_not() { + let Some((python, toolchain)) = environment() else { + return; + }; + let dir = std::env::temp_dir().join("by_diff_methoddecoratoronce_t"); + let _ = std::fs::remove_dir_all(&dir); + let built = match build_source( + MARKED_METHODS, + "by_diff_methoddecoratoronce_t", + &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 by_diff_methoddecoratoronce_t as m\n\ + print(type(m.C.__dict__['g']).__name__,\n\ + \x20 type(m.C.__dict__['doubled']).__name__,\n\ + \x20 type(m.C.__dict__['plain']).__name__)\n", + ); + // the two decorated methods are what their own decorators returned — `mark` hands + // back the interpreted function it was given, `double` hands back its wrapper — + // and the untouched sibling is still the compiled type's own + assert_eq!(out, "function function method_descriptor"); +} + /// the source both class-decorator tests below compile /// /// `mark` hands back what it was given and records only the name, so the binding is right @@ -2540,7 +2655,6 @@ fn a_class_decorator_runs_once() { } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_decorated_class_is_the_compiled_type() { // the counts above are answered identically by a class that fell back to its // interpreted definition, so they cannot say which build answered. @@ -2620,7 +2734,6 @@ TABLE = [Held] /// evaluates, so the module never holds the undecorated definition and the decorator can /// still move to init. without the future import this is `TABLE = [Held]` again #[test] -#[ignore = "native compilation is being fixed separately"] fn a_decorated_class_named_in_a_deferred_annotation_still_compiles() { let Some((python, toolchain)) = environment() else { return; @@ -2684,7 +2797,6 @@ def through(h: Held) -> int: /// which is what makes evaluating it at module init mean what it meant where the `def` /// stood #[test] -#[ignore = "native compilation is being fixed separately"] fn a_decorator_written_as_a_path_agrees() { agree_python( "pathdeco", @@ -2747,9 +2859,14 @@ def probe() -> int: /// the compiled leg answers the path-decorated definitions /// /// the differential legs agree whichever one answered, so a decorator test passes with -/// the codegen path switched off. this is where it is pinned +/// the codegen path switched off. this is where it is pinned. +/// +/// a *decorated method* is deliberately not one of the things `type` can pin any more: it +/// is the interpreted definition on purpose, because a decorator is handed whatever the +/// class body gave it and applying it again to the native method would run it twice. so +/// the class is pinned by its undecorated sibling, which is still the compiled type's own +/// `method_descriptor`, and the decorator by the effect it had #[test] -#[ignore = "native compilation is being fixed separately"] fn a_path_decorated_definition_is_the_compiled_one() { let Some((python, toolchain)) = environment() else { return; @@ -2778,6 +2895,9 @@ class Marks: def area(self) -> int: return 3 + def sized(self) -> int: + return 4 + @Wrappers.tag class Held: @@ -2810,13 +2930,14 @@ class Held: &dir, "import by_diff_pathdecolive as m\n\ print(type(m.cached).__name__, type(m.cached.__wrapped__).__name__)\n\ - print(type(m.Marks.area).__name__, type(m.Held.read).__name__)\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\ - method method_descriptor\n\ + function method_descriptor method_descriptor\n\ 8 True seen" ); } @@ -3076,7 +3197,6 @@ def other(x: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_construction_of_a_decorated_class_from_the_same_module_agrees() { // a construction is written against the *name*, and a class decorator is what the // name then holds. allocating the emitted layout instead skipped the decorator: @@ -3113,7 +3233,6 @@ def probe(x: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_method_modifier_is_not_looked_up_as_a_name() { // a modifier reaches the ast as a decorator with no `@`. it was emitted as a name // to look up in the module namespace at init, and there is no such name — so the @@ -3143,7 +3262,6 @@ def probe() -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_static_or_class_method_answers_the_same_through_the_class_and_through_an_instance() { // slot zero used to be forced to the receiver for every method, and these two say // it is not one — so the compiled `Box.make(3)` bound `3` to a `Box` and raised @@ -3204,7 +3322,6 @@ def probe() -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_static_or_class_method_is_the_compiled_one() { // neither `agree` can say which build answered — a class that declines answers // identically out of its interpreted definition. `type(C.__dict__['m'])` cannot @@ -3299,7 +3416,6 @@ class Sized(collections.abc.Sized): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_static_method_the_boundary_hands_over_reaches_the_plain_function() { // a boundary that cannot establish a parameter hands the whole call to the // interpreted twin, and for a *method* that twin is taken off the class with the @@ -3449,7 +3565,6 @@ def bump(n: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_global_a_frame_assigns_is_read_back_in_that_same_frame() { // the other half, and the one a write alone can get wrong in the opposite // direction: if the write reaches the namespace while a later read in the same @@ -3641,7 +3756,6 @@ def resets(): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_module_level_name_a_global_rebinds_is_found_through_the_namespace() { // taking the write opened this: a frame can now rebind a name the module *defined*, // and a call written against that name was reaching the definition directly. so the @@ -3687,7 +3801,6 @@ def replaces_itself() -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] 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 @@ -5773,7 +5886,6 @@ async def forwards(n: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_await_protocol_agrees() { // an `await` reaches its object through `__await__` and drives what comes back // by sending into it. both halves have edges worth pinning: what counts as @@ -5865,6 +5977,216 @@ def redelegated(v: object) -> object: ); } +/// what a resumable frame's `return` is worth, read through both faces of it +/// +/// a compiled frame reports finishing by writing the value into its state object and +/// handing back nothing, because the slot python prefers to ask with — `am_send` — +/// can then say what the frame returned without an exception ever being built. the +/// iterator protocol still owes python a `StopIteration`, so the other face builds +/// one, and the two have to agree about every shape a return can take. +/// +/// the shapes that break a careless implementation are the ones a `StopIteration` +/// would read as an *argument list*: a tuple spreads across it, an empty one leaves +/// nothing, a one-tuple collapses, and an exception instance is raised in place of +/// the error asked for. a subclass carrying its own `value` is the other half, since +/// what a delegation collects is the field rather than the attribute +#[test] +fn a_return_agrees_between_the_raise_and_the_send_slot() { + agree( + "sendslot", + "\ +def returning(v: object) -> object: + yield 1 + return v + +def straight(v: object) -> object: + return v + yield + +def relayed(v: object) -> object: + got = yield from returning(v) + yield got + +async def finishing(v: object) -> object: + return v + +async def relaying(v: object) -> object: + got = await finishing(v) + return got + +def counting(n: int) -> object: + i = 0 + while i < n: + yield i + i = i + 1 + +def silent() -> object: + yield 1 + return + +def relayed_silent(again: int) -> object: + inner = silent() + got = yield from inner + yield got + i = 0 + while i < again: + yield (yield from inner) + i = i + 1 + +async def nothing() -> None: + return + +async def relaying_nothing() -> object: + got = await nothing() + return got + +def echoing(n: int) -> object: + i = 0 + while i < n: + got = yield i + yield got + i = i + 1 + return 'end' + +def relaying_sends(n: int) -> object: + got = yield from echoing(n) + yield got + +def failing() -> object: + yield 1 + raise ValueError('inner') + +def relayed_failing() -> object: + got = yield from failing() + yield got + +def guarding(log: list[str]) -> object: + try: + yield 1 + yield 2 + except ValueError: + log.append('caught') + yield 3 + finally: + log.append('left') +", + &[ + // the raising face, which is the only one a python caller can reach + "[_value(m.returning(v)) for v in \ + ((1, 2), (), (1,), 5, None, [1, 2], 'ab', StopIteration(9), _Sub(3), _Shadowed(7))]", + // and the exception it builds is shaped the way python shapes one + "[(lambda e: (repr(e), e.args, e.value))(_capture(next, m.straight(v))) \ + for v in ((1, 2), (), (1,), 5, None, StopIteration(9), _Sub(3), _Shadowed(7))]", + "[_value(m.straight(v)) for v in ((1, 2), (), (1,), 5, None, _Shadowed(7))]", + // the send slot, reached three ways: a compiled `yield from` over a + // compiled generator, a compiled `await` of a compiled coroutine, and + // asyncio driving one from the outside + "[list(m.relayed(v)) for v in \ + ((1, 2), (), (1,), 5, None, [1, 2], 'ab', StopIteration(9), _Sub(3), _Shadowed(7))]", + "[_run(m.relaying(v)) for v in \ + ((1, 2), (), (1,), 5, None, [1, 2], StopIteration(9), _Sub(3), _Shadowed(7))]", + "[_run(m.finishing(v)) for v in \ + ((1, 2), (), (1,), 5, None, [1, 2], StopIteration(9), _Sub(3), _Shadowed(7))]", + // a frame that finishes without naming a value still finishes: the slot + // does not carry that one structurally, so this is the path that asks + // cpython what a raised `StopIteration` was worth — including the second + // time round, when the frame is already exhausted + "list(m.relayed_silent(0))", + "list(m.relayed_silent(2))", + "_run(m.relaying_nothing())", + // a value sent into a delegation reaches the inner frame through the same + // slot, and has to arrive there rather than at the frame that delegated + "_sent(m.relaying_sends(2), (7, 8, 9, 10, 11))", + "_sent(m.relaying_sends(2), ('a', None, 'b', None))", + // and an exception that is not a finish is not a return value + "_capture(list, m.relayed_failing())", + "[(lambda e: (type(e).__name__, str(e)))(_capture(list, m.relayed_failing()))]", + // a frame that finished stays finished, whichever face asked + "[(g := m.returning(4), _value(g), type(_capture(next, g)).__name__)[1:]]", + "[(g := m.returning(4), list(g), list(m.relayed(4)))[1:]]", + // `close` on a suspended frame and on a finished one are both clean, and + // both leave it exhausted + "[(g := m.counting(3), next(g), g.close(), type(_capture(next, g)).__name__)[3:]]", + "[(g := m.counting(1), list(g), g.close(), type(_capture(next, g)).__name__)[2:]]", + "[(g := m.returning(9), _value(g), g.close(), type(_capture(next, g)).__name__)[2:]]", + // `throw` resumes at the suspension: one the body catches carries on, one + // it does not comes out — and either way the cleanup runs exactly once + "[(log := [], g := m.guarding(log), next(g), g.throw(ValueError('x')), \ + type(_capture(next, g)).__name__, log)[3:]]", + "[(log := [], g := m.guarding(log), next(g), \ + type(_capture(g.throw, KeyError('k'))).__name__, \ + type(_capture(next, g)).__name__, log)[3:]]", + ], + ); +} + +/// the send slot is answered by the compiled state object, and its return arrives +/// without an exception +/// +/// `agree` cannot see this on its own: the slot is deliberately invisible from +/// python — the same values come back whether it is there or not, which is what +/// makes it safe to add — so a run of the differential tests above would pass with +/// every part of it switched off. what pins it is the emitted C, plus a build that +/// refuses to decline, so the answers really are the compiled frame's +#[test] +fn a_compiled_state_object_answers_the_send_slot() { + let Some((python, toolchain)) = environment() else { + return; + }; + let dir = std::env::temp_dir().join("by_diff_sendslot_pin"); + let _ = std::fs::remove_dir_all(&dir); + let source = "\ +def counting(n: int) -> object: + i = 0 + while i < n: + yield i + i = i + 1 + return 'end' + +async def finishing(v: object) -> object: + return v +"; + let options = Options { + require_native: true, + ..Options::default() + }; + if build_source(source, "by_diff_sendslot_pin", &toolchain, &dir, &options).is_err() { + eprintln!("skipping: no working C toolchain"); + return; + } + let emitted = std::fs::read_to_string(dir.join("by_diff_sendslot_pin.c")) + .expect("the generated C is written beside the extension"); + // both surfaces publish the slot, and both report their `return` into the state + // object rather than by raising + assert_eq!(emitted.matches(".am_send =").count(), 2, "{emitted}"); + assert_eq!(emitted.matches("By_SendGenerator(").count(), 2, "{emitted}"); + assert_eq!( + emitted.matches("->by_returned = by_t;").count(), + 2, + "{emitted}" + ); + // and nothing is left raising a `StopIteration` with a value from inside a frame + assert!( + !emitted.contains("By_RaiseWith(PyExc_StopIteration"), + "{emitted}" + ); + + let out = run( + &python, + &dir, + "import asyncio\n\ + import by_diff_sendslot_pin as m\n\ + print(type(m.counting).__name__)\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}" + ); +} + #[test] fn a_value_live_across_a_suspension_agrees() { // python evaluates left to right, so `total + await step(i)` has the read of @@ -6443,7 +6765,6 @@ data class Point: } #[test] -#[ignore = "native compilation is being fixed separately"] fn variadic_parameters_agree() { agree_with_declines( "variadic", @@ -6606,7 +6927,6 @@ data class Point: /// a spec out and its construction falls back to the interpreted definition, which /// already carries them #[test] -#[ignore = "native compilation is being fixed separately"] fn a_mutating_method_decorator_agrees() { agree_python( "mutatingdeco", @@ -7064,7 +7384,6 @@ def looped(n: int) -> float: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_from_import_inside_a_body_agrees() { // plain python, so the interpreted leg is the source itself: the transpiler's // lazy-import polyfill resolves `from pkg import submodule` wrongly, which would @@ -7281,7 +7600,6 @@ def counters() -> list[object]: } #[test] -#[ignore = "native compilation is being fixed separately"] fn dunder_methods_fill_their_type_slots() { // a method table cannot fill a slot: `repr(x)` reads `tp_repr` and never looks // the name up. so each of these has to work *both* ways — through the slot and @@ -7323,7 +7641,6 @@ class Bag: } #[test] -#[ignore = "native compilation is being fixed separately"] fn calling_a_resumable_frame_hands_back_the_state_object() { // whatever the annotation says the body produces, the *call* gives the state // object and the iteration or the `await` turns it back. there are three @@ -7398,7 +7715,6 @@ def drained(n: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_class_of_only_methods_compiles() { // no `__init__` is not the same as no *layout*: a class of methods has an empty // one, which is as representable as any other. `object.__init__` is what rejects a @@ -7602,6 +7918,71 @@ def numeric(x, scale=2.0): ); } +#[test] +fn a_parameter_its_own_body_rebinds_covers_both_representations() { + // an unannotated parameter is declared by its default, so `safe='/'` makes the + // register a `str` — and then the body puts bytes there. the store narrowed the + // object back to a `str` with a check, and the check raised on a call the + // interpreter answers: `urllib.parse.quote_from_bytes(b'a b')` was `'a%20b'` + // interpreted and `TypeError: expected str, got bytes` compiled + agree_python( + "reboundparam", + "\ +def quoted(bs, safe='/'): + if isinstance(safe, str): + safe = safe.encode('ascii', 'ignore') + return repr(bs) + repr(safe) + + +def stepped(n, step=1): + step = str(step) + return step * n + + +def flagged(x, on=False): + if x: + on = 'yes' + return repr(on) +", + &[ + "m.quoted(b'a b')", + // the boundary unboxed to the default's representation too, so a caller + // supplying the *other* one was refused before the body was reached + "m.quoted(b'a b', b'/')", + "m.stepped(2)", + "m.stepped(2, 3)", + "m.flagged(1)", + "m.flagged(0)", + "m.flagged(0, 'no')", + ], + ); +} + +#[test] +fn a_walrus_and_an_exception_handler_are_writes_a_register_has_to_cover_too() { + // the two binding forms an assignment statement does not cover. a walrus hides + // inside an expression and a handler's name is on the `try` rather than in its + // body, so neither was counted when a register's representation was decided — + // and both then stored through a check that refused the value they had just bound + agree_python( + "walrushandler", + "\ +def walrused(safe='/'): + if (safe := safe.encode('ascii')): + return repr(safe) + return 'empty' + + +def caught(tag='t'): + try: + raise ValueError('boom') + except ValueError as tag: + return repr(tag) +", + &["m.walrused()", "m.walrused('ab')", "m.caught()"], + ); +} + #[test] fn a_gradual_value_narrowed_on_both_arms_is_still_an_object() { // narrowing a gradual value gives an *intersection* holding it, so a conditional @@ -7897,7 +8278,6 @@ fn a_subclass_that_appends_nothing_past_a_base_agrees() { } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_subclass_that_appends_nothing_is_the_compiled_type() { // the behaviour above is answered identically by a class that fell back to its // interpreted definition, so it cannot say which build answered. @@ -8317,7 +8697,6 @@ def widened(value: object) -> str: /// exactly that up (`sys.modules.get(cls.__module__).__dict__`), so a package /// member with a dataclass in it failed to import at all #[test] -#[ignore = "native compilation is being fixed separately"] fn a_class_in_a_package_reports_the_package_it_came_from() { let Some((python, toolchain)) = environment() else { return; @@ -8535,7 +8914,6 @@ class Keyed(metaclass=ABCMeta): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_metaclass_that_remakes_a_class_level_constant_is_turned_down_after_the_call() { // the constants go into the namespace the metaclass is handed, which is enough for a // metaclass that only *reads* one. an `EnumType` does not read them: it builds a @@ -8618,7 +8996,6 @@ FIRST = Boundary.STRICT } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_dunder_the_module_body_hangs_on_a_class_keeps_it_off_the_compiled_surface() { // `ctypes` writes `c_byte.__ctype_le__ = c_byte.__ctype_be__ = c_byte` under the class // statement, and the adoption that carries a twin's attributes across leaves every @@ -8717,7 +9094,6 @@ Untouched.plain = 4 } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_constant_that_reads_back_differently_every_time_is_not_turned_down_for_it() { // a class-level constant is read out of the *mapping* the class body wrote rather // than through a lookup on the class, and `__class_getitem__ = classmethod(f)` is @@ -8793,7 +9169,6 @@ class Spec: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_metaclass_that_raises_on_the_namespace_it_is_handed_leaves_the_import_standing() { // `ssl.Purpose`'s shape, and the one thing worse than a wrong answer: `EnumType` is // handed a namespace whose members are the twin's *finished* ones and tries to build a @@ -9022,7 +9397,6 @@ class Open(ABC): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_class_constant_naming_another_class_reaches_the_metaclass_namespace_remapped() { // the value a constant carries comes off the twin, so `pair = Other` in a class body // hands over the *interpreted* `Other` — a class nothing else in the module can @@ -9173,7 +9547,6 @@ HIDDEN = {name: globals().pop(name) for name in (\"Gone\",)} } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_annotated_class_attribute_reaches_the_compiled_type() { // the statement was skipped in both the layout pass and the constant pass, so an // annotated class attribute was lost outright: `Tagged.KIND` raised where python @@ -9264,7 +9637,6 @@ class OnExternal(Exception): } #[test] -#[ignore = "native compilation is being fixed separately"] fn what_the_module_body_gives_a_class_after_its_statement_agrees() { // the interpreted definition runs first and the whole module body runs against it, // so a class the body keeps mutating is mutated *there* — and the compiled type that @@ -9324,7 +9696,6 @@ Text.marker = 7 } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_late_gift_that_could_hand_the_interpreted_class_back_is_left_alone() { // carrying an attribute across is only sound where the value provably cannot answer // with the interpreted definition, which is about to stop being the class under its @@ -9419,7 +9790,6 @@ Ordered.__ge__ = lambda self, right: True } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_class_answers_the_annotations_its_body_wrote() { // `__annotations__` is read through a getset on the metatype, which refuses outright // for a type that is not a heap type and otherwise reads the class's *own* dict — @@ -9465,7 +9835,6 @@ class Sub(Written): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_deferred_annotation_is_carried_as_the_string_the_body_wrote() { // `from __future__ import annotations` makes every annotation the text of itself, so // the mapping is one of strings and a name in it need never resolve — `Later` is @@ -9493,7 +9862,6 @@ class Later: } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_annotation_that_could_hand_the_interpreted_class_back_is_refused() { // an annotation is subject to the rule every carried attribute is: a value that *is* // an interpreted twin becomes the type standing in for it, and one that can still @@ -9657,7 +10025,6 @@ class Fine: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_class_that_keeps_a_dunder_of_its_own_stays_a_static_type() { // an emitted class has no instance dict, so an attribute of the *instance* is a // descriptor in the type's dict — and the type machinery reads a heap type's @@ -9924,7 +10291,6 @@ _pair() } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_private_name_in_a_class_body_is_mangled() { // python binds an identifier of two leading underscores written in `class C` as // `_C__spam`, whatever it names. the compiler read the written name, so the compiled @@ -10050,7 +10416,6 @@ class Tagged: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_decorated_class_carries_the_body_its_own_decorator_was_handed() { // a class-level constant is copied off the body the interpreted `class` statement // wrote, taken while that statement runs and before any of the class's decorators is @@ -10130,7 +10495,6 @@ class Holder: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_class_body_capture_reaches_only_this_module_s_own_body() { // the capture is a copy of the builtins mapping, carrying a `__build_class__` of ours, // put in this module's dict for the length of the fallback run — so no other module and @@ -10364,7 +10728,6 @@ def stays() -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_finalizer_over_fields_answers_for_a_construction_that_raised() { // `Held()` raises before `self.path` is written, and python releases the half-built // object — which runs `__del__` over fields that are still the zeroes `tp_alloc` @@ -10814,7 +11177,6 @@ class B(A): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_zero_argument_super_names_the_class_the_class_statement_made() { // python's `__class__` is a cell holding the class object, not a name lookup — // and a class decorator replaces the *namespace* entry, so the two are different @@ -11112,7 +11474,6 @@ class LocalShadow(Base): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_field_named_after_a_c_keyword_agrees() { // python has no reserved attribute names, C has forty-odd reserved words, and the // struct member took the attribute name verbatim — so `self.int = 1` emitted @@ -11153,7 +11514,6 @@ class Holder: } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_attribute_a_path_may_skip_agrees() { // python has no fixed layout, so an `if` with no `else` simply leaves the attribute // off that instance and a read raises. a compiled class keeps its layout and carries @@ -11389,7 +11749,6 @@ def shared_cell(a: object, n: int) -> object: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_method_default_that_is_not_a_literal_agrees() { // a default that is not an immediate is evaluated once, at definition time — the // interpreted twin already did that and holds the one object every call must share. @@ -11425,7 +11784,6 @@ class Joiner: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_numeric_slots_agree() { // every binary numeric dunder fills a `nb_*` slot, and python never looks one up by // name — so a class defining `__or__` without an adapter simply had no `|`. only @@ -11484,7 +11842,6 @@ def in_place(a: int, b: int) -> object: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_dunder_python_looks_up_by_name_agrees() { // a dunder is special to the emitter only when python reads it out of a *type // slot*: `repr(x)` reads `tp_repr` and never consults the name, so that one needs an @@ -11946,7 +12303,6 @@ def both(n: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_attribute_assigned_on_every_path_earns_a_field() { // a field is always present, where python raises `AttributeError` for one never // written — so the layout may hold what *every* path through `__init__` fills, @@ -12055,7 +12411,6 @@ def guarded(flag: bool, n: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_attribute_a_path_may_skip_is_declined() { // an `if` with no `else` leaves a path that assigns nothing, and a struct field // has no way to be absent — so this stays interpreted and keeps raising @@ -12223,7 +12578,6 @@ async def echoed(n: int) -> Any: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_generator_or_coroutine_may_be_a_method() { // the state object holds `self` like any other parameter, so the body reads // fields through it exactly as a plain method does. the state *class* is @@ -12275,7 +12629,6 @@ class Other: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_compiled_class_may_be_a_context_manager_or_an_async_iterator() { // `__enter__` and its three relatives are not slots: python reaches them by an // ordinary type lookup, which finds the method table without an adapter @@ -12439,7 +12792,6 @@ async def pair(manager: Any, log: list[str]) -> str: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_in_place_arithmetic_slots_agree() { // `a += b` rebinds `a` to whatever the method returned, so returning `self` and // returning a fresh object are both correct and both have to work — the identity @@ -12538,7 +12890,6 @@ def numeric(n: int, f: float) -> object: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_unary_and_call_slots_agree() { // a slot cannot be filled from the method table, so each of these is installed // twice — and `tp_call` is the one that has to *bind*, because it is handed a @@ -12594,7 +12945,6 @@ class Adder: } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_arity_error_counts_the_receiver() { // python describes the *function*, whose first parameter is `self`, rather than // the call the caller wrote — so a method's counts are one higher than its @@ -12628,7 +12978,6 @@ def free(x: int, step: int = 1) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_arity_error_is_worded_by_the_interpreter() { // python's arity wording has rules a reimplementation keeps getting one short of: // `and` from two names up, a comma *before* that `and` from three up, a range @@ -12955,7 +13304,6 @@ def grown(n: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_conjunction_pattern_agrees() { // basedpython's `case P and Q:` — every one has to match the *same* subject. // it is the mirror of `P | Q` and needs no restriction on what the alternatives @@ -13168,7 +13516,6 @@ class Pair[A, B]: } #[test] -#[ignore = "native compilation is being fixed separately"] fn mapping_patterns_agree() { // `case {}:` matches *any* mapping rather than an empty one — a mapping // pattern names the keys it cares about and ignores the rest, which is the @@ -13288,7 +13635,6 @@ def mixed(v: object) -> str: } #[test] -#[ignore = "native compilation is being fixed separately"] fn sequence_and_class_patterns_agree() { // a sequence pattern matches what the interpreter's own `MATCH_SEQUENCE` // matches — a type *flagged* as a sequence, which `str`, `bytes` and @@ -13511,7 +13857,6 @@ def caller() -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_constructors_computed_default_agrees() { // the constructor is a boundary of its own — `tp_init` binds from a tuple and a // dict rather than from a vector — and marking such a parameter *required* there @@ -13553,7 +13898,6 @@ class Grower: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_constructor_that_defers_is_still_the_compiled_type() { // `agree` cannot tell which build answered: a class that fell back to its // interpreted definition agrees with itself. `wrapper_descriptor` is what says @@ -13600,7 +13944,6 @@ class Holder: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_positional_only_constructor_still_fills_its_instance() { // a `/` moves the receiver into the positional-only list, and reading slot zero // off the first *ordinary* parameter instead found no attribute assignment at @@ -13631,7 +13974,6 @@ class Pair: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_variadic_constructor_agrees() { // the constructor slot bound `*args` and `**kwargs` as if they were ordinary named // parameters: a call that supplied neither was an arity error against a definition @@ -13672,7 +14014,6 @@ class Slot: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_positional_only_parameter_is_unreachable_by_name_from_either_boundary() { // `posonly` counts the receiver, and a boundary handed that receiver separately // has to shift it — leaving it unshifted made the parameter *after* the marker @@ -13707,7 +14048,6 @@ def free(a, /, b): } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_call_a_boundary_binds_nothing_from_is_rejected_in_pythons_wording() { // a boundary with no named parameters used to phrase its own refusal rather than // going through the binding, and one with only a `*args` did not refuse at all — @@ -13733,7 +14073,6 @@ def only_var(*args): } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_arithmetic_dunders_reach_both_directions() { // python hands `nb_add` its operands in the order they were written whichever // type it asked, so the adapter works out which side is ours before it knows @@ -13786,7 +14125,6 @@ class Vec: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_power_dunder_carries_the_modulus_through_its_slot() { // `nb_power` is the one *ternary* numeric slot: `pow(a, b, m)` passes the modulus // through it, and python spells `a ** b` as a `None` modulus. so the adapter has @@ -13845,7 +14183,6 @@ class Binary: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_power_dunder_is_answered_by_the_compiled_type() { // `wrapper_descriptor` says the slot itself is filled: `PyType_Ready` builds one // only for a slot that is, and it shadows the method table entry @@ -13893,7 +14230,6 @@ class Mod: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_containment_operator_agrees() { // `in` is the container's own protocol rather than a comparison: `__contains__` // where the type has one, and a scan of the iterator otherwise. so it reads its @@ -13956,7 +14292,6 @@ def counted(xs: list[int], wanted: list[int]) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_container_dunders_fill_their_slots() { // `len(g)`, `g[i]`, `g[i] = v`, `v in g` and iteration all read *slots* — the // method table is never consulted for any of them. `__getitem__` and @@ -14019,7 +14354,6 @@ def mutated(items: list[int], at: int, to: int) -> list[int]: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_assignment_slot_carries_both_of_its_methods() { // `mp_ass_subscript` is one slot for `__setitem__` and `__delitem__` — a NULL // value is the delete — so a class with only one of them still has to fill it, @@ -14072,7 +14406,6 @@ def dropped(items: list[int], at: int) -> list[int]: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_numeric_conversion_dunders_fill_their_slots() { // `int()`, `float()` and every use of `__index__` read `tp_as_number`, never // the method table @@ -14112,7 +14445,6 @@ class Loose: } #[test] -#[ignore = "native compilation is being fixed separately"] fn the_comparison_dunders_share_one_slot() { // python does not look these up by name: it calls `tp_richcompare` with an // opcode. so all six are one function, one the class does not define answers @@ -14178,7 +14510,6 @@ class Hashed: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_complex_conversion_is_found_by_name() { // `PyNumberMethods` has `nb_int`, `nb_float` and `nb_index` and no complex field // at all, so `complex(x)` looks the name up on the type — which is exactly what @@ -14212,7 +14543,6 @@ class Loose: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_complex_conversion_is_answered_by_the_compiled_type() { // `method_descriptor` is what says the compiled type answered: a class that fell // back to its interpreted definition answers `complex(x)` identically @@ -14258,7 +14588,6 @@ class Cell: } #[test] -#[ignore = "native compilation is being fixed separately"] fn an_await_method_fills_the_async_slot() { // `await x` reads `am_await` out of the async sub-table and never consults the // name, so without the slot the class is simply not awaitable — a `TypeError` @@ -14325,7 +14654,6 @@ class Answer: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_del_method_fills_the_finalizer_slot() { // `tp_finalize` is not reached by name and not reached by itself: `tp_dealloc` // has to call it, and a class that writes its own dealloc — which every compiled @@ -14415,7 +14743,6 @@ class Angry: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_getattr_hook_stands_behind_the_ordinary_lookup() { // `__getattr__` fills `tp_getattro`, which *replaces* attribute lookup — so the // adapter has to run the ordinary one first and reach the method only where that @@ -14487,7 +14814,6 @@ class Proxy: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_descriptor_get_fills_its_slot() { // an attribute lookup that finds a descriptor reads `tp_descr_get`, so a class // whose `__get__` only reaches the method table is not a descriptor at all: the @@ -14549,7 +14875,6 @@ class Doubler: } #[test] -#[ignore = "native compilation is being fixed separately"] fn append_takes_the_list_fast_path_only_when_it_is_one() { // the fast path skips the attribute lookup, so anything that overrides or // merely *has* an `append` must still reach its own — a subclass, a type of @@ -14588,7 +14913,6 @@ def onto(target: object, value: object) -> object: } #[test] -#[ignore = "native compilation is being fixed separately"] fn constructing_a_class_this_module_emits_agrees() { // the direct path allocates and calls the class's own `__init__` rather than // resolving the name through the module namespace. everything the interpreted @@ -14646,7 +14970,6 @@ def build(n: int) -> int: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_plain_class_agrees() { // an ordinary python class: no marker decorator, a hand-written `__init__`, // and the layout is whatever that constructor assigns @@ -15296,7 +15619,6 @@ def deep(n: float) -> str: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_subclass_that_writes_no_init_runs_the_base_one() { // a subclass's fields *are* its base's, so reading "has fields" as "has something // to initialize" synthesized a constructor taking one argument per inherited @@ -15470,7 +15792,6 @@ def linked(a, b): "; #[test] -#[ignore = "native compilation is being fixed separately"] fn the_attributes_a_slots_declaration_names_agree() { agree_python( "slots", @@ -15516,7 +15837,6 @@ fn the_attributes_a_slots_declaration_names_agree() { } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_slots_declaration_is_storage_the_emitted_type_owns() { // `agree` cannot say which build answered — a class that fell back to its interpreted // definition agrees with itself, and this one would have the very descriptors the @@ -15675,7 +15995,6 @@ class Widget: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_dataclass_decorator_builds_a_constructor_that_runs() { // `@dataclass` does not read a class so much as *generate* from it — an `__init__` // taking one argument per annotation and assigning one attribute each — so it is the @@ -15724,7 +16043,6 @@ def make(n: int) -> str: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_decorated_class_is_the_compiled_one_and_its_dict_is_collectable() { // the differential legs agree whichever class answered, so this is where the // compiled one is pinned down. it is not a formality: the managed dict this class @@ -15967,7 +16285,6 @@ def strings(d: dict[str, str]) -> str: } #[test] -#[ignore = "native compilation is being fixed separately"] fn more_than_one_with_item_agrees() { agree_with_declines( "withitems", @@ -16131,7 +16448,6 @@ def bad_merge(n: int) -> str: } #[test] -#[ignore = "native compilation is being fixed separately"] fn splatting_at_a_call_agrees() { agree( "splatcall", @@ -16177,7 +16493,6 @@ def through(f: object, xs: list[int]) -> object: } #[test] -#[ignore = "native compilation is being fixed separately"] fn keyword_only_and_positional_only_parameters_agree() { agree( "paramkinds", @@ -17051,7 +17366,6 @@ def carried(n: int) -> object: } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_module_level_name_bound_to_a_class_follows_the_class_it_named() { // the whole module body runs against the interpreted definitions, so a name it binds // to a class holds that definition — and the compiled type only ever replaces the one @@ -17117,7 +17431,6 @@ Rebound = Bare } #[test] -#[ignore = "native compilation is being fixed separately"] fn a_class_constant_naming_another_class_is_the_type_that_replaced_it() { // a class-level constant is taken off the interpreted definition, so `attr = C` in a // body hands over the *twin* — and copying that verbatim gave the compiled type an @@ -17359,3 +17672,51 @@ class OnLaid(Alias, codecs.Codec): function function" ); } + +#[test] +fn a_statement_after_a_return_is_dropped_rather_than_declining_the_function() { + // ty types every expression in unreachable code as `Never`, and `Never` is + // assignable to everything — so the first test in `map_type` won, which was + // `None`, a representation with no width at all. a dead statement whose value is + // unboxed then had nowhere to put it and declined the whole function: `textwrap + // .dedent` and both of `quopri`'s entry points ran interpreted over code that + // never runs + // + // `agree_python` rather than `agree_python_with_declines`: the point is that + // nothing here declines, so a decline must fail the test rather than be tolerated + agree_python( + "deadcode", + "\ +def after_return(line: str) -> int: + return len(line) + b = not line + + +def after_return_compare(a: int) -> int: + return a + 1 + c = a < 3 + + +def under_a_false_guard(a: int) -> int: + if 0: + d = not a + return a * 2 + + +def after_raise(a: int) -> int: + raise ValueError('no') + e = a > 0 + + +def live_negation(a: int) -> bool: + return not a +", + &[ + "m.after_return('abc')", + "m.after_return_compare(4)", + "m.under_a_false_guard(21)", + "str(_capture(m.after_raise, 1))", + "[m.live_negation(v) for v in (0, 1)]", + ], + ); +} diff --git a/crates/by_build/tests/end_to_end.rs b/crates/by_build/tests/end_to_end.rs index f304da365f..931b3aecdc 100644 --- a/crates/by_build/tests/end_to_end.rs +++ b/crates/by_build/tests/end_to_end.rs @@ -18,7 +18,7 @@ use std::process::Command; use by_build::{Options, Toolchain, build_module, build_source}; use by_ir::builder::FunctionBuilder; -use by_ir::function::{CallConvention, ModuleIr, ModuleName}; +use by_ir::function::{CallConvention, FallbackCode, ModuleIr, ModuleName}; use by_ir::ops::{BinOp, CmpOp, Op, Terminator, Value}; use by_ir::rtype::RType; @@ -122,6 +122,7 @@ fn arith_module() -> ModuleIr { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } @@ -186,6 +187,7 @@ fn fib_module() -> ModuleIr { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } @@ -322,6 +324,7 @@ fn division_floors_like_python_and_raises_on_zero() { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let Some(dir) = built(&module, &toolchain, "divzero") else { return; @@ -378,6 +381,7 @@ fn floats_are_unboxed_and_exclude_int() { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let Some(dir) = built(&module, &toolchain, "float") else { return; @@ -449,6 +453,7 @@ fn calls_between_compiled_functions_stay_native() { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let Some(dir) = built(&module, &toolchain, "call") else { return; @@ -568,9 +573,9 @@ def kept(a: str, b: str) -> object: return (joined, a) ", "by_e2e_append", + None, &dir, &Options::default(), - None, ) .expect("the module emits"); let emitted = std::fs::read_to_string(&built.artifact.source).expect("the C is readable"); @@ -1101,9 +1106,9 @@ fn the_emitted_c_names_no_pointer_type_it_does_not_mean() { let built = by_build::emit_source( POINTER_SOURCE, "by_e2e_pointers", + None, &dir, &Options::default(), - None, ) .expect("the module emits"); @@ -1223,3 +1228,318 @@ fn a_package_is_built_as_a_tree_and_imports_under_its_dotted_names() { assert!(fields[4].ends_with(&toolchain.ext_suffix), "{printed}"); } } + +/// a module that is nothing but its interpreted twin +/// +/// which of the two forms of that twin an import runs is what the tests below are +/// about, and a compiled function beside it would only be noise +fn twin_module(name: &str, source: &str, code: Option) -> ModuleIr { + let mut module = ModuleIr::new(name); + module.fallback_source = Some(source.to_string()); + module.fallback_code = code; + module +} + +/// the twin as source and the same twin compiled, saying *different* things +/// +/// nothing else can tell the two apart. an artefact whose code object silently will +/// not read falls back to its source and behaves identically — so a test that gave +/// both forms the same program would pass with the whole compiled path disabled, +/// and the only thing lost would be an import speed nobody asserts on +const TWIN_SOURCE: &str = "WHICH = \"source\"\n"; +const TWIN_CODE: &str = "WHICH = \"code\"\n"; + +/// what an import of a `twin_module` answered, or `None` where it did not import +fn which_twin_ran(python: &str, dir: &Path, name: &str) -> Option { + let printed = script( + python, + dir, + &format!( + "try:\n\ + \x20 import {name}\n\ + except BaseException as error:\n\ + \x20 print('!' + type(error).__name__)\n\ + else:\n\ + \x20 print({name}.WHICH)\n" + ), + ); + if printed.starts_with('!') { + return None; + } + Some(printed) +} + +#[test] +fn the_compiled_twin_is_what_an_import_runs() { + let Some((python, toolchain)) = environment() else { + return; + }; + let Some(code) = toolchain.marshal(TWIN_CODE) else { + eprintln!("skipping: the interpreter would not compile the twin"); + return; + }; + let module = twin_module("by_e2e_twincode", TWIN_SOURCE, Some(code)); + let Some(dir) = built(&module, &toolchain, "twincode") else { + return; + }; + assert_eq!( + which_twin_ran(&python, &dir, "by_e2e_twincode").as_deref(), + Some("code") + ); +} + +#[test] +fn a_twin_compiled_by_another_interpreter_is_left_where_it_is() { + // marshal promises nothing across versions, and it does not fail softly either: + // handing cpython 3.14 a code object 3.13 wrote segfaults the process outright. + // the bytecode magic is cpython's own answer to that — it is what makes an + // upgraded interpreter regenerate a `.pyc` rather than misread one — and one that + // does not match has to send the import back to the source + let Some((python, toolchain)) = environment() else { + return; + }; + let Some(mut code) = toolchain.marshal(TWIN_CODE) else { + eprintln!("skipping: the interpreter would not compile the twin"); + return; + }; + code.magic += 1; + let module = twin_module("by_e2e_twinmagic", TWIN_SOURCE, Some(code)); + let Some(dir) = built(&module, &toolchain, "twinmagic") else { + return; + }; + assert_eq!( + which_twin_ran(&python, &dir, "by_e2e_twinmagic").as_deref(), + Some("source") + ); +} + +#[test] +fn a_twin_compiled_at_another_optimization_level_is_left_where_it_is() { + // the level is part of what the source compiles *to*, not a setting beside it: + // `-O` takes `assert` out of the bytecode and `-OO` takes docstrings too. this + // test process runs at level 0, so a code object claiming any other level is one + // this interpreter would not have produced + let Some((python, toolchain)) = environment() else { + return; + }; + let Some(mut code) = toolchain.marshal(TWIN_CODE) else { + eprintln!("skipping: the interpreter would not compile the twin"); + return; + }; + code.optimize = 2; + let module = twin_module("by_e2e_twinoptimize", TWIN_SOURCE, Some(code)); + let Some(dir) = built(&module, &toolchain, "twinoptimize") else { + return; + }; + assert_eq!( + which_twin_ran(&python, &dir, "by_e2e_twinoptimize").as_deref(), + Some("source") + ); +} + +#[test] +fn a_twin_this_interpreter_should_read_and_cannot_fails_the_import() { + // the two guards above are mismatches, and a mismatch is ordinary: the source is + // compiled instead and nothing is wrong. a code object that says it *is* for this + // interpreter and then will not read is a broken artefact, and falling back to the + // source there would leave a defect in how these bytes are written costing nothing + // more visible than an import nobody times + let Some((python, toolchain)) = environment() else { + return; + }; + let Some(mut code) = toolchain.marshal(TWIN_CODE) else { + eprintln!("skipping: the interpreter would not compile the twin"); + return; + }; + // no marshal type is written as a NUL, so this is refused before anything is built + // out of it — a *truncated* code object would be read into one and is not a safe + // thing to hand an interpreter + code.marshalled = vec![0u8; 8].into(); + let module = twin_module("by_e2e_twinunreadable", TWIN_SOURCE, Some(code)); + let Some(dir) = built(&module, &toolchain, "twinunreadable") else { + return; + }; + assert_eq!(which_twin_ran(&python, &dir, "by_e2e_twinunreadable"), None); +} + +#[test] +fn a_twin_that_reads_back_as_something_other_than_code_fails_the_import() { + let Some((python, toolchain)) = environment() else { + return; + }; + let Some(mut code) = toolchain.marshal(TWIN_CODE) else { + eprintln!("skipping: the interpreter would not compile the twin"); + return; + }; + // marshal's `TYPE_INT`: the tag, then the value in four little-endian bytes. it + // reads back perfectly well and is not a code object, which is the case a bare + // "did it read" test would hand straight to the evaluator + code.marshalled = vec![b'i', 42, 0, 0, 0].into(); + let module = twin_module("by_e2e_twinnotcode", TWIN_SOURCE, Some(code)); + let Some(dir) = built(&module, &toolchain, "twinnotcode") else { + return; + }; + assert_eq!(which_twin_ran(&python, &dir, "by_e2e_twinnotcode"), None); +} + +#[test] +fn running_the_interpreter_with_o_still_means_o_for_the_twin() { + // the whole-artefact statement of the level guard. the twin has always been + // compiled by the importing interpreter, so `python -O` took its `assert` + // statements out; a code object compiled at the build's own level would quietly + // put them back. the module *body* is where this is visible, because that is the + // part of a compiled module that always runs interpreted + let Some((python, toolchain)) = environment() else { + return; + }; + let dir = std::env::temp_dir().join("by_e2e_twinoptrun"); + let _ = std::fs::remove_dir_all(&dir); + let source = "\ +try: + assert False, \"still here\" +except AssertionError: + ASSERTED = True +else: + ASSERTED = False +"; + let Ok(_) = build_source( + source, + "by_e2e_twinoptrun", + &toolchain, + &dir, + &Options { + language: by_irbuild::Language::Python, + ..Options::default() + }, + ) else { + eprintln!("skipping: no working C toolchain"); + return; + }; + let asserted = |flags: &[&str]| { + let mut command = Command::new(&python); + command.args(flags).args([ + "-c", + &format!( + "import sys\nsys.path.insert(0, {:?})\n\ + import by_e2e_twinoptrun as m\nprint(m.ASSERTED)\n", + dir.display().to_string() + ), + ]); + let out = command.output().expect("the interpreter runs"); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + assert_eq!(asserted(&[]), "True"); + assert_eq!(asserted(&["-O"]), "False"); +} + +#[test] +fn compiling_the_twin_twice_gives_the_same_bytes() { + // the emitted C has to be a function of the source alone, or a rebuild recompiles a + // module nothing about which changed — and the C compiler is the slowest step there + // is. a `frozenset` of strings is the one constant whose written form could turn on + // something outside the source, because string hashes are seeded per process, so it + // is what this asks about + let Some((_, toolchain)) = environment() else { + return; + }; + let source = + "PICKED = \"m\" in {\"a\", \"quite\", \"long\", \"spread\", \"of\", \"words\", \"m\"}\n"; + let (Some(first), Some(second)) = (toolchain.marshal(source), toolchain.marshal(source)) else { + eprintln!("skipping: the interpreter would not compile the twin"); + return; + }; + assert_eq!(first, second); +} + +/// the header's version branches are decided by the headers the build compiled against, +/// so an artefact loaded by another minor version runs branches for a layout that +/// interpreter does not have — a crash rather than a wrong answer. the running version is +/// read out of `Py_GetVersion`'s banner, which is prose with two numbers on the front, so +/// what that reading does with a real banner and with junk is worth executing rather than +/// reasoning about +#[test] +fn the_running_version_is_read_off_the_banner() { + let Some((python, toolchain)) = environment() else { + return; + }; + let dir = std::env::temp_dir().join("by_e2e_version"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("the build directory is made"); + std::fs::write(dir.join(by_rt::BY_H_NAME), by_rt::BY_H).expect("the header is written"); + + // banners cpython has really printed, then the shapes a reading could get wrong: a + // major with no minor, a minor that is not a number, an empty string + let source = r#" +#include "by.h" + +static PyObject *parse(PyObject *self, PyObject *text) { + int major, minor; + (void)self; + By_ParseVersion(PyUnicode_AsUTF8(text), &major, &minor); + return Py_BuildValue("(ii)", major, minor); +} + +static PyObject *running(PyObject *self, PyObject *unused) { + int major, minor; + (void)self; + (void)unused; + By_ParseVersion(Py_GetVersion(), &major, &minor); + return Py_BuildValue("(ii)", major, minor); +} + +static PyMethodDef methods[] = {{"parse", parse, METH_O, NULL}, + {"running", running, METH_NOARGS, NULL}, + {NULL, NULL, 0, NULL}}; +static struct PyModuleDef def = {PyModuleDef_HEAD_INIT, "by_e2e_version", NULL, -1, + methods, NULL, NULL, NULL, NULL}; +PyMODINIT_FUNC PyInit_by_e2e_version(void) { return PyModule_Create(&def); } +"#; + let c = dir.join("by_e2e_version.c"); + std::fs::write(&c, source).expect("the probe is written"); + let output = dir.join(format!("by_e2e_version{}", toolchain.ext_suffix)); + let args = by_build::compile_command(&toolchain, &c, &output, &dir); + let (program, rest) = args + .split_first() + .expect("the compiler command is not empty"); + let compiled = Command::new(program) + .args(rest) + .output() + .expect("the compiler runs"); + if !compiled.status.success() { + eprintln!( + "skipping: no working C toolchain\n{}", + String::from_utf8_lossy(&compiled.stderr) + ); + return; + } + + let answers = script( + &python, + &dir, + "import sys, by_e2e_version as m\n\ + for text in ['3.14.0a1 (main, x) [Clang]', '3.9.7 (default, y)', '3.13.0', '3',\n\ + '3.x.1', '', '.13', 'python 3.13']:\n\ + \x20 print(m.parse(text))\n\ + print(m.running())\n\ + print((sys.version_info[0], sys.version_info[1]))\n", + ); + let mut lines = answers.lines(); + for expected in [ + "(3, 14)", "(3, 9)", "(3, 13)", + // a major alone names no minor, so it names no interpreter + "(-1, -1)", "(-1, -1)", "(-1, -1)", "(-1, -1)", "(-1, -1)", + ] { + assert_eq!(lines.next(), Some(expected), "in:\n{answers}"); + } + // and the reading of a live banner is the interpreter's own answer about itself + let running = lines.next().expect("the running version is printed"); + assert_eq!( + running, + lines.next().expect("`sys.version_info` is printed") + ); +} diff --git a/crates/by_codegen_c/src/lib.rs b/crates/by_codegen_c/src/lib.rs index 8427a3a1de..f6f19b2966 100644 --- a/crates/by_codegen_c/src/lib.rs +++ b/crates/by_codegen_c/src/lib.rs @@ -79,6 +79,24 @@ fn field_decl<'a>( .and_then(|candidate| candidate.fields.iter().find(|decl| decl.name == field)) } +/// whether `function` is the method a resumable class steps through +/// +/// its `return` is not an ordinary one: it is the end of a generator or a coroutine, +/// and how it is reported is what the send slot and the iterator protocol disagree +/// about +fn resumes(module: &ModuleIr, function: &Function) -> bool { + let Some(owner) = &function.owner else { + return false; + }; + module.classes.iter().any(|class| { + &class.name == owner + && class + .resume + .as_ref() + .is_some_and(|resume| resume.method == function.name) + }) +} + fn mangle_member(name: &str) -> String { by_ir::function::FieldDecl { name: name.to_string(), @@ -492,6 +510,14 @@ fn emit_class_struct(module: &ModuleIr, class: &ClassIr) -> String { "typedef struct {} {{\n{header}", class.struct_name(module.name.dotted()) ); + // where a `return` puts its value. a resumable frame reports finishing by writing + // here and handing back nothing, so that the slot python asks with — `am_send` — + // can say what the frame returned without an exception ever being built. it is not + // one of the frontend's fields because no python code can name it and nothing + // parks across a suspension in it: it is written once, on the way out + if class.resume.is_some() { + out.push_str(" PyObject *by_returned;\n"); + } for field in &class.fields { let _ = writeln!(out, " {} {};", ctype(module, &field.ty), field.member()); // `tp_alloc` zeroes the instance, so "never written" is the state an object @@ -545,6 +571,11 @@ fn emit_class_type(module: &ModuleIr, class: &ClassIr) -> String { if keeps_a_dict { out.push_str(" By_ClearManagedDict((PyObject *)self);\n"); } + // a return nobody asked for still owns its value: a generator dropped between + // its frame finishing and the finish being read leaves one here + if class.resume.is_some() { + out.push_str(" Py_XDECREF(self->by_returned);\n"); + } for field in &class.fields { if let Some(release) = dec_ref(&field.ty, &format!("self->{}", field.member())) { let _ = writeln!(out, " {release}"); @@ -1024,7 +1055,7 @@ fn emit_class_members(module: &ModuleIr, class: &ClassIr) -> String { \x20 PyObject *by_old = self->{};\n\ \x20 self->{} = By_NewRef(args[0]);\n\ \x20 Py_XDECREF(by_old);\n\ - \x20 return By_StepGenerator(selfobj, &self->{state},\n\ + \x20 return By_StepGenerator(selfobj, &self->by_returned, &self->{state},\n\ \x20 (PyObject *(*)(PyObject *)){symbol});\n}}", mangle_member(crate::GENERATOR_SENT), mangle_member(crate::GENERATOR_SENT), @@ -1037,7 +1068,8 @@ fn emit_class_members(module: &ModuleIr, class: &ClassIr) -> String { "static PyObject *{type_name}_close(PyObject *selfobj, PyObject *const *args, Py_ssize_t nargs) {{\n\ \x20 (void)args; (void)nargs;\n\ \x20 {struct_name} *self = ({struct_name} *)selfobj;\n\ - \x20 int by_r = By_CloseGenerator(selfobj, &self->{}, &self->{state},\n\ + \x20 int by_r = By_CloseGenerator(selfobj, &self->{}, &self->by_returned,\n\ + \x20 &self->{state},\n\ \x20 (PyObject *(*)(PyObject *)){symbol});\n\ \x20 By_FinishGenerator(&self->{state});\n\ \x20 if (by_r < 0) return NULL;\n\ @@ -1055,6 +1087,7 @@ fn emit_class_members(module: &ModuleIr, class: &ClassIr) -> String { \x20 return NULL;\n\ \x20 }}\n\ \x20 return By_ThrowInto(selfobj, &(({struct_name} *)selfobj)->{},\n\ + \x20 &(({struct_name} *)selfobj)->by_returned,\n\ \x20 &(({struct_name} *)selfobj)->{state}, args[0],\n\ \x20 (PyObject *(*)(PyObject *)){symbol});\n}}", mangle_member(crate::GENERATOR_THROWN), @@ -1160,10 +1193,34 @@ fn emit_class_members(module: &ModuleIr, class: &ClassIr) -> String { let _ = writeln!( out, "static PyObject *{type_name}_iternext(PyObject *self) {{\n\ - \x20 return By_StepGenerator(self, &(({struct_name} *)self)->{state},\n\ + \x20 return By_StepGenerator(self, &(({struct_name} *)self)->by_returned,\n\ + \x20 &(({struct_name} *)self)->{state},\n\ \x20 (PyObject *(*)(PyObject *)){symbol});\n}}", state = mangle_member(crate::GENERATOR_STATE) ); + // the slot `PyIter_Send` prefers, and the reason a `return` is reported by + // writing it down rather than by raising: an `await` that completes gets its + // answer without an exception being built and immediately unpacked again. + // + // an async generator's state object deliberately has none — python's own + // has none either. what a caller sends into one goes through `asend`, whose + // awaitable is a different object with a different suspension to report + if resume.surface != Surface::AsyncGenerator { + let _ = writeln!( + out, + "#if PY_VERSION_HEX >= 0x030A0000\n\ + static PySendResult {type_name}_send_slot(PyObject *self, PyObject *by_arg,\n\ + \x20 PyObject **by_result) {{\n\ + \x20 return By_SendGenerator(self, &(({struct_name} *)self)->{sent},\n\ + \x20 &(({struct_name} *)self)->by_returned,\n\ + \x20 &(({struct_name} *)self)->{state},\n\ + \x20 (PyObject *(*)(PyObject *)){symbol},\n\ + \x20 by_arg, by_result);\n}}\n\ + #endif", + sent = mangle_member(crate::GENERATOR_SENT), + state = mangle_member(crate::GENERATOR_STATE) + ); + } // the member the frontend writes the suspension kind into let kind_member = class .fields @@ -1208,6 +1265,7 @@ fn emit_class_members(module: &ModuleIr, class: &ClassIr) -> String { \x20 PyObject *by_step;\n\ \x20 if (by_carried != NULL && by_self->by_mode == 3) {{\n\ \x20 by_step = By_ThrowInto((PyObject *)by_gen, &by_gen->{thrown},\n\ + \x20 &by_gen->by_returned,\n\ \x20 &by_gen->{state}, by_carried,\n\ \x20 (PyObject *(*)(PyObject *)){symbol});\n\ \x20 Py_DECREF(by_carried);\n\ @@ -1217,7 +1275,8 @@ fn emit_class_members(module: &ModuleIr, class: &ClassIr) -> String { \x20 by_gen->{sent} = by_carried;\n\ \x20 Py_XDECREF(by_old);\n\ \x20 }}\n\ - \x20 by_step = By_StepGenerator((PyObject *)by_gen, &by_gen->{state},\n\ + \x20 by_step = By_StepGenerator((PyObject *)by_gen, &by_gen->by_returned,\n\ + \x20 &by_gen->{state},\n\ \x20 (PyObject *(*)(PyObject *)){symbol});\n\ \x20 }}\n\ \x20 if (by_step == NULL) return By_EndAsyncIteration();\n\ @@ -1302,14 +1361,27 @@ fn emit_class_members(module: &ModuleIr, class: &ClassIr) -> String { "static PyObject *{type_name}_await(PyObject *self) {{\n\ \x20 return By_NewRef(self);\n}}\n\ static PyAsyncMethods {type_name}_async = {{\n\ - \x20 .am_await = {type_name}_await,\n}};" + \x20 .am_await = {type_name}_await,\n\ + #if PY_VERSION_HEX >= 0x030A0000\n\ + \x20 .am_send = {type_name}_send_slot,\n\ + #endif\n}};" ); format!( " .tp_as_async = &{type_name}_async,\n .tp_iternext = {type_name}_iternext,\n .tp_finalize = {type_name}_finalize,\n" ) } else { + // a generator answers the send slot too — a `yield from` reaches it the + // same way an `await` does. the table exists for that one entry, and the + // awaitable slots stay empty so a generator is still not awaitable + let _ = writeln!( + out, + "#if PY_VERSION_HEX >= 0x030A0000\n\ + static PyAsyncMethods {type_name}_async = {{\n\ + \x20 .am_send = {type_name}_send_slot,\n}};\n\ + #endif" + ); format!( - " .tp_iter = PyObject_SelfIter,\n .tp_iternext = {type_name}_iternext,\n .tp_finalize = {type_name}_finalize,\n" + "#if PY_VERSION_HEX >= 0x030A0000\n .tp_as_async = &{type_name}_async,\n#endif\n .tp_iter = PyObject_SelfIter,\n .tp_iternext = {type_name}_iternext,\n .tp_finalize = {type_name}_finalize,\n" ) } } @@ -2931,13 +3003,6 @@ fn object_expr(function: &Function, value: &Value) -> String { } } -/// whether reading this operand produces a value the reader must release -/// -/// nothing does any more: a string literal is a borrowed static -fn value_is_owned(_value: &Value) -> bool { - false -} - fn c_string(text: &str) -> String { c_byte_string(text.as_bytes()) } @@ -3151,12 +3216,11 @@ fn emit_op( }; let expr = value_expr(src); let mut out = String::new(); - if value_is_owned(src) || !decl.ty.is_refcounted() { + // an operand is always borrowed — a literal is a static and a register is + // the frame's — so an assignment never has an error edge of its own, and + // one whose destination holds no reference is a plain store + if !decl.ty.is_refcounted() { out.push_str(&assign_owned(module, function, *dest, &expr)); - if value_is_owned(src) { - let check = error_check(&decl.ty, &local(*dest)); - let _ = writeln!(out, " if ({check}) goto by_error;"); - } } else { // copying a register: retain the new value before releasing the // old, so `a = a` is safe @@ -4017,12 +4081,36 @@ fn emit_op( let _ = writeln!(out, " {}.f1 = (char)by_done; }}", local(*dest)); out } - Op::RaiseWith { error, value } => format!( - " By_RaiseWith({}, {});\n goto {};\n", - error.c_name(), - value_expr(value), - error_label(error_target) - ), + Op::RaiseWith { error, value } => { + // a resumable frame's `return` is the one raise that is not really one: the + // value is written into the state object and the frame hands back nothing, + // so that `am_send` can report a return for the price of a pointer read + // instead of building a `StopIteration` for its caller to unpack again. + // whoever owes python an exception builds it from there, in `By_TakeReturn`. + // + // only the form that carries a value takes this route. a bare `return` is + // lowered to the same op a written `raise StopIteration` is, and the two + // must not be confused: one finishes the frame, the other is an exception + // the body chose to raise + if *error == by_ir::ops::StandardError::StopIteration && resumes(module, function) { + let receiver = local(RegisterId(0)); + format!( + " {{ PyObject *by_t = By_NewRef({});\n\ + \x20 Py_XDECREF({receiver}->by_returned);\n\ + \x20 {receiver}->by_returned = by_t; }}\n\ + \x20 goto {};\n", + value_expr(value), + error_label(error_target) + ) + } else { + format!( + " By_RaiseWith({}, {});\n goto {};\n", + error.c_name(), + value_expr(value), + error_label(error_target) + ) + } + } Op::GetCell { dest, receiver, @@ -4873,6 +4961,14 @@ fn emit_wrapper(module: &ModuleIr, function: &Function, is_method: bool) -> Stri ctype(module, &function.ret), function.native_symbol(module.name.dotted()) ); + // a resume hands back nothing both when the frame returned and when it raised, and + // the second is the only one a python caller can be given. anything reaching the + // step through this wrapper rather than through the iterator protocol still gets + // the `StopIteration`, because a wrapper that returned NULL with no exception set + // would be a `SystemError` at best + if resumes(module, function) { + out.push_str(" if (by_result == NULL) (void)By_TakeReturn(&a0->by_returned);\n"); + } if function.convention.can_fail() { let _ = writeln!( out, @@ -4953,6 +5049,22 @@ fn c_string_chunked(text: &str) -> String { out } +/// a C string literal for arbitrary bytes, split into adjacent literals +/// +/// as [`c_string_chunked`], but the input is not text: a marshalled code object is +/// mostly bytes no character stands for, and every one of them is escaped. the length +/// is carried separately because these bytes contain NULs +fn c_bytes_chunked(bytes: &[u8]) -> String { + if bytes.is_empty() { + return "\"\"".to_string(); + } + bytes + .chunks(1024) + .map(c_byte_string) + .collect::>() + .join("\n") +} + /// the statements that resolve a dotted name out of the module namespace into `into` /// /// the interpreted fallback has already run, so an imported name is in the module dict; @@ -5151,6 +5263,30 @@ fn emit_module_init(module: &ModuleIr) -> String { "static const char by_fallback_source[] =\n{};\n", c_string_chunked(module.fallback_source.as_deref().unwrap_or("")) ); + // and the same program as a code object, so that an import reads it rather than + // parsing the text over again. the text stays: a code object is only good for the + // interpreter that wrote it — `By_Fallback` says which — and a build with no + // interpreter to ask has none at all + match &module.fallback_code { + Some(code) => { + let _ = writeln!( + out, + "static const char by_fallback_code[] =\n{};\n\n\ + static const By_Fallback by_fallback = {{\n\ + \x20 by_fallback_source, by_fallback_code, {}, {}L, {}}};\n", + c_bytes_chunked(&code.marshalled), + code.marshalled.len(), + code.magic, + code.optimize + ); + } + None => { + let _ = writeln!( + out, + "static const By_Fallback by_fallback = {{by_fallback_source, NULL, 0, 0L, 0}};\n" + ); + } + } out.push_str("static PyMethodDef by_methods[] = {\n"); for function in &module.functions { @@ -5223,12 +5359,17 @@ fn emit_module_init(module: &ModuleIr) -> String { conditions.push("!BY_HAS_MANAGED_DICT"); } // whether the fallback source has to be run with its class bodies captured. the - // capture costs a dict copy per class the body writes, so a module with no constant - // to carry runs its body the plain way - let captures_bodies = module - .classes - .iter() - .any(|class| class.exported && !class.constants.is_empty()); + // capture costs a dict copy per class the body writes, so a module with nothing to + // take out of one runs its body the plain way. a decorated method is taken out of a + // body too — the body is where the decorator's single application landed + let captures_bodies = module.classes.iter().any(|class| { + class.exported + && (!class.constants.is_empty() + || class + .methods + .iter() + .any(|method| !method.decorators.is_empty())) + }); let release_bodies = if captures_bodies { " Py_XDECREF(by_bodies);\n" } else { @@ -5317,7 +5458,15 @@ fn emit_module_init(module: &ModuleIr) -> String { // constant's value comes from — the twin has been through its own decorators by // now, and `By_RunModuleBody` says what that costs. borrowed from `by_bodies`, // which is held for the whole of this function - if twins.iter().any(|class| !class.constants.is_empty()) { + // a decorated method takes its value from here too, so the bodies are needed + // whenever either asks for one + if twins.iter().any(|class| { + !class.constants.is_empty() + || class + .methods + .iter() + .any(|method| !method.decorators.is_empty()) + }) { let _ = writeln!(twin_init, " PyObject *by_body[{count}];"); for (slot, class) in twins.iter().enumerate() { let _ = writeln!( @@ -5522,13 +5671,18 @@ fn emit_module_init(module: &ModuleIr) -> String { .map(|decorator| c_string(&decorator.dotted())) .collect::>() .join(", "); + // a class with no interpreted `class` statement has no body to take the + // decorator's answer from — and never ran the decorators either, so + // applying them there is the only application rather than a second one + let body = slot.map_or_else(|| "NULL".to_string(), |slot| format!("by_body[{slot}]")); let _ = writeln!( class_init, " {{ static const char *const by_decorators[] = {{{names}}};\n\ - \x20 if (By_ApplyMethodDecorators((PyTypeObject *){type_name}_OBJ, dict, {}, {}, by_decorators, {}) < 0) return -1; }}", + \x20 if (By_DecoratedMethod({body}, (PyTypeObject *){type_name}_OBJ, dict, {}, {}, by_decorators, {}, by_twin, by_type, {}) < 0) return -1; }}", c_string(&class.name), c_string(&method.name), - method.decorators.len() + method.decorators.len(), + twins.len() ); } // a closure environment is a real type with a real layout, and nothing @@ -5605,7 +5759,7 @@ fn emit_module_init(module: &ModuleIr) -> String { let run_body = if captures_bodies { "\x20 PyObject *by_bodies = NULL;\n\ \x20 if (by_fallback_source[0] != '\\0') {\n\ - \x20 by_bodies = By_RunModuleBody(by_fallback_source, dict);\n\ + \x20 by_bodies = By_RunModuleBody(&by_fallback, dict);\n\ \x20 if (by_bodies == NULL) return -1;\n\ \x20 }\n" .to_string() @@ -5615,7 +5769,7 @@ fn emit_module_init(module: &ModuleIr) -> String { \x20 PyDict_SetItemString(dict, \"__builtins__\", PyEval_GetBuiltins()) < 0) {\n\ \x20 return -1;\n\ \x20 }\n\ - \x20 PyObject *result = PyRun_String(by_fallback_source, Py_file_input, dict, dict);\n\ + \x20 PyObject *result = By_ExecModuleBody(&by_fallback, dict);\n\ \x20 if (result == NULL) return -1;\n\ \x20 Py_DECREF(result);\n\ \x20 }\n" @@ -5656,6 +5810,10 @@ fn emit_module_init(module: &ModuleIr) -> String { \x20 PyModuleDef_HEAD_INIT, \"{last}\", NULL, 0, NULL, by_slots, NULL, NULL, NULL\n\ }};\n\n\ PyMODINIT_FUNC {}(void) {{\n\ + \x20 /* before `by_module` itself is handed over: a module definition is a\n\ + \x20 * struct this build laid out, so a mismatched interpreter must be turned\n\ + \x20 * away without reading one */\n\ + \x20 if (!By_InterpreterMatches()) return NULL;\n\ \x20 return PyModuleDef_Init(&by_module);\n\ }}\n", module.init_symbol() @@ -5687,6 +5845,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } @@ -6120,6 +6279,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let c = emit_module(&module); // the forward declaration precedes the body, so take the last split @@ -6314,6 +6474,7 @@ mod tests { }], lines: None, fallback_source: None, + fallback_code: None, }; let c = emit_module(&module); assert!(c.contains("PyMODINIT_FUNC PyInit_app(void)")); @@ -6323,6 +6484,26 @@ mod tests { )); } + /// the version tag an artefact carries is in its file name, and every 3.x also + /// accepts a bare `.so` — so a renamed artefact is offered to an interpreter the + /// build never saw. the emitted init refuses one before it reads `by_module`, + /// because that struct's layout is the build's own + #[test] + fn the_module_init_refuses_a_mismatched_interpreter_first() { + let c = emit_module(&module_with(add())); + let init = c + .split_once("PyMODINIT_FUNC PyInit_app(void)") + .expect("the module init is emitted") + .1; + let guard = init + .find("if (!By_InterpreterMatches()) return NULL;") + .expect("the init guards on the running interpreter"); + let hand_over = init + .find("PyModuleDef_Init(&by_module)") + .expect("the init hands the definition over"); + assert!(guard < hand_over); + } + /// cpython reads a class's `__module__` off the front of its `tp_name` and its /// `__name__` off the back, so a class in a package needs the *whole* dotted /// name there. emitting only the last component made a class in `tkinter/m.py` @@ -6362,7 +6543,7 @@ mod tests { let c = emit_module(&module); // the natives must be installed from the exec slot, not from m_methods, // or the interpreted definitions would overwrite them - assert!(c.contains("PyRun_String(by_fallback_source"), "{c}"); + assert!(c.contains("By_ExecModuleBody(&by_fallback, dict)"), "{c}"); assert!( c.contains("PyModule_AddFunctions(module, by_methods)"), "{c}" @@ -6413,6 +6594,121 @@ mod tests { ); } + /// read a run of adjacent C string literals back into the bytes they stand for + /// + /// the emitted escaping is only worth having if it is exact, and "the module still + /// imports" cannot say that it is: an artefact whose code object will not read + /// falls back to its source and behaves identically, just slower. so the literal is + /// decoded here and compared byte for byte + fn decode_c_literals(text: &str) -> Vec { + let bytes = text.as_bytes(); + let mut out = Vec::new(); + let mut at = 0; + let mut inside = false; + while at < bytes.len() { + let byte = bytes[at]; + at += 1; + if !inside { + inside = byte == b'"'; + continue; + } + match byte { + b'"' => inside = false, + b'\\' => { + let escape = bytes[at]; + at += 1; + match escape { + b'n' => out.push(b'\n'), + b'r' => out.push(b'\r'), + b't' => out.push(b'\t'), + b'"' => out.push(b'"'), + b'\\' => out.push(b'\\'), + // octal, always exactly three digits so a digit that follows + // stays a digit + _ => { + let digits = std::str::from_utf8(&bytes[at - 1..at + 2]) + .expect("an octal escape is ascii"); + out.push(u8::from_str_radix(digits, 8).expect("three octal digits")); + at += 2; + } + } + } + other => out.push(other), + } + } + out + } + + #[test] + fn the_compiled_twin_is_emitted_byte_for_byte_beside_its_source() { + // marshalled bytes are not text: every value from 0 to 255 turns up, NULs and + // quotes and backslashes among them, and a digit landing right after an escaped + // byte is what a two-digit octal escape would swallow + let mut module = module_with(add()); + module.fallback_source = Some("x = 1\n".to_string()); + let marshalled: Vec = (0..=255u8).chain([b'\\', b'1', 1, b'7', b'"', 0]).collect(); + module.fallback_code = Some(by_ir::function::FallbackCode { + marshalled: marshalled.clone().into(), + magic: 168_627_699, + optimize: 2, + }); + let c = emit_module(&module); + let declaration = c + .split("static const By_Fallback") + .next() + .expect("the literal precedes the struct"); + let literal = declaration + .split("static const char by_fallback_code[] =") + .nth(1) + .expect("the code object is emitted"); + assert_eq!(decode_c_literals(literal), marshalled); + assert!( + c.contains(&format!( + "by_fallback_source, by_fallback_code, {}, 168627699L, 2}}", + marshalled.len() + )), + "{c}" + ); + } + + #[test] + fn a_module_with_no_compiled_twin_stands_a_null_in_its_place() { + // `--emit-c-only` with no interpreter to ask is the case: the artefact still + // carries its source, and the runtime reads a NULL as "compile that instead" + let mut module = module_with(add()); + module.fallback_source = Some("x = 1\n".to_string()); + let c = emit_module(&module); + assert!(!c.contains("by_fallback_code[]"), "{c}"); + assert!( + c.contains( + "static const By_Fallback by_fallback = {by_fallback_source, NULL, 0, 0L, 0};" + ), + "{c}" + ); + } + + #[test] + fn a_long_compiled_twin_is_split_into_adjacent_literals() { + // the same implementation-defined maximum the source literal has to dodge + let mut module = module_with(add()); + module.fallback_source = Some("x = 1\n".to_string()); + module.fallback_code = Some(by_ir::function::FallbackCode { + marshalled: vec![b'a'; 5000].into(), + magic: 1, + optimize: 0, + }); + let c = emit_module(&module); + let literal = c + .split("static const char by_fallback_code[] =") + .nth(1) + .expect("the code object is emitted") + .split("static const By_Fallback") + .next() + .expect("the struct follows it"); + assert_eq!(literal.matches("\"\n\"").count(), 4, "{literal}"); + assert_eq!(decode_c_literals(literal), vec![b'a'; 5000]); + } + #[test] fn a_string_literal_is_interned_once_rather_than_built_per_use() { // building it per use created a reference nobody released — a leak diff --git a/crates/by_ir/src/function.rs b/crates/by_ir/src/function.rs index 63404beb54..fbb365b9f9 100644 --- a/crates/by_ir/src/function.rs +++ b/crates/by_ir/src/function.rs @@ -676,6 +676,40 @@ pub struct ModuleIr { /// declined functions exist, and the natively compiled functions are then /// installed over the top of their interpreted definitions pub fallback_source: Option, + /// the same program, already compiled by the interpreter this artefact is + /// being built for, when the build had one to ask. + /// + /// parsing a module the size of `argparse` costs milliseconds and reading a + /// marshalled code object costs tens of microseconds, and every import pays + /// the difference. it is a *cache* of [`Self::fallback_source`] and never a + /// replacement for it — see [`FallbackCode`] for what an interpreter has to + /// match before it may use one + pub fallback_code: Option, +} + +/// a module body compiled to a code object, and what an interpreter has to match +/// before it may run that code object rather than the source +/// +/// both conditions are about the code object being *the same program* the source +/// would have compiled to on this interpreter. neither is a guess: they are the two +/// things cpython itself keys a `.pyc` on +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FallbackCode { + /// `marshal.dumps` of the module body's code object + pub marshalled: Box<[u8]>, + /// the bytecode magic number of the interpreter that wrote it. + /// + /// cpython bumps this whenever a code object stops meaning what it did, which + /// is why an upgraded interpreter regenerates `.pyc` files instead of misreading + /// them. an interpreter whose own magic differs falls back to the source + pub magic: i64, + /// the optimization level it was compiled at. + /// + /// `-O` takes `assert` out of the bytecode and `-OO` takes docstrings too, so + /// source compiled at one level is a different program from the same source + /// compiled at another. an interpreter running at a different level falls back + /// to the source, which is what makes `python -O` still mean `-O` here + pub optimize: i32, } /// a gradual type in a compiled signature @@ -804,6 +838,7 @@ impl ModuleIr { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_ir/src/print.rs b/crates/by_ir/src/print.rs index 0b7bd4a7af..03729d33ef 100644 --- a/crates/by_ir/src/print.rs +++ b/crates/by_ir/src/print.rs @@ -745,6 +745,7 @@ b2: promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let text = print_module(&module); assert!(text.starts_with("module app\n")); diff --git a/crates/by_ir/src/verify.rs b/crates/by_ir/src/verify.rs index c329901ddd..9a7e7a3bd0 100644 --- a/crates/by_ir/src/verify.rs +++ b/crates/by_ir/src/verify.rs @@ -1861,6 +1861,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let errors = verify_module(&module).unwrap_err(); assert_eq!(errors.len(), 1); @@ -1896,6 +1897,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; let errors = verify_module(&module).unwrap_err(); assert!( diff --git a/crates/by_irbuild/src/lib.rs b/crates/by_irbuild/src/lib.rs index 7eb9bae8ef..2d9f7f23c2 100644 --- a/crates/by_irbuild/src/lib.rs +++ b/crates/by_irbuild/src/lib.rs @@ -4662,6 +4662,35 @@ fn signature( .chain(parameters.args.iter()) .chain(parameters.kwonlyargs.iter()) .collect::>(); + + // a parameter's register has to cover every value written into it, and its own + // body is one of the writers. an *unannotated* parameter is declared by its + // default — ty reads `def quote_from_bytes(bs, safe='/')` as taking a `str` — and + // python is perfectly happy for the body to rebind the name, which + // `safe = safe.encode('ascii', 'ignore')` does. a register shaped for the default + // then either refuses that store or narrows it with a check, and the check raises + // on a call the interpreter answers without complaint + // + // this lives here rather than where the body is lowered because a parameter's + // representation is part of the calling convention: a caller coerces its arguments + // to it and the boundary unboxes to it, so every reader of the signature has to + // see the same widening the body will + // + // the editions are deliberately empty rather than the real ones. which lists live + // in an unboxed buffer is still being settled when the signature tables are built, + // so consulting them here would make a signature depend on *when* it was computed. + // a parameter that already is a buffer is left alone below for the same reason + let rebound: HashMap = local_representations( + db, + env, + model, + &function.body, + layouts, + &ArrayEditions::new(), + ) + .into_iter() + .collect(); + for (index, parameter) in named.iter().enumerate() { // a *literal* default is evaluated once in python and cannot change, so // inlining it is the same thing. @@ -4721,7 +4750,29 @@ fn signature( } } }; - params.push((parameter.parameter.name.to_string(), rtype)); + let name = parameter.parameter.name.as_str(); + let rtype = match rebound.get(name) { + // an unboxed edition's parameter *is* the caller's buffer, and a buffer is + // not a value that widens: handing one out means copying it, and a copy is + // a different list. the store in the body declines instead, as it already + // did before a parameter's writes were counted at all + Some(_) if matches!(rtype, RType::Array(_)) => rtype, + Some(written) => { + let covered = covering(&rtype, written); + // slot zero is the receiver, which every field read in the body is + // written against — a frame whose `self` has become an object no + // longer has one to read a field off + if covered != rtype && index == 0 && matches!(receiver, Some(Receiver::Explicit(_))) + { + return Err(Decline::new( + "a method whose body rebinds its receiver is not lowered yet", + )); + } + covered + } + None => rtype, + }; + params.push((name.to_string(), rtype)); } // the return type comes from the returns themselves rather than from the @@ -4960,6 +5011,23 @@ fn return_type( } } +/// the one representation that covers two writes to the same place +/// +/// there is no union representation, so two that do not already agree meet at the +/// object at the top of the lattice. a `bit` and a `bool` are the same byte, and so +/// meet at `bool` rather than falling all the way up +fn covering(left: &RType, right: &RType) -> RType { + if left == right { + return left.clone(); + } + if matches!(left, RType::Primitive(Primitive::Bit | Primitive::Bool)) + && matches!(right, RType::Primitive(Primitive::Bit | Primitive::Bool)) + { + return RType::BOOL; + } + RType::OBJECT +} + /// the representation each local needs, covering every value assigned to it /// /// computed before any lowering, because a register is declared once and every @@ -4975,24 +5043,19 @@ fn local_representations( let mut order: Vec = Vec::new(); let mut found: HashMap = HashMap::new(); - let mut record = |name: &str, rtype: RType, found: &mut HashMap| { - match found.get(name) { + let mut record = + |name: &str, rtype: RType, found: &mut HashMap| match found.get(name) { None => { order.push(name.to_string()); found.insert(name.to_string(), rtype); } - Some(existing) if *existing == rtype => {} - // a `bit` and a `bool` are the same byte; anything else widens - Some(RType::Primitive(Primitive::Bit | Primitive::Bool)) - if matches!(rtype, RType::Primitive(Primitive::Bit | Primitive::Bool)) => - { - found.insert(name.to_string(), RType::BOOL); - } - Some(_) => { - found.insert(name.to_string(), RType::OBJECT); + Some(existing) => { + let covered = covering(existing, &rtype); + if covered != *existing { + found.insert(name.to_string(), covered); + } } - } - }; + }; // deliberately *not* `map_local_type`: that one may answer with an unboxed // array, and the array decision belongs to the two sites below that ask @@ -5076,6 +5139,15 @@ fn local_representations( let mut targets: Vec<(String, RType)> = Vec::new(); for expr in crate::closures::statement_expressions(stmt) { crate::closures::visit_expressions(expr, &mut |child| { + // `x := v` binds wherever it stands, so it is a write like any other — + // and one that hides inside an expression rather than standing as a + // statement, which is why it is looked for here + if let Expr::Named(node) = child + && let Expr::Name(name) = node.target.as_ref() + { + targets.push((name.id.to_string(), peek(&node.value))); + return; + } let generators = match child { Expr::ListComp(node) => &node.generators, Expr::SetComp(node) => &node.generators, @@ -5150,6 +5222,17 @@ fn local_representations( record(&name, rtype, &mut found); } } + // `except E as e` binds the caught exception, which the lowering fetches + // as a plain object — so this is the one write whose representation is + // known without asking the checker anything + Stmt::Try(node) => { + for handler in &node.handlers { + let ast::ExceptHandler::ExceptHandler(handler) = handler; + if let Some(bound) = &handler.name { + record(bound.as_str(), RType::OBJECT, &mut found); + } + } + } // a nested `def` binds its name to a callable Stmt::FunctionDef(node) => { record(node.name.as_str(), RType::OBJECT, &mut found); diff --git a/crates/by_irbuild/src/mapper.rs b/crates/by_irbuild/src/mapper.rs index 3048b10291..340ad44075 100644 --- a/crates/by_irbuild/src/mapper.rs +++ b/crates/by_irbuild/src/mapper.rs @@ -180,6 +180,22 @@ pub fn map_type( return Ok(RType::OBJECT); } + // the bottom of the lattice proves nothing either, for the mirror image of the + // reason the top does: `Never` is assignable to *everything*, so every test below + // answers yes and whichever is written first wins. that was `None`, a representation + // with no width at all — and ty gives every expression in unreachable code the type + // `Never`, so + // + // def f(line: str): + // return + // b = not line + // + // asked to store a `bit` into a place that cannot hold one, and declined a function + // for code that never runs. the same two statements without the `return` compile + if ty.is_never() { + return Ok(RType::OBJECT); + } + let none = Type::none(db, env); if ty.is_assignable_to(db, env, none) { return Ok(RType::NONE); @@ -217,7 +233,8 @@ mod tests { fn param_repr(annotation: &str) -> Result { with_source( &format!( - "from typing import Any, Literal\ndef f(a: {annotation}) -> None:\n pass\n" + "from typing import Any, Literal, Never\n\ + def f(a: {annotation}) -> None:\n pass\n" ), |db, env, model, suite| { let ruff_python_ast::Stmt::FunctionDef(function) = &suite[1] else { @@ -292,6 +309,17 @@ mod tests { assert_eq!(param_repr("Any"), Ok(RType::OBJECT)); } + #[test] + fn the_bottom_of_the_lattice_is_the_widest_representation_too() { + // `Never` is assignable to *everything*, so every test in `map_type` answers + // yes and whichever is written first wins. that was `None`, which has no width + // at all — and since ty types every expression in unreachable code as `Never`, + // a dead `b = not line` after a `return` had a `bit` and nowhere to put it + assert_eq!(param_repr("Never"), Ok(RType::OBJECT)); + // and a real `None` still gets the representation that says so + assert_eq!(param_repr("None"), Ok(RType::NONE)); + } + #[test] fn a_type_with_no_unboxed_representation_is_still_an_object() { assert_eq!(param_repr("list[int]"), Ok(RType::OBJECT)); diff --git a/crates/by_irbuild/src/tests.rs b/crates/by_irbuild/src/tests.rs index b74ae14c0b..312dc2da48 100644 --- a/crates/by_irbuild/src/tests.rs +++ b/crates/by_irbuild/src/tests.rs @@ -576,6 +576,96 @@ def f(a: int, /, b: int, *, c: int) -> int: ); } +/// the representation of one parameter of the single function in `source` +fn param_type(source: &str, function: &str, parameter: &str) -> RType { + with_source(source, |db, env, model, suite| { + let module = crate::build_module(db, env, model, suite, "app", true); + assert!(module.declined.is_empty(), "{:?}", module.declined); + module + .all_functions() + .find(|candidate| candidate.name == function) + .and_then(|lowered| { + lowered + .params() + .iter() + .find(|decl| decl.name.as_deref() == Some(parameter)) + .map(|decl| decl.ty.clone()) + }) + .unwrap_or_else(|| panic!("{function} has no parameter {parameter}")) + }) +} + +#[test] +fn a_parameter_its_own_body_rebinds_covers_every_write() { + // an unannotated parameter is declared by its default, so `safe='/'` alone would + // make the register a `str`. the body writes to it too, and `safe.encode(...)` is + // bytes: a `str` register would have to narrow that store with a check, and the + // check raises on a call the interpreter answers + assert_eq!( + param_type( + "\ +def quoted(safe='/'): + safe = safe.encode('ascii') + return repr(safe) +", + "quoted", + "safe", + ), + RType::OBJECT + ); +} + +#[test] +fn a_walrus_and_a_handler_name_are_writes_a_parameter_has_to_cover() { + // neither is an assignment statement: a walrus binds from inside an expression, + // and a handler's name hangs off the `try` rather than standing in its body. both + // were invisible to the walk that decides a register's representation + assert_eq!( + param_type( + "\ +def walrused(safe='/'): + if (safe := safe.encode('ascii')): + return repr(safe) + return 'empty' +", + "walrused", + "safe", + ), + RType::OBJECT + ); + assert_eq!( + param_type( + "\ +def caught(tag='t'): + try: + raise ValueError('boom') + except ValueError as tag: + return repr(tag) +", + "caught", + "tag", + ), + RType::OBJECT + ); +} + +#[test] +fn a_parameter_its_own_body_leaves_alone_keeps_its_declared_representation() { + // the widening is per parameter and driven by the writes, so a body that only + // *reads* one costs it nothing — a `str` here stays laid out as a `str` + assert_eq!( + param_type( + "\ +def quoted(safe='/'): + return repr(safe) +", + "quoted", + "safe", + ), + RType::STR + ); +} + #[test] fn a_comprehension_gives_each_for_its_own_header() { // an `if` guard skips to the next value of *its own* loop, so a guard on the @@ -1047,6 +1137,30 @@ class Tagged: ); } +#[test] +fn a_method_that_rebinds_its_receiver_declines() { + // every other parameter widens to cover what its body writes into it. slot zero + // cannot: it is the receiver each field read in the body is addressed against, and + // a frame whose `self` has become an ordinary object has no layout left to read + let reasons = declines( + "\ +class Tagged: + def __init__(self, kind: str) -> None: + self.kind = kind + + def read(self) -> object: + self = 3 + return self +", + ); + assert!( + reasons + .iter() + .any(|(name, reason)| name == "Tagged" && reason.contains("rebinds its receiver")), + "{reasons:?}" + ); +} + #[test] fn a_decorated_class_with_a_class_level_constant_is_lowered() { // the constant is copied off the body the interpreted `class` statement wrote, which diff --git a/crates/by_opt/src/coalesce.rs b/crates/by_opt/src/coalesce.rs index 01bf1718ad..4ffbac893a 100644 --- a/crates/by_opt/src/coalesce.rs +++ b/crates/by_opt/src/coalesce.rs @@ -120,6 +120,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/copy_propagation.rs b/crates/by_opt/src/copy_propagation.rs index 96cce8d981..a0a977fe3d 100644 --- a/crates/by_opt/src/copy_propagation.rs +++ b/crates/by_opt/src/copy_propagation.rs @@ -351,6 +351,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/dead_registers.rs b/crates/by_opt/src/dead_registers.rs index 9c271c5a9b..f3b0ec7891 100644 --- a/crates/by_opt/src/dead_registers.rs +++ b/crates/by_opt/src/dead_registers.rs @@ -462,6 +462,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/fold.rs b/crates/by_opt/src/fold.rs index 134f8545c1..3bc879e55f 100644 --- a/crates/by_opt/src/fold.rs +++ b/crates/by_opt/src/fold.rs @@ -528,6 +528,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/infallible.rs b/crates/by_opt/src/infallible.rs index d14e237e7c..6fba257b9c 100644 --- a/crates/by_opt/src/infallible.rs +++ b/crates/by_opt/src/infallible.rs @@ -277,6 +277,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/lib.rs b/crates/by_opt/src/lib.rs index 6c826c1119..30a9857a34 100644 --- a/crates/by_opt/src/lib.rs +++ b/crates/by_opt/src/lib.rs @@ -158,6 +158,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; assert!(optimize(&mut module).is_ok()); let fixed = |id: by_ir::ops::RegisterId| { @@ -193,6 +194,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, }; assert!(optimize(&mut module).is_ok()); // both passes fired: the copy is gone and the function is infallible diff --git a/crates/by_opt/src/refcount.rs b/crates/by_opt/src/refcount.rs index 85b901dfa2..64bca5f20b 100644 --- a/crates/by_opt/src/refcount.rs +++ b/crates/by_opt/src/refcount.rs @@ -110,6 +110,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/str_append.rs b/crates/by_opt/src/str_append.rs index 77d1792b41..95cdc4cf83 100644 --- a/crates/by_opt/src/str_append.rs +++ b/crates/by_opt/src/str_append.rs @@ -186,6 +186,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/str_item_compare.rs b/crates/by_opt/src/str_item_compare.rs index e6d2522f92..1bf1bc4a78 100644 --- a/crates/by_opt/src/str_item_compare.rs +++ b/crates/by_opt/src/str_item_compare.rs @@ -156,6 +156,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_opt/src/unswitch.rs b/crates/by_opt/src/unswitch.rs index 938fefa991..47018139c4 100644 --- a/crates/by_opt/src/unswitch.rs +++ b/crates/by_opt/src/unswitch.rs @@ -380,6 +380,7 @@ mod tests { promoted: Vec::new(), lines: None, fallback_source: None, + fallback_code: None, } } diff --git a/crates/by_rt/include/by.h b/crates/by_rt/include/by.h index 0b26ffa2e1..418406a36e 100644 --- a/crates/by_rt/include/by.h +++ b/crates/by_rt/include/by.h @@ -26,11 +26,59 @@ #define PY_SSIZE_T_CLEAN #include +/* `PyMarshal_ReadObjectFromString`, which reads the interpreted twin's code object + * back. `Python.h` does not pull this one in */ +#include #include #include #include #include +/* the major and minor at the front of a cpython version string + * + * `Py_GetVersion` answers the whole banner — `"3.14.0a1 (main, ...) [Clang ...]"` — and + * only its first two numbers are wanted. anything that does not begin with two + * dot-separated runs of digits leaves both at -1, which no build matches, so an + * unreadable banner is refused exactly as a mismatched one is */ +static inline void By_ParseVersion(const char *version, int *major, int *minor) { + const char *at = version; + int read_major = 0; + int read_minor = 0; + *major = -1; + *minor = -1; + if (version == NULL || *at < '0' || *at > '9') return; + while (*at >= '0' && *at <= '9') read_major = read_major * 10 + (*at++ - '0'); + if (*at != '.' || at[1] < '0' || at[1] > '9') return; + at++; + while (*at >= '0' && *at <= '9') read_minor = read_minor * 10 + (*at++ - '0'); + *major = read_major; + *minor = read_minor; +} + +/* does the interpreter that is running match the one this module was built against? + * + * every `PY_VERSION_HEX` branch in this header is decided by the headers the build + * compiled against, so an artefact loaded by a different minor version runs branches + * written for a layout that interpreter does not have. that is a crash rather than a + * wrong answer, and nothing upstream of here refuses it: the version tag lives in the + * *file name*, and a bare `.so` — which every 3.x lists in `EXTENSION_SUFFIXES` — is + * offered to whatever is running. so an artefact renamed, or copied out of a wheel built + * elsewhere, reaches module init with no check having happened. + * + * `Py_GetVersion` rather than `Py_Version`: it is the one of the two that every version + * this header compiles against exports, and a module built against newer headers naming + * a symbol the running interpreter lacks is the same failure by another road */ +static inline int By_InterpreterMatches(void) { + int major; + int minor; + By_ParseVersion(Py_GetVersion(), &major, &minor); + if (major == PY_MAJOR_VERSION && minor == PY_MINOR_VERSION) return 1; + PyErr_Format(PyExc_ImportError, + "this module was compiled for python %d.%d, and python %d.%d is running", + PY_MAJOR_VERSION, PY_MINOR_VERSION, major, minor); + return 0; +} + /* ── tagged integers ────────────────────────────────────────────────────────── * * a ByTagged is a pointer-sized word. an even value is a "short": the integer @@ -2287,9 +2335,114 @@ static PyObject *By_CaptureClassBody(PyObject *state, PyObject *args, PyObject * return cls; } -/* run a module's fallback source, capturing each class body before its decorators run +/* a module's interpreted twin, in the two forms an artefact carries it + * + * `source` is the twin as text and is always here. `code` is the same program already + * compiled, by the interpreter this artefact was built for, and it is the whole reason + * an import is cheap: parsing a module the size of `argparse` costs milliseconds and + * reading a marshalled code object costs tens of microseconds. + * + * it is a cache of the source rather than a replacement for it, and the two fields below + * it say who may use it. an interpreter that does not match compiles the source instead, + * which is slower and is the same program — the outcome must never turn on which of the + * two ran */ +typedef struct { + const char *source; + /* `marshal.dumps` of the module body's code object, or NULL where the build had no + * interpreter to compile it with */ + const char *code; + Py_ssize_t length; + /* the bytecode magic of the interpreter that wrote it. cpython bumps this whenever a + * code object stops meaning what it did, which is why an upgraded interpreter + * regenerates a `.pyc` rather than misreading one — the same check, for the same + * reason */ + long magic; + /* the optimization level it was compiled at. `-O` takes `assert` out of the bytecode + * and `-OO` takes docstrings too, so the same source at another level is a different + * program. running the twin under `python -O` has always meant `-O`, and reading back + * a code object compiled without it would quietly stop meaning that */ + int optimize; +} By_Fallback; + +/* this interpreter's optimization level, or -1 where it will not say + * + * `sys.flags.optimize` rather than any of the C-level flags: those have been deprecated + * and moved about across the versions this compiler targets, and this one is the reading + * python's own `compile` takes */ +static inline int By_OptimizeLevel(void) { + PyObject *flags = PySys_GetObject("flags"); /* borrowed */ + PyObject *level; + long value; + if (flags == NULL) { + PyErr_Clear(); + return -1; + } + level = PyObject_GetAttrString(flags, "optimize"); + if (level == NULL) { + PyErr_Clear(); + return -1; + } + value = PyLong_AsLong(level); + Py_DECREF(level); + if (value == -1 && PyErr_Occurred()) { + PyErr_Clear(); + return -1; + } + return (int)value; +} + +/* the twin's code object, where this interpreter may use the one the artefact carries + * + * hands back a new reference, or NULL. NULL with no exception set means there is nothing + * here for *this* interpreter and the source should be compiled instead; NULL with one + * set means there was and it would not read, which is a broken artefact rather than a + * mismatched one and is raised rather than papered over. that distinction is what keeps a + * defect in how these bytes are emitted from showing up as nothing worse than a slow + * import nobody looks at */ +static inline PyObject *By_FallbackCode(const By_Fallback *fallback) { + PyObject *code; + if (fallback->code == NULL || fallback->length <= 0) return NULL; + if (PyImport_GetMagicNumber() != fallback->magic) { + PyErr_Clear(); + return NULL; + } + if (By_OptimizeLevel() != fallback->optimize) return NULL; + code = PyMarshal_ReadObjectFromString(fallback->code, fallback->length); + if (code == NULL) { + if (!PyErr_Occurred()) { + PyErr_SetString(PyExc_ImportError, + "the interpreted definitions of this module could not be read"); + } + return NULL; + } + if (!PyCode_Check(code)) { + Py_DECREF(code); + PyErr_SetString(PyExc_ImportError, + "the interpreted definitions of this module are not a code object"); + return NULL; + } + return code; +} + +/* run the twin in `dict`, from the code object where there is a usable one * - * the source is the whole interpreted module and it runs to completion before any emitted + * `PyEval_EvalCode` and `PyRun_String` reach the same evaluator by the same route, and + * both take the builtins the frame runs against from `dict["__builtins__"]` — which is + * what lets the capture below swap that entry and have it seen */ +static inline PyObject *By_ExecModuleBody(const By_Fallback *fallback, PyObject *dict) { + PyObject *code = By_FallbackCode(fallback), *result; + if (code != NULL) { + result = PyEval_EvalCode(code, dict, dict); + Py_DECREF(code); + return result; + } + if (PyErr_Occurred()) return NULL; + return PyRun_String(fallback->source, Py_file_input, dict, dict); +} + +/* run a module's interpreted twin, capturing each class body before its decorators run + * + * the twin is the whole interpreted module and it runs to completion before any emitted * type is built, so by the time a class-level constant is copied onto one, the class it * would be copied off has already been through its own decorators. a decorator that only * *reads* the class leaves the value where the body put it, but one that makes something @@ -2311,7 +2464,7 @@ static PyObject *By_CaptureClassBody(PyObject *state, PyObject *args, PyObject * * * hands back `{name: body}` for the classes the body wrote at module level, as a new * reference, or NULL with an exception set where the body raised */ -static inline PyObject *By_RunModuleBody(const char *source, PyObject *dict) { +static inline PyObject *By_RunModuleBody(const By_Fallback *fallback, PyObject *dict) { static PyMethodDef capture = {"__build_class__", (PyCFunction)(void (*)(void))By_CaptureClassBody, METH_VARARGS | METH_KEYWORDS, NULL}; @@ -2344,7 +2497,7 @@ static inline PyObject *By_RunModuleBody(const char *source, PyObject *dict) { failed = wrapper == NULL || PyDict_SetItemString(builtins, "__build_class__", wrapper) < 0 || PyDict_SetItemString(dict, "__builtins__", builtins) < 0; Py_XDECREF(wrapper); - result = failed ? NULL : PyRun_String(source, Py_file_input, dict, dict); + result = failed ? NULL : By_ExecModuleBody(fallback, dict); Py_XDECREF(result); /* whatever the body did, the capture stops here */ { @@ -3384,6 +3537,40 @@ static inline int By_ApplyMethodDecorators(PyTypeObject *type, PyObject *dict, return 0; } +/* the decorated method, taken from the class body where the body already built one + * + * a method's decorators run *inside* the class body: `@mark def g` is a `def` statement, + * and the interpreted definition ran it before anything of this module existed. so the + * body already holds the decorator's answer, and applying the decorators again to the + * native method calls them a **second time**. a decorator that only reads its argument is + * unharmed; one that registers registers twice, which is a silent miscompile — + * `@atexit.register`, a route table, any `SEEN.append(fn)`. + * + * so the body's answer is taken where there is one. the price is that such a method is the + * *interpreted* one: a decorator is handed whatever the body gave it, and there is no way + * to hand it the native method without calling it again. an undecorated method is not + * touched and stays native, which is where the speed of a compiled class lives anyway. + * + * where there is no body answer to take, the decorators are applied — and that is not a + * second application but the only one, because the double is *caused* by a body having run + * them. a class with no interpreted `class` statement never ran any. + * + * a class whose construction fell back to the interpreted definition is already exactly + * what this would build, and is under its own name in the namespace where nothing this + * module built can be yet */ +static inline int By_DecoratedMethod(PyObject *body, PyTypeObject *type, PyObject *dict, + const char *owner, const char *name, + const char *const *decorators, Py_ssize_t count, + PyObject *const *twins, PyObject *const *types, + Py_ssize_t classes) { + if (count <= 0) return 0; + if ((PyObject *)type == PyDict_GetItemString(dict, owner)) return 0; + if (body != NULL && PyDict_GetItemString(body, name) != NULL) { + return By_CopyClassConstant(body, type, name, twins, types, classes); + } + return By_ApplyMethodDecorators(type, dict, owner, name, decorators, count); +} + /* apply the decorator `decorator` names to `dict[name]`, in place. this is what * lets a decorated function still be compiled: the native one goes into the * namespace, then the decorator wraps it, exactly as the `def` statement would @@ -3508,6 +3695,10 @@ static inline void By_RaiseWithMessage(PyObject *cls, const char *message) { PyErr_SetString(cls, message); } +/* defined with the rest of the await protocol, below; a resumable frame's return + * has to be able to reach it from here */ +static inline void By_RaiseWith(PyObject *error, PyObject *value); + /* the frame has left for good, so `$state` says finished * * python marks a generator completed the moment control leaves its frame, whether @@ -3521,12 +3712,32 @@ static inline void By_FinishGenerator(ByTagged *state) { *state = By_ShortFrom(-1); } -/* resume a generator's frame, finishing it when the frame leaves by raising */ -static inline PyObject *By_StepGenerator(PyObject *self, ByTagged *state, +/* the value a `return` handed back, turned into the exception the iterator protocol + * expects + * + * a resume reports its return by *storing* it in `$returned` rather than by raising, + * so that `am_send` can answer what a frame returned without an exception ever being + * built. every consumer that owes python a raise builds it here instead, which is the + * one place the two faces can drift apart and so the one place to keep them together. + * + * `*returned` empty means the frame left by raising and the error is already set */ +static inline PyObject *By_TakeReturn(PyObject **returned) { + PyObject *value = *returned; + if (value == NULL) return NULL; + *returned = NULL; + By_RaiseWith(PyExc_StopIteration, value); + Py_DECREF(value); + return NULL; +} + +/* resume a generator's frame, finishing it when the frame leaves for good */ +static inline PyObject *By_StepGenerator(PyObject *self, PyObject **returned, + ByTagged *state, PyObject *(*resume)(PyObject *)) { PyObject *result = resume(self); - if (result == NULL) By_FinishGenerator(state); - return result; + if (result != NULL) return result; + By_FinishGenerator(state); + return By_TakeReturn(returned); } /* `throw(exc)`: raise it *at the suspension point*. @@ -3538,8 +3749,9 @@ static inline PyObject *By_StepGenerator(PyObject *self, ByTagged *state, * only the resumption finishes the machine. rejecting the argument never reaches * the frame at all, and python leaves a generator resumable after a `throw` it * refused to make sense of */ -static inline PyObject *By_ThrowInto(PyObject *self, PyObject **thrown, ByTagged *state, - PyObject *exception, PyObject *(*resume)(PyObject *)) { +static inline PyObject *By_ThrowInto(PyObject *self, PyObject **thrown, PyObject **returned, + ByTagged *state, PyObject *exception, + PyObject *(*resume)(PyObject *)) { if (thrown == NULL) return NULL; PyObject *instance = NULL; if (PyExceptionInstance_Check(exception)) { @@ -3554,18 +3766,18 @@ static inline PyObject *By_ThrowInto(PyObject *self, PyObject **thrown, ByTagged PyObject *old = *thrown; *thrown = instance; Py_XDECREF(old); - return By_StepGenerator(self, state, resume); + return By_StepGenerator(self, returned, state, resume); } /* `close()`: throw `GeneratorExit` in and accept the three legal outcomes. * * exhausting, re-raising `GeneratorExit`, or being already finished are all a clean * close. *yielding* is not — cpython calls that a `RuntimeError` */ -static inline int By_CloseGenerator(PyObject *self, PyObject **thrown, ByTagged *state, - PyObject *(*resume)(PyObject *)) { +static inline int By_CloseGenerator(PyObject *self, PyObject **thrown, PyObject **returned, + ByTagged *state, PyObject *(*resume)(PyObject *)) { PyObject *exit = PyObject_CallNoArgs(PyExc_GeneratorExit); if (exit == NULL) return -1; - PyObject *result = By_ThrowInto(self, thrown, state, exit, resume); + PyObject *result = By_ThrowInto(self, thrown, returned, state, exit, resume); Py_DECREF(exit); if (result != NULL) { Py_DECREF(result); @@ -4312,6 +4524,77 @@ static inline void By_RaiseWith(PyObject *error, PyObject *value) { PyErr_SetObject(error, value); } +/* an iterator that has already failed: stepping it hands back nothing and leaves the + * pending exception exactly where it was. + * + * a frame that finishes *by raising* still has to be reported to `am_send`'s caller + * as one of the three outcomes, and a `StopIteration` among those raises is a + * *return* rather than an error. the rule for reading its value back is subtle — a + * bare one carries `None`, a subclass carries whatever its own `value` holds, a + * raised type has to be instantiated first, and a tuple must not be spread across the + * constructor — and a copy of it that drifted would be a wrong answer about what a + * frame returned. so rather than restate the rule, this asks for it: handing cpython + * an iterator that has already failed is the shape `PyIter_Send` applies the rule to, + * and the answer comes back the same as if the slot had never been there. + * + * the type is never readied and never instantiated. `PyIter_Send` reads `tp_as_async` + * and `tp_iternext` off it and nothing else, and both are what the initializer says */ +typedef struct { + PyObject_HEAD +} ByRaisedIter; + +static PyObject *By_RaisedIter_next(PyObject *self) { + (void)self; + return NULL; +} + +static PyTypeObject By_RaisedIter_Type = { + PyVarObject_HEAD_INIT(NULL, 0) + .tp_name = "by.raised", + .tp_basicsize = sizeof(ByRaisedIter), + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_iternext = By_RaisedIter_next, +}; + +static ByRaisedIter By_RaisedIter = {PyObject_HEAD_INIT(&By_RaisedIter_Type)}; + +/* one step of a resumable frame, reported the way `PyIter_Send` reports one + * + * a yielded value, a return, or a real error — and the return arrives structurally, + * out of `$returned`, which is the whole reason this slot is worth answering. that is + * the difference between a completed `await` costing an exception and costing a + * pointer read. + * + * `arg` is dispatched exactly as `PyIter_Send` would have dispatched it against a + * type with no `am_send`: `None` goes the way `tp_iternext` goes and carries nothing + * in, and anything else parks the value the suspended `yield` evaluates to, the way + * `send` does. keeping that split is what makes the slot invisible rather than a + * second set of semantics */ +static inline PySendResult By_SendGenerator(PyObject *self, PyObject **sent, + PyObject **returned, ByTagged *state, + PyObject *(*resume)(PyObject *), PyObject *arg, + PyObject **result) { + PyObject *step; + if (arg != Py_None) { + PyObject *old = *sent; + *sent = By_NewRef(arg); + Py_XDECREF(old); + } + step = resume(self); + if (step != NULL) { + *result = step; + return PYGEN_NEXT; + } + By_FinishGenerator(state); + step = *returned; + if (step != NULL) { + *returned = NULL; + *result = step; + return PYGEN_RETURN; + } + return By_IterSend((PyObject *)&By_RaisedIter, Py_None, result); +} + /* read a shared closure cell. a cell starts unset, exactly as a python cell does, * and reading one before it is written is `UnboundLocalError` rather than a zero */ static inline PyObject *By_ReadCell(PyObject *value, const char *name, int free) { diff --git a/crates/by_rt/src/lib.rs b/crates/by_rt/src/lib.rs index 32210b1036..07733aa612 100644 --- a/crates/by_rt/src/lib.rs +++ b/crates/by_rt/src/lib.rs @@ -40,6 +40,7 @@ mod tests { "By_ApplyMethodDecorators", "By_SpecClass", "By_SpecSubclass", + "By_InterpreterMatches", ] { assert!(BY_H.contains(symbol), "the runtime is missing {symbol}"); } diff --git a/crates/by_transforms/src/transforms/lazy_import.rs b/crates/by_transforms/src/transforms/lazy_import.rs index fad76c7aaf..fc173c464b 100644 --- a/crates/by_transforms/src/transforms/lazy_import.rs +++ b/crates/by_transforms/src/transforms/lazy_import.rs @@ -442,8 +442,16 @@ const LAZY_ATTR_PROXY: &str = r#"class _LazyAttr: try: v = _by_il.import_module(self._by_mod + "." + self._by_attr) except ImportError: + # worded as the import machinery words it, down to the module's + # file: a `from x import y` that fails is something programs catch + # and report, so the report must not say where the import was + # written. `name_from` is left off — cpython's own constructor + # only took it from 3.12, and this polyfill runs on 3.9 + p = getattr(m, "__file__", None) raise ImportError("cannot import name " + repr(self._by_attr) + - " from " + repr(self._by_mod), name=self._by_mod) from None + " from " + repr(self._by_mod) + + ("" if p is None else " (" + p + ")"), + name=self._by_mod, path=p) from None object.__setattr__(self, "_by_val", v) object.__setattr__(self, "_by_has", True) return self._by_val diff --git a/crates/by_transforms/src/transforms/soundness.rs b/crates/by_transforms/src/transforms/soundness.rs index f9acfe2e0f..0e4714a079 100644 --- a/crates/by_transforms/src/transforms/soundness.rs +++ b/crates/by_transforms/src/transforms/soundness.rs @@ -65,7 +65,7 @@ use std::fmt::Write as _; use ruff_python_ast::visitor::{Visitor, walk_expr, walk_stmt}; -use ruff_python_ast::{Comprehension, Expr, ExprCall, Stmt, StmtFunctionDef, UnaryOp}; +use ruff_python_ast::{Comprehension, Expr, ExprCall, Parameter, Stmt, StmtFunctionDef, UnaryOp}; use ruff_text_size::{Ranged, TextRange, TextSize}; use super::ast_driver::{Fragment, PassContext, TypeAwarePass}; @@ -230,6 +230,15 @@ impl<'a> Soundness<'a> { .filter(|plan| !matches!(plan, SoundnessCheck::Isinstance(t) if t == "type(None)")) } + /// [`Self::check_plan`] for a parameter, read off the parameter rather than off + /// its annotation — which is the only way to see a type the source stated with a + /// default instead of with an annotation + fn parameter_plan(&self, parameter: &Parameter) -> Option { + self.types + .parameter_check_plan(parameter) + .filter(|plan| !matches!(plan, SoundnessCheck::Isinstance(t) if t == "type(None)")) + } + /// wrap `source[range]` in `helper(, )`. `trailing` /// carries its own leading `, ` (e.g. `", str"` or `", A[int], (0,)"`) fn wrap_call(&mut self, range: TextRange, helper: &str, trailing: &str) { @@ -352,11 +361,18 @@ impl<'a> Soundness<'a> { } } - /// insert entry guards validating each annotated, checkable parameter of - /// `func` at the top of its body — the `parameters` position, defending - /// the contract against callers the checker never saw. variadic - /// (`*args` / `**kwargs`) and unannotated parameters, and those whose type - /// has no runtime test, are skipped + /// insert entry guards validating each checkable parameter of `func` at the + /// top of its body — the `parameters` position, defending the contract + /// against callers the checker never saw. variadic (`*args` / `**kwargs`) + /// parameters are skipped, and so is any parameter whose source states no + /// type: an unannotated one with no default states nothing, and `x=None` + /// says the argument may be left out rather than that `None` belongs there + /// + /// the plan is read off the *parameter*, not off its annotation, because a + /// default is a written type too. `def f(safe='/')` says `safe` is a `str` + /// — the native backend lays the parameter out at that bound and checks it + /// at the boundary, and asking the annotation node left the interpreted twin + /// silently more permissive than its own compiled form fn insert_param_guards(&mut self, func: &StmtFunctionDef) { let params = &func.parameters; let mut guards: Vec = Vec::new(); @@ -367,9 +383,7 @@ impl<'a> Soundness<'a> { .chain(¶ms.kwonlyargs) { let parameter = &pwd.parameter; - if let Some(annotation) = ¶meter.annotation - && let Some(plan) = self.check_plan(annotation) - { + if let Some(plan) = self.parameter_plan(parameter) { let guard = self.guard_stmt(parameter.name.as_str(), &plan); guards.push(guard); } @@ -1191,6 +1205,34 @@ mod tests { assert!(out.contains("_soundness_check(n, int)"), "got:\n{out}"); } + /// a default is a written type. `def f(safe='/')` says `safe` is a `str` as + /// plainly as an annotation would — if something else belonged there, something + /// else would be written — and the native backend already lays the parameter out + /// at that bound and refuses `f(b'x')` at its boundary. reading the plan off the + /// annotation *node* could not see it, so the interpreted twin was silently more + /// permissive than its own compiled form + #[test] + fn a_default_states_a_type_as_an_annotation_does() { + let out = check_with("def f(safe = \"/\", n = 1): ...\n", params_only()); + assert!(out.contains("_soundness_check(safe, str)"), "got:\n{out}"); + // and the literal is promoted, so it is the class rather than the value + assert!(!out.contains("Literal"), "got:\n{out}"); + assert!(out.contains("_soundness_check(n, int)"), "got:\n{out}"); + } + + /// what the source states is the *default*, not everything the bound accumulates. + /// a parameter with nothing to state gets no guard: `None` is the sentinel every + /// optional parameter is spelled with — it says the argument may be left out, not + /// that `None` is what belongs there — and a bare parameter states nothing at all + #[test] + fn a_parameter_whose_source_states_nothing_is_not_guarded() { + let out = check_with( + "def f(bare, maybe = None, *rest, **kw): ...\n", + params_only(), + ); + assert!(!out.contains("_soundness_check"), "got:\n{out}"); + } + #[test] fn user_generic_param_deep_guarded_at_entry() { let out = check_with( diff --git a/crates/by_transforms/src/type_info.rs b/crates/by_transforms/src/type_info.rs index b6f442e160..871b76ab38 100644 --- a/crates/by_transforms/src/type_info.rs +++ b/crates/by_transforms/src/type_info.rs @@ -2,7 +2,7 @@ use crate::transforms::trailing_lambda::RECEIVER_PARAMETER; use ruff_python_ast::helpers::is_dotted_name; -use ruff_python_ast::{Expr, ExprCall, ExprName, ExprRef, Stmt, StmtClassDef}; +use ruff_python_ast::{Expr, ExprCall, ExprName, ExprRef, Parameter, Stmt, StmtClassDef}; use ruff_python_parser::parse_expression; use ruff_python_stdlib::basedpython::IMPLICIT_TYPING_NAMES; use ruff_text_size::TextRange; @@ -404,6 +404,16 @@ pub(crate) trait TypeInfo { /// at module scope fn soundness_check_plan(&self, expr: &Expr) -> Option; + /// the runtime soundness check for a *parameter*, planned against the type its + /// source states rather than the one its annotation node spells + /// + /// the two differ exactly where there is no annotation to spell. `def f(safe='/')` + /// opens a hole bounded by everything the function requires of `safe`, and a hole + /// has no runtime test — so asking about the annotation answered nothing, and the + /// interpreted leg wrote no check where the native backend was already enforcing + /// the bound at its boundary + fn parameter_check_plan(&self, parameter: &Parameter) -> Option; + /// the soundness check for the parameter that the positional argument at /// `index` binds to in a call through `callee` (see /// [`soundness_check_plan`](TypeInfo::soundness_check_plan)). `None` when @@ -1034,6 +1044,16 @@ impl TypeInfo for SemanticModel<'_> { ) } + fn parameter_check_plan(&self, parameter: &Parameter) -> Option { + let ty = parameter.inferred_type(self)?; + ty_python_semantic::types::soundness::parameter_runtime_check_plan( + self.db(), + &self.program_environment(), + self.file(), + ty, + ) + } + fn call_positional_param_plan(&self, callee: &Expr, index: usize) -> Option { let ty = callee.inferred_type(self)?; ty_python_semantic::types::soundness::parameter_check_plan( diff --git a/crates/ty/src/by_commands.rs b/crates/ty/src/by_commands.rs index 2f15a62359..b01a61082d 100644 --- a/crates/ty/src/by_commands.rs +++ b/crates/ty/src/by_commands.rs @@ -387,19 +387,31 @@ fn resolved_module_name(db: &ProjectDatabase, file: ruff_db::files::File) -> Opt /// package, and its artefact is the `__init__` inside the directory. A file the /// resolver could not reach has no dotted name at all: it falls back to its stem, /// which is the only name it could be imported under, from its own directory. +/// +/// `None` where even that fallback names nothing the source meant. An +/// `__init__.py` is the body of the package its directory names, and a directory +/// the resolver could not reach names no package — `a-one/__init__.py` is the +/// plainest case, since `a-one` is not an identifier and nothing can import it. +/// Compiling such a file under its stem produces a module called `__init__`: it +/// loads, it answers `__name__ == "__init__"`, its relative imports have no +/// package to be relative to, and its submodules are bound to nothing. Two of +/// them in different directories then claim one artefact, which is how the clash +/// error was first proven reachable. A source whose own identity the artefact +/// cannot carry is declined rather than half-built. fn compiled_module_name( db: &ProjectDatabase, path: &Path, file: ruff_db::files::File, -) -> anyhow::Result { +) -> anyhow::Result> { let stem = path .file_stem() .and_then(|stem| stem.to_str()) .context("a source file has no usable module name")?; Ok(match resolved_module_name(db, file) { - Some(resolved) if stem == "__init__" => by_ir::ModuleName::package(resolved), - Some(resolved) => by_ir::ModuleName::new(resolved), - None => by_ir::ModuleName::new(stem), + Some(resolved) if stem == "__init__" => Some(by_ir::ModuleName::package(resolved)), + Some(resolved) => Some(by_ir::ModuleName::new(resolved)), + None if stem == "__init__" => None, + None => Some(by_ir::ModuleName::new(stem)), }) } @@ -637,10 +649,20 @@ pub(crate) fn cmd_compile( // what each source will be compiled as, worked out before anything is written: // two sources that land on the same artefact used to leave only the second, and // nothing said so - let mut names: Vec = Vec::with_capacity(handles.len()); + let mut planned: Vec<(&(PathBuf, ruff_db::files::File), by_ir::ModuleName)> = + Vec::with_capacity(handles.len()); let mut claimed: HashMap = HashMap::new(); - for (path, file) in &handles { - let name = compiled_module_name(&db, path, *file)?; + for handle in &handles { + let (path, file) = handle; + let Some(name) = compiled_module_name(&db, path, *file)? else { + eprintln!( + "skipping {}: it is the body of the package `{}` names, \ + and no import path reaches that directory", + path.display(), + path.parent().unwrap_or(path).display() + ); + continue; + }; // keyed on the artefact rather than on the name, because the artefact is // what would be overwritten — and a file the resolver cannot name falls // back to its stem, which two directories can share @@ -654,10 +676,10 @@ pub(crate) fn cmd_compile( name.dotted() ); } - names.push(name); + planned.push((handle, name)); } - for ((path, file), name) in handles.iter().zip(names) { + for ((path, file), name) in planned { let source = fs::read_to_string(path) .with_context(|| format!("could not read {}", path.display()))?; @@ -685,7 +707,7 @@ pub(crate) fn cmd_compile( )); let built = if emit_c_only { - by_build::emit_lowered(lowered, &source, &out_dir, &options, toolchain.version) + by_build::emit_lowered(lowered, &source, Some(&toolchain), &out_dir, &options) } else { by_build::build_lowered(lowered, &source, &toolchain, &out_dir, &options).inspect( |built| { diff --git a/crates/ty/tests/by_e2e.rs b/crates/ty/tests/by_e2e.rs index 84d61c62f6..acb56aa96c 100644 --- a/crates/ty/tests/by_e2e.rs +++ b/crates/ty/tests/by_e2e.rs @@ -221,6 +221,43 @@ fn compile_refuses_two_sources_that_would_write_the_same_artifact() { assert!(!out.join("m.c").exists(), "{stderr}"); } +#[test] +fn compile_declines_a_package_body_whose_package_has_no_importable_name() { + // an `__init__.py` is the body of the package its directory names, and `a-one` + // is not a name python can import — so there is no package for the file to be + // the body of. compiled under its stem it became a module called `__init__`, + // which loads and answers `__name__ == "__init__"`: its relative imports have + // no package to be relative to and its submodules are bound to nothing. a + // sibling that *is* nameable from its own directory still compiles, because + // its stem really is the only name it could be imported under + let dir = std::env::temp_dir().join("by_cli_unnameable_package"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("a-one")).unwrap(); + std::fs::write( + dir.join("pyproject.toml"), + "[project]\nname=\"s\"\nversion=\"0\"\nrequires-python=\">=3.13\"\n", + ) + .unwrap(); + std::fs::write(dir.join("a-one/__init__.py"), "VALUE = 1\n").unwrap(); + std::fs::write(dir.join("a-one/inner.py"), "VALUE = 2\n").unwrap(); + + let out = dir.join("o"); + let result = Command::new(env!("CARGO_BIN_EXE_by")) + .args(["compile", "-o"]) + .arg(&out) + .arg("--emit-c-only") + .current_dir(&dir) + .output() + .expect("failed to spawn by"); + let stderr = String::from_utf8_lossy(&result.stderr); + // declining one source is not a failed build — the rest of the project is + // compiled, and what was left out is said rather than silently produced + assert!(result.status.success(), "{stderr}"); + assert!(stderr.contains("skipping"), "{stderr}"); + assert!(!out.join("__init__.c").exists(), "{stderr}"); + assert!(out.join("inner.c").exists(), "{stderr}"); +} + #[test] fn compile_transpiles_the_fallback_with_the_lowering_options_it_was_given() { // a declined function *runs* from the embedded source, so `by compile` has to @@ -2795,3 +2832,98 @@ def main(): "expected the override to be reported once enabled:\n{rendered}" ); } + +#[test] +#[expect( + clippy::print_stderr, + reason = "a skipped test prints why it was skipped" +)] +fn a_lazy_from_import_resolves_a_submodule_and_refuses_a_missing_name_as_python_does() { + // `_LazyAttr` defers the attribute read, and reading an attribute is not how a + // submodule gets bound: `urllib/__init__.py` never imports `parse`, and cpython + // binds it only because `__import__` is handed a fromlist. so a transpiled + // `from urllib import parse` used to raise `AttributeError` where the same source + // run by cpython is fine — a wrong answer in shipped output rather than a decline + // + // the refusal for a name that really is missing is asserted against the + // interpreter's own, on the same interpreter, rather than against a string + // written here: a program that catches this reports it, so the report must not + // say where the import was written + let Some(python) = ["python3.13", "python3"].into_iter().find(|p| { + Command::new(p) + .arg("--version") + .output() + .is_ok_and(|o| o.status.success()) + }) else { + eprintln!("skipping: no python interpreter available"); + return; + }; + + let dir = tempfile::tempdir().expect("tempdir"); + fs::write( + dir.path().join("pyproject.toml"), + "[project]\nname=\"s\"\nversion=\"0\"\nrequires-python=\">=3.13\"\n", + ) + .unwrap(); + fs::write( + dir.path().join("main.by"), + "from urllib import parse\nfrom urllib import nosuch\n\n\n\ + def main():\n\ + \x20 print(parse.quote(\"a b\"))\n\ + \x20 try:\n\ + \x20 print(nosuch)\n\ + \x20 except ImportError as e:\n\ + \x20 print(type(e).__name__, str(e), e.name, e.path, sep=\"|\")\n", + ) + .unwrap(); + + let transpiled = Command::new(env!("CARGO_BIN_EXE_by")) + .args(["transpile", "main.by"]) + .env("PYTHON", python) + .current_dir(dir.path()) + .output() + .expect("failed to spawn by"); + assert!( + transpiled.status.success(), + "{}", + String::from_utf8_lossy(&transpiled.stderr) + ); + let program = String::from_utf8_lossy(&transpiled.stdout).into_owned(); + // the laziness is the thing under test, so its absence must not pass silently + assert!( + program.contains("_lazy_attr(\"urllib\", \"parse\")"), + "{program}" + ); + fs::write(dir.path().join("prog.py"), &program).unwrap(); + + let run = |body: &str| { + let out = Command::new(python) + .args(["-c", body]) + .current_dir(dir.path()) + .output() + .expect("the interpreter runs"); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() + }; + let mut lines = run("import prog; prog.main()") + .lines() + .map(str::to_string) + .collect::>(); + assert_eq!( + lines.first().map(String::as_str), + Some("a%20b"), + "{lines:?}" + ); + let ours = lines.pop().expect("the refusal is printed"); + + // the same import, written the way python writes it, refused by python itself + let theirs = run( + "try:\n from urllib import nosuch\nexcept ImportError as e:\n\ + \x20 print(type(e).__name__, str(e), e.name, e.path, sep='|')\n", + ); + assert_eq!(ours, theirs); +} diff --git a/crates/ty_python_semantic/resources/mdtest/basedpython_sound_types.md b/crates/ty_python_semantic/resources/mdtest/basedpython_sound_types.md index 0a12fe5103..60081dd40b 100644 --- a/crates/ty_python_semantic/resources/mdtest/basedpython_sound_types.md +++ b/crates/ty_python_semantic/resources/mdtest/basedpython_sound_types.md @@ -179,7 +179,7 @@ class A: def f(x): return x.name -reveal_type(f(A())) # revealed: object +reveal_type(f(A())) # revealed: Unknown f(A()) # ok f(1) # error: [invalid-argument-type] @@ -225,6 +225,210 @@ f(A()) # ok f(1) ``` +### a subscript + +`x[k]` is a call on `__getitem__`, which is a member like any other, so subscripting a parameter is +a requirement on the argument in the same way reading an attribute off it is + +```by +def f(s): + return s[:5] + +# revealed: def f(s: some protocol(def __getitem__(self, slice[None, 5, None], /) -> Unknown)) -> Unknown +reveal_type(f) + +f("hello") # ok +f(1) # error: [invalid-argument-type] +``` + +`x[k] = v` asks for `__setitem__` instead, taking the key and the value + +```by +def store(d): + d["k"] = 1 + +reveal_type(store) # revealed: def store(d: some protocol(def __setitem__(self, str, int, /) -> Unknown)) + +store({"k": 1}) # ok +store([]) # error: [invalid-argument-type] +``` + +### an operator + +an operator is a call on the left operand's dunder + +```by +def sub(n): + return n - 1 + +reveal_type(sub) # revealed: def sub(n: some protocol(def __sub__(self, int, /) -> Unknown)) -> Unknown + +sub(5) # ok +sub("nope") # error: [invalid-argument-type] +``` + +the unary operators and the ordering comparisons are read the same way. `==`, `!=` and the identity +tests are not, because `object` answers those itself and they require nothing + +```by +def measure(v): + return -v + +reveal_type(measure) # revealed: def measure(v: some protocol(def __neg__(self, /) -> Unknown)) -> Unknown + +measure(5) # ok +measure(None) # error: [invalid-argument-type] + +def ordered(v): + return v < 0 + +reveal_type(ordered) # revealed: def ordered(v: some protocol(def __lt__(self, int, /) -> Unknown)) -> Unknown + +ordered(5) # ok +ordered(None) # error: [invalid-argument-type] + +def identical(v): + return v == 1 + +identical(object()) # ok +``` + +the value an operator produces is not itself a value this tracks, so what the body goes on to do +with `n - 1` is no requirement on what `n`'s `__sub__` returns + +```by +def stepped(n): + return (n - 1).bit_length() + +reveal_type(stepped) # revealed: def stepped(n: some protocol(def __sub__(self, int, /) -> Unknown)) -> Unknown +``` + +### iteration + +iterating a parameter asks for `__iter__`, and for a `__next__` on whatever that hands back. the two +together are what makes an *element* a value in its own right, so what the loop body does with the +loop variable is a requirement on what the argument yields rather than on the argument + +```by +def total(xs): + for x in xs: + x.bit_length() + +# revealed: def total(xs: some protocol(def __iter__(self, /) -> protocol(def __next__(self, /) -> protocol(def bit_length(self, /) -> Unknown)))) +reveal_type(total) + +total([1, 2]) # ok +total(["a"]) # error: [invalid-argument-type] +total(1) # error: [invalid-argument-type] +``` + +a comprehension, a splatted argument and a destructuring assignment all iterate too + +```by +def comprehended(xs): + return [x for x in xs] + +def splatted(xs): + print(*xs) + +def destructured(xs): + a, b = xs + +comprehended([1]) # ok +comprehended(1) # error: [invalid-argument-type] +splatted([1]) # ok +splatted(1) # error: [invalid-argument-type] +destructured([1, 2]) # ok +destructured(1) # error: [invalid-argument-type] +``` + +### calling the parameter itself + +calling a parameter asks for a `__call__` shaped like the call, which is what a function has + +```by +def apply(fn): + return fn(1) + +reveal_type(apply) # revealed: def apply(fn: some protocol(def __call__(self, int, /) -> Unknown)) -> Unknown + +def takes_int(a: int) -> str: + return "" + +apply(takes_int) # ok +apply(1) # error: [invalid-argument-type] +``` + +### a member called twice takes both + +each call is a separate requirement and all of them have to hold, so the member has to accept every +argument any of them passed. a parameter is contravariant, so the calls combine by unioning position +by position + +```by +class Two: + def group(self, which: str) -> int: + return 1 + +class One: + def group(self, which: "indent") -> int: + return 1 + +def f(m): + m.group("indent") + m.group("source") + +reveal_type(f) # revealed: def f(m: some protocol(def group(self, str, /) -> Unknown)) + +f(Two()) # ok +f(One()) # error: [invalid-argument-type] +``` + +### calls that do not agree on their shape + +two calls of different arity need an overload, which cannot be written here. such a member degrades +to asking only that it exist and be callable + +```by +def f(m): + m.group("indent") + m.group(1, 2) + +reveal_type(f) # revealed: def f(m: some protocol(def group(self, /, *args: Any, **kwargs: Any) -> Unknown)) +``` + +### a member nothing was required of reads back gradually + +recording a requirement is about what the *call site* has to supply, and it does not change what the +body itself reads. `x[k]` and `x - 1` read as `Unknown` whether or not anything was recorded, which +is what keeps a body that used to check from acquiring errors about a value nothing learned anything +about + +a member the body *named* reads back the same way. the requirement is that the member **exist** — +nothing about it says what it holds, and `object` would not describe that value, it would forbid +every use of it. that is a stronger claim than the source made, and it travels: a member's type +becomes the recovered *return* type of the function that read it, so `config = yaml.safe_load(fp)` +came back as an `object` and `config["plugins"]` was an error no annotation could take back + +```py +def f(x): + reveal_type(x[0]) # revealed: Unknown + reveal_type(x - 1) # revealed: Unknown + reveal_type(x.name) # revealed: Unknown +``` + +### a requirement another requirement already meets is left out + +`int` has the `__mul__` the body needs, so intersecting the two would only cost the body the `int` +it could otherwise read back + +```py +def twice(n=1): + return n * 2 + +reveal_type(twice) # revealed: def twice(n: some int = 1) -> int +``` + ### a declared place says what the member's value has to be reading a member into somewhere that says what it holds is a requirement on that member, not just on @@ -468,8 +672,9 @@ g(Deep()) # error: [invalid-argument-type] ### a chain through a reassigned local says nothing -which of a name's values a later use is about is not a question this can answer, so the chain stops -there and the member's value only has to exist +which of a name's values a later use is about is not a question this can answer. so the chain stops +there: the member still has to be there, and what it hands back is a value nothing here can say +anything about ```by class A: @@ -482,7 +687,7 @@ def f(x, flag): a = 1 assert a is int -reveal_type(f) # revealed: def f(x: some protocol(def foo(self, /) -> object), flag) +reveal_type(f) # revealed: def f(x: some protocol(def foo(self, /) -> Unknown), flag) f(A(), True) # ok ``` @@ -499,7 +704,7 @@ def guarded(x): if a is int: a.bit_length() -reveal_type(guarded) # revealed: def guarded(x: some protocol(def foo(self, /) -> object)) +reveal_type(guarded) # revealed: def guarded(x: some protocol(def foo(self, /) -> Unknown)) def returned(x): a = x.foo() @@ -507,14 +712,14 @@ def returned(x): return None a.bit_length() -reveal_type(returned) # revealed: def returned(x: some protocol(def foo(self, /) -> object)) +reveal_type(returned) # revealed: def returned(x: some protocol(def foo(self, /) -> Unknown)) def branched(x, flag): a = x.foo() if flag: a.bar() -# revealed: def branched(x: some protocol(def foo(self, /) -> protocol(def bar(self, /) -> object)), flag) +# revealed: def branched(x: some protocol(def foo(self, /) -> protocol(def bar(self, /) -> Unknown)), flag) reveal_type(branched) ``` @@ -523,13 +728,268 @@ reveal_type(branched) nothing is invented from a use that was not understood, so a body keeps type-checking exactly as it did and its call sites stay unchecked +`in` is such a use where the container decides what it takes. it runs through `__contains__`, +`__iter__` *or* `__getitem__` on the right-hand operand, and asking for any one of the three would +demand something the body never needed + ```py def f(x): - return x + 1 + return 1 in x f("anything") # ok ``` +an operation whose *left* operand is not the parameter is another. python only reaches the right +operand's reflected dunder when the left operand's own returns `NotImplemented`, and which of the +two routes it takes is decided by the argument: `"%s" % attr` runs entirely through `str.__mod__`, +and requiring `attr` to have `__rmod__` would reject every `str` + +```py +def g(x): + return "%s" % x + +g("anything") # ok +``` + +### a bound has to type the body it was read off + +every requirement above is read off the body, so the body is checked against the bound they add up +to. a use of the *same* parameter that no requirement can state is what makes that fail: the bound +is built without that use and then checked against it anyway, and the function's own code stops +fitting the signature the function itself produced + +so a use like that takes the bound away rather than being passed over. reading `r.bit_length()` asks +for a protocol, and `2 * r` two lines later is exactly the operation such a protocol cannot answer, +so `r` keeps nothing + +```py +def area(r): + a = r.bit_length() + return 2 * r + +reveal_type(area) # revealed: def area(r) -> Unknown +area("anything") # ok +``` + +the same body without that line keeps everything it read + +```py +def bits(r): + a = r.bit_length() + return r + +# revealed: def bits(r: some ) -> r +reveal_type(bits) +``` + +### a use that cannot be stated only takes away what it is about + +a use of a *member's* value says nothing about the member itself. `x.foo` still has to be there and +still has to be callable the way the body called it; what it hands back is the part nothing can be +said about, so that part reads back as gradual + +```py +def held(x): + return 1 + x.foo() + +# revealed: def held(x: some ) -> Unknown +reveal_type(held) + +held(1) # error: [invalid-argument-type] +``` + +### a narrowed use takes the bound away too + +a name a test narrowed stands for something narrower than the argument, so nothing done with it is a +requirement on the argument. it is still the same argument underneath, though, and the type it now +has still mentions the hole — so the body goes on being checked against whatever bound the rest of +it recovers, through a use no requirement was ever built from. that is the failure above reached by +a route the walk does not see, and it is closed the same way + +```py +def bits(x): + x.bit_length() + if x: + return 2 * x + return 0 + +reveal_type(bits) # revealed: def bits(x) -> int +bits("anything") # ok +``` + +a member read under a narrowing goes the same way, and for a reason of its own: the branch was +written because the author meant the other one to be reachable, so requiring of every argument what +this one does would reject the very calls the test exists for + +```py +def branch(x): + x.foo() + if x: + return x.bar() + return 0 + +reveal_type(branch) # revealed: def branch(x) -> Unknown | Literal[0] +``` + +### a narrowing the bound already implies is no narrowing + +`assert isinstance(x, int)` puts `int` into the bound, and from there `x` narrowed to an `int` is +the hole itself. so the uses below such an `assert` are recorded like any other — and what they +record is checked against the very bound the `assert` put there, which is what makes recording them +safe + +```py +def f(x): + assert isinstance(x, int) + return x + 1 + +reveal_type(f) # revealed: def f(x: some int) -> int + +f(1) # ok +f("a") # error: [invalid-argument-type] +``` + +### a narrowing costs only a bound the body itself recovered + +what it takes away is what the walk read off the body, so where the walk read nothing there is +nothing for the narrowed use to fail against and nothing to take. that is what lets an `assert` be +read from inside its own test: `isinstance(proto, int)` narrows `proto` for the arm beside it, and +nothing else in this body asks anything of `proto` at all + +```py +def opcode(proto): + assert isinstance(proto, int) and proto <= 5 + +reveal_type(opcode) # revealed: def opcode(proto: some int) + +opcode(1) # ok +opcode("a") # error: [invalid-argument-type] +``` + +### the uses that cannot be stated + +each of these asks something of the parameter that no requirement here can write down, so each one +leaves the parameter gradual however much else its body said + +writing a member, rather than reading one: + +```py +def written(x): + x.foo() + x.other = 1 + +reveal_type(written) # revealed: def written(x) +``` + +a call whose arguments are splatted, since no fixed parameter list says how many of them there are: + +```py +def spread(x, args): + x.foo(*args) + x.bar() + +# revealed: def spread(x, args: some ) +reveal_type(spread) +``` + +and the statements that ask for a shape of their own — `raise` for a `BaseException`, `with` for a +pair of context-manager methods: + +```py +def thrown(x): + x.foo() + raise x + +reveal_type(thrown) # revealed: def thrown(x) -> Never + +def entered(x): + x.foo() + with x: + pass + +reveal_type(entered) # revealed: def entered(x) +``` + +### a position that takes anything asks nothing + +the other side of the same rule: where a position accepts `object` it accepts whatever bound the +rest of the body recovers, so the body goes on checking and the bound stays + +that covers a great deal of ordinary python — a value printed or formatted, one read for its truth, +a key looked up in a mapping, an argument to an overloaded callee every reading of which takes +anything + +```py +def shown(x): + x.foo() + print(x) + print(f"{x}") + print(str(x)) + return x or 0 + +# revealed: def shown(x: some ) -> (x & ~AlwaysFalsy) | Literal[0] +reveal_type(shown) + +def keyed(x, d: dict[str, int]): + x.foo() + return d[x] if x in d else 0 + +# revealed: def keyed(x: some , d: dict[str, int]) -> int +reveal_type(keyed) +``` + +`"%s" % x` is the same thing read through an operator: `str.__mod__` takes anything, so the operand +written on its right is asked nothing and keeps whatever else the body said about it + +```py +def formatted(x): + x.foo() + return "%s" % x + +# revealed: def formatted(x: some ) -> str +reveal_type(formatted) +``` + +a narrowed value is still a value, and a position like this takes one of those too, so a narrowing +whose uses all sit in positions like this costs nothing + +```py +def guarded(x): + x.foo() + if x: + print(x) + print(f"{x}") + print("%s" % x) + return 0 + +# revealed: def guarded(x: some ) -> Literal[0] +reveal_type(guarded) +``` + +### a place that says what it holds says it about the parameter too + +reading a *member* into a declared place constrains that member. reading the parameter itself into +one constrains the parameter, for the same reason and by the same rule + +```py +def stored(x): + a: int = x + return a + +reveal_type(stored) # revealed: def stored(x: some int) -> x + +stored("no") # error: [invalid-argument-type] +``` + +```py +def returned(x) -> str: + return x + +reveal_type(returned) # revealed: def returned(x: some str) -> str + +returned(1) # error: [invalid-argument-type] +``` + ### an operation on a hole nothing bounded answers gradually a hole nothing bounded is the gradual type it replaced, so it proves no more than that type did. @@ -850,7 +1310,7 @@ def f(x, y: object): if hasattr(y, "a"): x.b(y) -# revealed: def f(x: some protocol(def b(self, protocol(a: object), /) -> object), y: object) +# revealed: def f(x: some protocol(def b(self, protocol(a: object), /) -> Unknown), y: object) reveal_type(f) ``` @@ -862,7 +1322,7 @@ def f(x, y: object): case [_]: x.b(y) -# revealed: def f(x: some protocol(def b(self, Sequence[object] & protocol(def __getitem__(self, index: 0, /) -> object; def __len__(self, /) -> 1 | True) & not str & not bytes & not bytearray, /) -> object), y: object) +# revealed: def f(x: some protocol(def b(self, Sequence[object] & protocol(def __getitem__(self, index: 0, /) -> object; def __len__(self, /) -> 1 | True) & not str & not bytes & not bytearray, /) -> Unknown), y: object) reveal_type(f) ``` @@ -878,7 +1338,7 @@ def f(x, y: T | dict[str, int]): if "k" in y: x.b(y) -# revealed: def f(x: some protocol(def b(self, T | (dict[str, int] & protocol(def __contains__(self, key: "k", /) -> True)), /) -> object), y: T | dict[str, int]) +# revealed: def f(x: some protocol(def b(self, T | (dict[str, int] & protocol(def __contains__(self, key: "k", /) -> True)), /) -> Unknown), y: T | dict[str, int]) reveal_type(f) ``` diff --git a/crates/ty_python_semantic/resources/mdtest/cycle.md b/crates/ty_python_semantic/resources/mdtest/cycle.md index 1927cc9026..0e8ac85b88 100644 --- a/crates/ty_python_semantic/resources/mdtest/cycle.md +++ b/crates/ty_python_semantic/resources/mdtest/cycle.md @@ -870,6 +870,25 @@ def run(): reveal_type(t) # revealed: float ``` +## a float literal a recursive function recomputes from itself through a generic call + +A loop is not the only thing that can recompute a value from itself. `rec` calls itself to get the +value it doubles, so its inferred return type grows the same way — but there is no loop header here, +and the only union that grows is the one the generic call's solve builds out of `min`'s arguments. + +The solve collects one lower bound per element, so the union `rec` returned is taken apart before +the solve ever builds anything. Whatever recorded that the union was defined in terms of itself has +to survive being taken apart, or the union the solve builds back up has no reason to give its +literals up and grows one element per round with no fixed point. + +```by +def rec(): + return min(0.1, rec() * 2) + +def check(): + reveal_type(rec()) # revealed: float +``` + ## a complex literal a loop recomputes from itself Complex literals fold the same way and are held the same way, so they need the same bound. diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index e99c2868f4..808f5bcf77 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -1778,7 +1778,7 @@ impl<'db> Type<'db> { ) } - pub(crate) const fn is_never(&self) -> bool { + pub const fn is_never(&self) -> bool { matches!( self, Type::Never @@ -2897,7 +2897,7 @@ impl<'db> Type<'db> { } /// Detects types which are valid to appear inside a `Literal[…]` type annotation. - fn is_literal_or_union_of_literals( + pub(crate) fn is_literal_or_union_of_literals( &self, db: &'db dyn Db, env: &ProgramEnvironment<'db>, diff --git a/crates/ty_python_semantic/src/types/inferred_signature.rs b/crates/ty_python_semantic/src/types/inferred_signature.rs index d5c8a1ad58..51b2884b62 100644 --- a/crates/ty_python_semantic/src/types/inferred_signature.rs +++ b/crates/ty_python_semantic/src/types/inferred_signature.rs @@ -20,6 +20,7 @@ use ruff_db::parsed::parsed_module; use ruff_python_ast::name::Name; use ruff_python_ast::visitor::{Visitor, walk_expr, walk_stmt}; use ruff_python_ast::{self as ast, Expr, Stmt}; +use ruff_text_size::{Ranged, TextRange}; use rustc_hash::{FxHashMap, FxHashSet}; use ty_python_core::ast_ids::HasScopedUseId; use ty_python_core::definition::{Definition, DefinitionKind, ParameterDefinitionNodeKind}; @@ -38,7 +39,7 @@ use crate::types::constraints::{ConstraintSetBuilder, max_constructor_and_typeva use crate::types::function::OverloadLiteral; use crate::types::narrow::{NarrowingConstraint, infer_narrowing_constraints}; use crate::types::protocol_class::InlineProtocolMember; -use crate::types::signatures::{Parameter, Parameters, Signature}; +use crate::types::signatures::{Parameter, ParameterKind, Parameters, Signature}; use crate::types::typevar::{ TypeVarBoundOrConstraintsEvaluation, TypeVarDefaultEvaluation, TypeVarIdentity, TypeVarInstance, TypeVarKind, @@ -289,6 +290,31 @@ pub(crate) fn inferred_parameter_default<'db>( ) } +/// The part of a parameter's bound the *source states*, as opposed to the part its body +/// samples. +/// +/// A default is a written type. `def f(safe='/')` says `safe` is a `str` as plainly as an +/// annotation would — if something else belonged there, something else would be written — +/// which is why its requirement is [`WhenContradicted::Stands`] and survives a body that +/// contradicts it. Everything else in the bound is recovered from how the body happens to +/// use the parameter, and a recovered requirement can be withdrawn. +/// +/// `None` where the source states nothing: no default at all, or the `None` sentinel every +/// optional parameter is spelled with — that says the argument may be left out, not that +/// `None` is the kind of thing that belongs there. Bounding by it would reject every call +/// that supplies one, which is what `def f(x=None)` exists for. +/// +/// The literal is promoted, so `safe='/'` states `str` rather than `Literal['/']`. +pub(crate) fn stated_parameter_bound<'db>( + db: &'db dyn Db, + parameter: Definition<'db>, +) -> Option> { + let env = &ProgramEnvironment::from_definition(parameter); + inferred_parameter_default(db, parameter) + .filter(|default| !default.is_none(db)) + .map(|default| default.promote(db, env)) +} + /// Everything the function requires of `parameter`, as the upper bound of the hole /// it opens. /// @@ -315,25 +341,27 @@ pub(crate) fn inferred_parameter_bound<'db>( parameter: Definition<'db>, ) -> Type<'db> { let env = &ProgramEnvironment::from_definition(parameter); - // `None` is the sentinel every optional parameter is spelled with — it says the argument - // may be left out, not that `None` is the kind of thing that belongs there. bounding by it - // would reject every call that supplies one, which is what `def f(x=None)` exists for - let from_default = inferred_parameter_default(db, parameter) - .filter(|default| !default.is_none(db)) - .map(|default| default.promote(db, env)); + let from_default = stated_parameter_bound(db, parameter); let from_body = parameter_function_definition(db, parameter) .map(|function| body_parameter_constraints(db, function).get(parameter)) .unwrap_or_default(); + let constraints: Vec> = from_default + .map(|default| Requirement { + ty: default, + when_contradicted: WhenContradicted::Stands, + }) + .into_iter() + .chain(from_body) + .collect(); + if constraints.is_empty() { + return Type::unknown(); + } + let mut bound = IntersectionBuilder::new(db, env); - let mut constrained = false; - for constraint in from_default.into_iter().chain(from_body) { - constrained = true; + for constraint in settled_requirements(db, env, &constraints) { bound = bound.add_positive(constraint); } - if !constrained { - return Type::unknown(); - } let bound = bound.build(); // requirements that cannot all hold are the function's own problem. a bound of `Never` @@ -345,6 +373,97 @@ pub(crate) fn inferred_parameter_bound<'db>( bound } +/// One thing a parameter's bound has to say, and what becomes of it when something the program +/// states rules it out. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize, salsa::SalsaValue)] +struct Requirement<'db> { + ty: Type<'db>, + when_contradicted: WhenContradicted<'db>, +} + +/// What becomes of a requirement that something the program states rules out. +/// +/// Only a requirement the body reached through *syntax* — `x - 1`, `x[k]`, `for _ in x` — gives +/// way. Such an operation used to be read against whatever else bounded the parameter, and +/// reported there when it did not fit: `def g(x=0): x + "foo"` is an error in `g`'s own body. +/// Recording the operation as a requirement instead moves that error to every call site — +/// including `g(0)`, which passes the very default the bound came from — and leaves the mistake +/// itself unreported. So where something the program states rules the operation out, the +/// statement wins and the operation is checked against it as before. +/// +/// Another *recovered* requirement never rules one out this way. Two of those are two things the +/// same body asked for, and neither was ever going to imply the other. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize, salsa::SalsaValue)] +enum WhenContradicted<'db> { + /// it stands, and the argument has to meet it as well. `def f(x="s"): x.extra()` asks for a + /// `str` that also has an `extra`, which is a `str` subclass and is what the body plainly + /// means + Stands, + /// it falls back to what the body asked for by *name*, which is everything it asked for + /// except the operations + Reduces(Type<'db>), + /// it goes entirely, because the operations were all it asked for + Goes, +} + +/// The requirements in `constraints` that still say something. +/// +/// A requirement some other requirement implies adds nothing to the bound, and leaving it in +/// costs precision rather than buying any: `def twice(n=1): return n * 2` learns `int` from the +/// default and `__mul__` from the body, and every `int` has that `__mul__`, but an intersection +/// of the two is a type nothing resolves an operator through, so the body stops reading `int` +/// back out of `n * 2`. +/// +/// Implication is *assignability* and not subtyping, because a requirement this analysis wrote +/// down for an operator returns `Unknown` — a gradual type is assignable from anything and a +/// subtype of nothing, so subtyping would find no redundancy at all here. +/// +/// Two requirements that imply each other are the same requirement twice, and the first of them +/// is the one that stays. A requirement something stated rules out is first cut down to what +/// [`WhenContradicted`] says is left of it. +fn settled_requirements<'db>( + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + constraints: &[Requirement<'db>], +) -> Vec> { + let contradicted = |requirement: &Requirement<'db>| { + constraints.iter().any(|other| { + !other.ty.mentions_recovered_protocol(db, env) + && !other.ty.is_assignable_to(db, env, requirement.ty) + }) + }; + + let kept: Vec> = constraints + .iter() + .filter_map(|requirement| match requirement.when_contradicted { + WhenContradicted::Stands => Some(requirement.ty), + _ if !contradicted(requirement) => Some(requirement.ty), + WhenContradicted::Reduces(reduced) => Some(reduced), + WhenContradicted::Goes => None, + }) + .collect(); + + kept.iter() + .enumerate() + .filter(|(index, constraint)| { + !kept + .iter() + .enumerate() + .any(|(other_index, other)| match other_index.cmp(index) { + std::cmp::Ordering::Equal => false, + std::cmp::Ordering::Less => other.is_assignable_to(db, env, **constraint), + // a later requirement only absorbs an earlier one it is strictly stronger + // than, so two that imply each other do not absorb each other away + std::cmp::Ordering::Greater => { + other.is_assignable_to(db, env, **constraint) + && !constraint.is_assignable_to(db, env, *other) + } + }) + }) + .map(|(_, constraint)| *constraint) + .collect() +} + /// What each of a function's unannotated parameters must be, according to its body. /// /// Every parameter is answered in one pass, because the expensive half — inferring @@ -400,6 +519,18 @@ pub(crate) fn body_parameter_constraints<'db>( .map(|symbol| symbol.name().clone()) .collect(); + let parameters: BTreeMap> = node + .parameters + .iter_non_variadic_params() + .filter(|parameter| parameter.parameter.annotation().is_none()) + .map(|parameter| { + ( + parameter.parameter.name.id.clone(), + index.expect_single_definition(¶meter.parameter), + ) + }) + .collect(); + let mut collector = UseCollector { db, env: env.clone(), @@ -407,9 +538,11 @@ pub(crate) fn body_parameter_constraints<'db>( use_def: use_def_map(db, body_scope), expression_type: |expr: &Expr| inference.expression_type(expr), uses: FxHashMap::default(), - sink: None, + sinks: FxHashMap::default(), declared_return, + parameters, locals: BTreeMap::default(), + narrowed: FxHashSet::default(), captured: FxHashSet::default(), single_bindings, in_nested_scope: false, @@ -427,7 +560,29 @@ pub(crate) fn body_parameter_constraints<'db>( &mut uses, ); - let mut entries = path_bounds(db, env, uses); + let mut unstatable = Vec::new(); + let mut entries = path_bounds(db, env, uses, &mut unstatable); + + // a use through a name a test narrowed is not a use of the argument, and no requirement was + // built from it — but the narrowed name still carries the hole, so the bound goes on being + // checked against it. that is the same thing a use nothing can be said about does, reached by + // a route the walk does not see, so it costs the same thing. + // + // it costs it only where there is a *recovered* bound to lose. the bound and the body are + // settled by running them against each other, and a round that has read nothing off the body + // yet has nothing for such a use to fail against — while answering otherwise would take a + // bound away on the strength of that round and never give it back, since a parameter never + // becomes statable again. it is also what leaves an `assert` readable from inside its own + // test: `assert isinstance(x, int) and x <= 5` narrows `x` for its second arm, and the round + // that reads that arm is the round `int` is already `x`'s bound, where the narrowing leaves + // the hole itself and the use is recorded like any other + unstatable.extend( + entries + .iter() + .map(|(parameter, _)| *parameter) + .filter(|parameter| collector.narrowed.contains(parameter)), + ); + entries.extend(asserted_parameter_types(db, env, index, body_scope, node)); // a parameter a nested scope captured keeps nothing: that body is checked against this @@ -453,26 +608,36 @@ pub(crate) fn body_parameter_constraints<'db>( }); entries.sort_by_key(|(parameter, _)| *parameter); + entries.retain(|(parameter, _)| !unstatable.contains(parameter)); + unstatable.sort(); + unstatable.dedup(); ParameterConstraints { entries: entries.into_boxed_slice(), + unstatable: unstatable.into_boxed_slice(), } } /// The bound each parameter's body contributes, keyed by the parameter's definition. #[derive(Debug, Clone, PartialEq, Eq, Hash, Default, get_size2::GetSize, salsa::SalsaValue)] pub(crate) struct ParameterConstraints<'db> { - entries: Box<[(Definition<'db>, Type<'db>)]>, + entries: Box<[(Definition<'db>, Requirement<'db>)]>, + /// the parameters the body used in a way no requirement can state, which therefore keep no + /// bound however much else the body said about them + unstatable: Box<[Definition<'db>]>, } impl<'db> ParameterConstraints<'db> { /// Every bound recorded for `parameter`. A parameter can appear more than once — an /// `assert` and a use are separate requirements that both have to hold. - fn get(&self, parameter: Definition<'db>) -> Vec> { + fn get(&self, parameter: Definition<'db>) -> Vec> { + if self.unstatable.contains(¶meter) { + return Vec::new(); + } self.entries .iter() .filter(|(key, _)| *key == parameter) - .map(|(_, ty)| *ty) + .map(|(_, requirement)| *requirement) .collect() } @@ -490,27 +655,40 @@ impl<'db> ParameterConstraints<'db> { /// the iteration has no fixed point to reach. /// /// Requirements only ever being added is what leaves it one. - fn keeping_requirements_seen(mut self, previous: &Self) -> Self { - let dropped: Vec<_> = previous + /// + /// A parameter found to be unstatable is the one thing that goes the other way, and it has to: + /// a use this analysis cannot state is a fact about the body in exactly the way a requirement + /// is, and the rule above would otherwise resurrect the very bound that use rules out. So the + /// unstatable parameters accumulate too, and take their requirements with them — which is + /// still a monotone iteration, because a parameter never becomes statable again. + fn keeping_requirements_seen(self, previous: &Self) -> Self { + let mut unstatable: Vec> = self + .unstatable + .iter() + .chain(&previous.unstatable) + .copied() + .collect(); + unstatable.sort(); + unstatable.dedup(); + + let mut entries: Vec<_> = self .entries .iter() - .filter(|(parameter, _)| { + .chain(previous.entries.iter().filter(|(parameter, _)| { !self .entries .iter() .any(|(answered, _)| answered == parameter) - }) + })) + .filter(|(parameter, _)| !unstatable.contains(parameter)) .copied() .collect(); - if dropped.is_empty() { - return self; - } - - let mut entries = self.entries.into_vec(); - entries.extend(dropped); entries.sort_by_key(|(parameter, _)| *parameter); - self.entries = entries.into_boxed_slice(); - self + + Self { + entries: entries.into_boxed_slice(), + unstatable: unstatable.into_boxed_slice(), + } } } @@ -586,7 +764,7 @@ fn asserted_parameter_types<'db>( index: &ty_python_core::SemanticIndex<'db>, body_scope: ScopeId<'db>, node: &ast::StmtFunctionDef, -) -> Vec<(Definition<'db>, Type<'db>)> { +) -> Vec<(Definition<'db>, Requirement<'db>)> { let place_table = index.place_table(body_scope.file_scope_id(db)); let parameters: Vec<(ScopedPlaceId, Definition<'db>)> = node .parameters @@ -623,7 +801,13 @@ fn asserted_parameter_types<'db>( .merge_constraint_and(constraint) .evaluate_constraint_type(db, env); if !narrowed.is_object() && !narrowed.is_never() { - asserted.push((*definition, narrowed)); + asserted.push(( + *definition, + Requirement { + ty: narrowed, + when_contradicted: WhenContradicted::Stands, + }, + )); } } } @@ -666,13 +850,127 @@ impl<'db> MemberPath<'db> { /// What the value at one path was used for. #[derive(Default)] struct PathUses<'db> { - /// members read off it, and how each was called — as the parameters of the first call, since - /// a name called two different ways would need an overloaded member, which cannot be written - /// here. requiring only the first shape under-constrains, which is the safe direction - members: BTreeMap>>, + /// members read off it, and what the body did with each + members: BTreeMap>, /// declared types the value itself had to fit: a place it was read into, or a parameter it /// was forwarded to value: Vec>, + /// whether the body did something with this value that no requirement can state + unstatable: bool, +} + +/// What one member of the value at a path was used for. +#[derive(Default)] +struct MemberUses<'db> { + /// the parameters of every call the body made through this member. a member nothing called + /// has none, and is asked for as a plain attribute + calls: Vec>, + /// how the body got to the member, which decides what its value reads back as when nothing + /// in the body says what that value is + reach: MemberReach, +} + +/// How a body reached a member: by writing its name, or through the syntax the member is the +/// meaning of. +/// +/// This decides only what the member's value reads back as when nothing else says. A member the +/// body *named* reads back as `object`: the requirement is that it exist, and `object` is what a +/// value nothing was required of has to be. +/// +/// A member reached through syntax reads back as `Unknown` instead. Recording a requirement is +/// about what the *call site* has to supply, and it should not change what the body itself reads. +/// `x[k]`, `x - 1` and the element of `for _ in x` all read as `Unknown` before any of this, and +/// answering `object` there hands a body that used to check a run of errors about a value the +/// analysis never learned anything about. +#[derive(Copy, Clone, Default, PartialEq, Eq)] +enum MemberReach { + /// `x.foo`, `x.foo()` + ByName, + /// `x[k]`, `x - 1`, `-x`, `x(1)`, `for _ in x` + #[default] + ThroughSyntax, +} + +/// The one call shape a member has to fit, given every call the body made through it. +/// +/// Each call is a separate requirement and all of them have to hold, so the member has to accept +/// every argument any of them passed: a parameter is contravariant, so the shapes combine by +/// unioning position by position. `m.group("a")` and `m.group("b")` ask for a `group` that takes +/// `"a" | "b"`, which is what a real `group` accepts and what pinning the member to the first +/// call site rejected. +/// +/// Calls that disagree about their *shape* — a different arity, a different keyword — cannot be +/// written as one signature at all; that needs an overload, which nothing here can spell. Such a +/// member degrades to the gradual form, which keeps the honest half of the requirement (the +/// member exists and is callable) without inventing a shape no argument could match. +fn merged_call_shape<'db>( + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + shapes: &[Parameters<'db>], +) -> Parameters<'db> { + let mut merged: Option> = None; + for shape in shapes { + merged = Some(match merged { + None => shape.clone(), + Some(merged) => match merged_parameters(db, env, &merged, shape) { + Some(merged) => merged, + None => return callable_any_way(), + }, + }); + } + merged.unwrap_or_else(callable_any_way) +} + +/// The shape of a member that only has to be callable — `(self, *args, **kwargs)`. +fn callable_any_way<'db>() -> Parameters<'db> { + Parameters::standard([ + Parameter::positional_only(Some(Name::new_static("self"))), + Parameter::variadic(Name::new_static("args")).with_annotated_type(Type::any()), + Parameter::keyword_variadic(Name::new_static("kwargs")).with_annotated_type(Type::any()), + ]) +} + +/// Two call shapes combined, or `None` when they do not line up. +fn merged_parameters<'db>( + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + left: &Parameters<'db>, + right: &Parameters<'db>, +) -> Option> { + if left.len() != right.len() { + return None; + } + let mut merged = Vec::with_capacity(left.len()); + for (left, right) in left.iter().zip(right.iter()) { + let combined = match (left.kind(), right.kind()) { + (ParameterKind::PositionalOnly { name, .. }, ParameterKind::PositionalOnly { .. }) => { + Parameter::positional_only(name.clone()) + } + ( + ParameterKind::KeywordOnly { + name: left_name, .. + }, + ParameterKind::KeywordOnly { + name: right_name, .. + }, + ) if left_name == right_name => Parameter::keyword_only(left_name.clone()), + _ => return None, + }; + // an argument whose type could not be written down leaves its position unannotated, + // which already accepts anything the other call passed there + merged.push( + if left.should_annotation_be_displayed() && right.should_annotation_be_displayed() { + combined.with_annotated_type(UnionType::from_elements( + db, + env, + [left.annotated_type(), right.annotated_type()], + )) + } else { + combined + }, + ); + } + Some(Parameters::standard(merged)) } /// The bound each parameter's uses add up to. @@ -681,60 +979,167 @@ struct PathUses<'db> { /// is already a type by the time the member that produced it is written down. A path with both a /// value requirement and members of its own intersects them, the same way the parameter's own /// bound intersects its protocol with the types it was forwarded into. +/// +/// A path the body used in a way no requirement can state resolves to the gradual type instead of +/// to what its other uses added up to. That is what keeps a recovered signature able to type the +/// body it was recovered from: a bound is checked against *every* use, including the ones it could +/// not be built from, and a gradual type is the only one all of them are guaranteed to pass. For +/// the parameter's own path that means it keeps no bound at all; for a member's it means the member +/// still has to exist and be callable the way the body called it, but nothing is claimed about the +/// value it hands back. fn path_bounds<'db>( db: &'db dyn Db, env: &ProgramEnvironment<'db>, uses: FxHashMap, PathUses<'db>>, -) -> Vec<(Definition<'db>, Type<'db>)> { + unstatable: &mut Vec>, +) -> Vec<(Definition<'db>, Requirement<'db>)> { let mut uses: Vec<_> = uses.into_iter().collect(); uses.sort_by_key(|(path, _)| std::cmp::Reverse(path.members.len())); let mut resolved: FxHashMap, Type<'db>> = FxHashMap::default(); let mut entries = Vec::new(); for (path, path_uses) in uses { - let mut bound = IntersectionBuilder::new(db, env); - let mut constrained = false; - - if !path_uses.members.is_empty() { - let members: Vec<_> = path_uses - .members - .into_iter() - .map(|(name, called)| { - // `object` is what a value nothing required anything of has to be: the - // member only has to exist - let value = resolved - .get(&path.member(&name)) - .copied() - .unwrap_or_else(Type::object); - let member = match called { - Some(parameters) => InlineProtocolMember::Method( - CallableType::function_like(db, Signature::new(parameters, value)), - ), - None => InlineProtocolMember::ReadOnlyAttribute(value), - }; - (name, member) - }) - .collect(); - bound = bound.add_positive(Type::recovered_protocol(db, env, members)); - constrained = true; + if path_uses.unstatable { + if path.members.is_empty() { + unstatable.push(path.parameter); + } + resolved.insert(path, Type::unknown()); + continue; } - for value in path_uses.value { - bound = bound.add_positive(value); - constrained = true; + + // the members the body named and the members it reached through syntax go into one + // protocol, but they are kept apart on the way there: what becomes of the requirement + // when the rest of the bound rules it out depends on which of the two it is + let mut named = Vec::new(); + let mut through_syntax = Vec::new(); + for (name, uses) in path_uses.members { + // a member the body never constrained is one nothing is known about, and the + // way to say that is a gradual type. `object` would be a stronger claim than + // the source made — it does not describe the value, it forbids every use of + // it, and that travels: the member's type becomes the function's own return + // type, so an unannotated helper's result reached callers unusable. reading + // `yaml.safe_load(fp)` as `object` made `config["plugins"]` an error, and no + // annotation could take it back — `object` is not assignable to `dict` + let value = resolved + .get(&path.member(&name)) + .copied() + .unwrap_or_else(Type::unknown); + let member = if uses.calls.is_empty() { + InlineProtocolMember::ReadOnlyAttribute(value) + } else { + InlineProtocolMember::Method(CallableType::function_like( + db, + Signature::new(merged_call_shape(db, env, &uses.calls), value), + )) + }; + match uses.reach { + MemberReach::ByName => named.push((name, member)), + MemberReach::ThroughSyntax => through_syntax.push((name, member)), + } } - if !constrained { + + // a path with both a value requirement and members of its own intersects them, the same + // way the parameter's own bound intersects its protocol with the types it was forwarded + // into + let asking_for = |members: Vec<(Name, InlineProtocolMember<'db>)>| { + if members.is_empty() && path_uses.value.is_empty() { + return None; + } + let mut bound = IntersectionBuilder::new(db, env); + if !members.is_empty() { + bound = bound.add_positive(Type::recovered_protocol(db, env, members)); + } + for value in &path_uses.value { + bound = bound.add_positive(*value); + } + Some(bound.build()) + }; + + let when_contradicted = if through_syntax.is_empty() { + WhenContradicted::Stands + } else { + match asking_for(named.clone()) { + Some(named_only) => WhenContradicted::Reduces(named_only), + None => WhenContradicted::Goes, + } + }; + let Some(bound) = asking_for(named.into_iter().chain(through_syntax).collect()) else { continue; - } + }; - let bound = bound.build(); if path.members.is_empty() { - entries.push((path.parameter, bound)); + entries.push(( + path.parameter, + Requirement { + ty: bound, + when_contradicted, + }, + )); } resolved.insert(path, bound); } entries } +/// The `for` clauses of a comprehension. +fn comprehension_generators(expr: &Expr) -> &[ast::Comprehension] { + match expr { + Expr::ListComp(comprehension) => &comprehension.generators, + Expr::SetComp(comprehension) => &comprehension.generators, + Expr::DictComp(comprehension) => &comprehension.generators, + Expr::Generator(comprehension) => &comprehension.generators, + _ => &[], + } +} + +/// The dunder a binary operator dispatches to, for the operators that exist at runtime. +/// +/// basedpython's own `??` and `?` are written as binary operators but are not dispatched through +/// a method, so a body that uses one requires nothing of its operands. +fn runtime_dunder(op: ast::Operator) -> Option<&'static str> { + match op { + ast::Operator::Coalesce | ast::Operator::Result => None, + op => Some(op.dunder()), + } +} + +/// The dunder a unary operator dispatches to. +/// +/// `not x` is missing because it dispatches to `__bool__`, which every object has, so it is no +/// requirement at all. basedpython's postfix operators are not dispatched through a method. +fn unary_dunder(op: ast::UnaryOp) -> Option<&'static str> { + match op { + ast::UnaryOp::Invert => Some("__invert__"), + ast::UnaryOp::UAdd => Some("__pos__"), + ast::UnaryOp::USub => Some("__neg__"), + ast::UnaryOp::Not + | ast::UnaryOp::Optional + | ast::UnaryOp::Propagate + | ast::UnaryOp::Force => None, + } +} + +/// The dunder a comparison dispatches to, for the comparisons that require anything. +/// +/// `==`, `!=` and the identity tests are answered by `object` itself, so they are no requirement. +/// `in` is left out for the opposite reason: it can run through `__contains__`, `__iter__` *or* +/// `__getitem__` on the right operand, and asking for any one of the three would demand something +/// the body never needed. That disjunction is not a shape a protocol member can state. +fn comparison_dunder(op: ast::CmpOp) -> Option<&'static str> { + match op { + ast::CmpOp::Lt => Some("__lt__"), + ast::CmpOp::LtE => Some("__le__"), + ast::CmpOp::Gt => Some("__gt__"), + ast::CmpOp::GtE => Some("__ge__"), + ast::CmpOp::Eq + | ast::CmpOp::NotEq + | ast::CmpOp::Is + | ast::CmpOp::IsNot + | ast::CmpOp::In + | ast::CmpOp::NotIn => None, + } +} + /// The name a parameter definition binds. fn parameter_definition_name<'db>(db: &'db dyn Db, parameter: Definition<'db>) -> Option { let DefinitionKind::Parameter(ParameterDefinitionNodeKind::Parameter(node)) = @@ -766,6 +1171,23 @@ fn record_captured_names_in_expr(expr: &Expr, into: &mut FxHashSet) { walk_expr(&mut CapturedNames(into), expr); } +/// What the place a value is being read into says the value has to be. +/// +/// Every position a tracked value can be written in gets one of these from the construct that +/// wrote it there, and a position that gets none is one this analysis did not account for. That is +/// the whole of how the walk stays honest: it is [`UseCollector::visit_expr`]'s *default* to treat +/// a tracked value as used in a way it cannot state, so a construct nobody has taught it about +/// costs a bound rather than inventing a requirement that does not hold. +#[derive(Debug, Clone, Copy)] +enum Sink<'db> { + /// the place takes whatever it is handed — an expression statement's value, a `bool` test, an + /// argument to a parameter typed `object` — so nothing about it has to be recorded for the + /// body to go on checking + Anything, + /// the place says what it holds, and says it in something a call site could be asked for + Required(Type<'db>), +} + /// Reads a body for what it does with each parameter hole, and with each value reached from one. /// /// A use of the parameter itself is recognised by the *type* of the expression it is made on, not @@ -773,6 +1195,13 @@ fn record_captured_names_in_expr(expr: &Expr, into: &mut FxHashSet) { /// hole's type, and nothing it is then used for is a requirement on the argument. A local that a /// value was assigned to has no such type of its own, so the same two questions — is this still /// that value, and is it narrowed — are asked of its bindings instead. +/// +/// The walk is *fail-closed*. Each construct declares, for every sub-expression it is made of, what +/// that position asks of the value written there — a [`Sink`] — and a sub-expression left without +/// one is taken to be a use that cannot be stated. So the requirements a bound is built from are +/// exactly the uses the walk understood, and every use it did not understand takes the bound away +/// instead of being silently passed over. That is what makes the bound one the body itself still +/// type-checks against, which is the only kind of bound worth recovering. struct UseCollector<'db, F> { db: &'db dyn Db, env: ProgramEnvironment<'db>, @@ -780,15 +1209,21 @@ struct UseCollector<'db, F> { use_def: &'db UseDefMap<'db>, expression_type: F, uses: FxHashMap, PathUses<'db>>, - /// the declared type the expression about to be visited has to fit, when it is being - /// read into a place that has one - sink: Option>, + /// what each sub-expression the walk has not reached yet is being read into, keyed by its + /// range. a construct fills this in for its own parts before walking them + sinks: FxHashMap>, /// the function's declared return type, when it wrote one down. an *inferred* return /// type is no constraint on the body — it is read off it declared_return: Option>, + /// the function's own unannotated parameters, by the name the body writes them under. a name + /// that is one of these is the only one a *narrowing* of the argument can be written on + parameters: BTreeMap>, /// locals a value at some path was assigned to, so that what is done with the local — and a /// later `assert` about it — reads back as a requirement on that value locals: BTreeMap>, + /// the parameters the body used through a name a test had narrowed, which is a use no + /// requirement was built from and the bound is checked against anyway + narrowed: FxHashSet>, /// names a nested scope reads, whose uses this walk cannot see captured: FxHashSet, /// the names this scope binds exactly once, which are the only ones that stand for one value @@ -812,6 +1247,38 @@ where .then(|| typevar.definition(self.db))? } + /// Whether `expr` reads `parameter`'s hole *narrowed*: something narrower than the argument, + /// but still described in terms of it. + /// + /// A name a test narrowed stands for something narrower than the argument, so nothing done + /// with it is a requirement on the argument and [`Self::hole`] rightly does not answer for it. + /// It is still the same argument underneath, though, and the type it now has still mentions + /// the hole — so the body goes on being checked against whatever bound the hole ends up with, + /// through a use no requirement was ever built from. That is the one way a use reaches the + /// bound without this walk seeing it, and it is closed the way every unreadable use is: the + /// parameter keeps no bound. + /// + /// A narrowing the bound already implies is not one of these, and needs no special case to say + /// so. `assert isinstance(x, int)` puts `int` into the bound, and from there narrowing `x` to + /// an `int` leaves the hole itself — which is the one case where recording the use is safe, + /// because what is recorded is then checked against the very bound it went into. Neither is a + /// narrowing that left a type of its own, with the hole nowhere in it: the use is then checked + /// against that type, and the bound has nothing to do with it. + fn narrows_hole(&self, expr: &Expr, parameter: Definition<'db>, name: &Name) -> bool { + let db = self.db; + let ty = (self.expression_type)(expr); + // the argument itself rather than a narrowing of it + if ty.is_inferred_parameter_hole(db) { + return false; + } + let env = self.env.clone(); + ty.references_typevar( + db, + &env, + inferred_parameter_typevar(db, name, parameter).identity(db), + ) + } + /// A type is only a requirement on the argument if it can be written down outside the /// body. Anything mentioning a type variable — another hole, or the callee's own /// generics — would either escape its binding context or make two holes depend on each @@ -824,22 +1291,56 @@ where /// never settle, since each round nests the round before it one level deeper. A protocol the /// *program* states, written as `protocol(...)` or established by a narrowing, is a /// requirement like any other and stays. + /// + /// `Never` is ruled out too. Nothing is of that type, so it is never a requirement anybody + /// could meet — and it is what an empty collection the body is still filling reads back as, so + /// taking it at its word would ask a member to hand back a value that cannot exist. + /// + /// A use-site modifier — `final T`, `literal T` — is ruled out on the same grounds. It says how + /// the value is used at the one place it is written, not what kind of thing the value is, so it + /// is not something a call site could be asked to supply. fn portable(&self, ty: Type<'db>) -> Option> { let env = self.env.clone(); (!ty.has_typevar_or_typevar_instance(self.db, &env) && !ty.is_dynamic() && !ty.is_object() + && !ty.is_never() + && !matches!(ty, Type::Restricted(_)) && !ty.mentions_recovered_protocol(self.db, &env)) .then_some(ty) } + /// The type an argument contributes to the shape of the member it was passed to. + /// + /// An argument is a *sample* of what the member has to accept rather than the only thing it + /// will ever be handed, so it is promoted first — the same reading a parameter's default + /// value gets. Without that, `i + 1` asks for an `__add__` that takes `Literal[1]`, and a + /// second function whose own body writes `i + 3` cannot pass its parameter to the first: + /// two requirements that any `int` meets would fail against each other. + /// + /// Only a literal is widened that way. Anything else an argument already is says what it is, + /// and a shape the *program* stated — the intersection a `match` pattern or a `hasattr` + /// established — is a requirement in its own right; widening it would throw away the part of + /// the shape that was stated rather than sampled. + fn argument_type(&self, expr: &Expr) -> Option> { + let env = self.env.clone(); + let argument = (self.expression_type)(expr); + let sample = if argument.is_literal_or_union_of_literals(self.db, &env) { + argument.promote(self.db, &env) + } else { + argument + }; + self.portable(sample) + } + /// Record that the value at `path` has to have a member called `name`, shaped like - /// `called` when it was called at all. + /// `called` when it was called at all, and reached the way `reach` says. fn record_member( &mut self, path: &MemberPath<'db>, name: &Name, called: Option>, + reach: MemberReach, ) { let member = self .uses @@ -849,7 +1350,251 @@ where .entry(name.clone()) .or_default(); if let Some(called) = called { - member.get_or_insert(called); + member.calls.push(called); + } + // a member the body reached by name as well as through syntax — `d[k]` and + // `d.__getitem__(k)` — was named + if reach == MemberReach::ByName { + member.reach = reach; + } + } + + /// The parameters a member has to take to be callable the way the body called it: a receiver, + /// then one positional per operand, typed as far as the operand's own type can be written + /// down. + /// + /// The operators, subscripting and iteration all reach their member this way — Python spells + /// them as syntax, but each one is a call on an ordinary member, and so is an ordinary + /// requirement on the value it was written against. + /// An operand written as `None` stands for a value the syntax passes but does not name — the + /// value `x[k] += 1` stores back — and leaves its position unannotated, which is what says + /// the member has to take one without saying what. + fn operand_parameters(&self, operands: &[Option<&Expr>]) -> Parameters<'db> { + let mut parameters = vec![Parameter::positional_only(Some(Name::new_static("self")))]; + for operand in operands { + let mut parameter = Parameter::positional_only(None); + if let Some(ty) = operand.and_then(|operand| self.argument_type(operand)) { + parameter = parameter.with_annotated_type(ty); + } + parameters.push(parameter); + } + Parameters::standard(parameters) + } + + /// Record a dunder the body reached through syntax — `x[k]`, `x - 1`, `-x` — as a member the + /// value at `path` has to have. + fn record_operator( + &mut self, + path: &MemberPath<'db>, + dunder: &'static str, + operands: &[Option<&Expr>], + sink: Option>, + ) { + let dunder = Name::new_static(dunder); + let parameters = self.operand_parameters(operands); + self.record_member(path, &dunder, Some(parameters), MemberReach::ThroughSyntax); + if let Some(sink) = sink { + self.record_value(path.member(&dunder), sink); + } + } + + /// Record that the value at `path` has to be iterable, and answer the path its elements are + /// reached by. + /// + /// Iteration is two members deep: `__iter__` hands back an iterator, and that iterator's + /// `__next__` hands back an element. Recording both is what keeps the elements a value in + /// their own right, so that what the loop body does with the loop variable is a requirement + /// on what the argument yields rather than on the argument itself. + fn record_iteration(&mut self, path: &MemberPath<'db>) -> MemberPath<'db> { + let iter = Name::new_static("__iter__"); + let next = Name::new_static("__next__"); + let receiver_only = self.operand_parameters(&[]); + self.record_member( + path, + &iter, + Some(receiver_only.clone()), + MemberReach::ThroughSyntax, + ); + let iterator = path.member(&iter); + self.record_member( + &iterator, + &next, + Some(receiver_only), + MemberReach::ThroughSyntax, + ); + iterator.member(&next) + } + + /// Record that `target` names the value at `path`, when it is a name that stands for one + /// value. + /// Answers whether it did: a name bound more than once cannot stand for one value, so the + /// uses below it are not uses of the value that was assigned to it and this walk cannot see + /// them at all. + fn bind_local(&mut self, target: &Expr, path: MemberPath<'db>) -> bool { + if let Expr::Name(target) = target + && self.single_bindings.contains(&target.id) + { + self.locals.insert(target.id.clone(), path); + return true; + } + false + } + + /// Say what `expr`'s position asks of the value written there, for the walk to read when it + /// reaches it. + fn reads_into(&mut self, expr: &Expr, sink: Sink<'db>) { + self.sinks.insert(expr.range(), sink); + } + + /// Say that `expr`'s position asks nothing of the value written there. + fn accounted_for(&mut self, expr: &Expr) { + self.reads_into(expr, Sink::Anything); + } + + /// Say that each of `exprs` sits in a position that asks nothing. + fn all_accounted_for<'e>(&mut self, exprs: impl IntoIterator) { + for expr in exprs { + self.accounted_for(expr); + } + } + + /// Whether `call` still binds when the argument written at `range` is `object`. + /// + /// This is the question a position that could not be read any other way is settled by. A + /// position that accepts `object` accepts every type this analysis could recover, because a + /// recovered bound is an ordinary type and `object` is above all of them — so such a position + /// asks nothing that has to be written down, and the body goes on checking whatever the bound + /// turns out to be. + /// + /// A position that does *not* accept `object` may still accept some particular bound, but + /// which one is not a thing a requirement can state: the checker has to pick an overload, or + /// a union element, and the argument is what decides which. So the value written there is + /// left unstatable rather than guessed at. + fn call_accepts_anything(&self, call: &ast::ExprCall, range: TextRange) -> bool { + let db = self.db; + let env = self.env.clone(); + let callee = (self.expression_type)(&call.func); + let arguments = CallArguments::from_arguments_typed(&call.arguments, |expr| { + if expr.range() == range { + Type::object() + } else { + (self.expression_type)(expr) + } + }); + callee + .bindings(db, &env) + .match_parameters(db, &env, &arguments) + .check_types( + db, + &env, + &ConstraintSetBuilder::new(), + &arguments, + TypeContext::default(), + &[], + ) + .is_ok() + } + + /// Whether `receiver`'s `dunder` takes `object`, which is what the syntax that dispatches to + /// it asks of the operand it passes there. + /// + /// The same reading as [`Self::call_accepts_anything`], for the operators. It is a *sufficient* + /// test and not an exact one: `left + right` can also succeed through `right`'s reflected + /// dunder, so an operand this says nothing about may still be fine. What it does establish is + /// that the operand is fine whatever it turns out to be, which is all that is being asked. + /// It is only asked where the answer can change one, because asking is a call of its own: a + /// query run from inside the fixed point that settles a parameter's bound joins that fixed + /// point, and one asked about an expression holding no tracked value would join it for nothing. + fn dunder_accepts_anything(&self, operand: &Expr, receiver: &Expr, dunder: &str) -> bool { + if !self.mentions_tracked_value(operand) { + return false; + } + let env = self.env.clone(); + (self.expression_type)(receiver) + .try_call_dunder( + self.db, + &env, + dunder, + CallArguments::positional([Type::object()]), + TypeContext::default(), + ) + .is_ok() + } + + /// What a place declaring `declared` asks of the value read into it, or `None` when what it + /// asks is something this analysis cannot write down. + fn declared_sink(&self, declared: Type<'db>) -> Option> { + if let Some(required) = self.portable(declared) { + return Some(Sink::Required(required)); + } + // a gradual place, or one typed `object`, accepts whatever it is handed, so a value read + // into it needs nothing recorded for the body to keep checking + (declared.is_dynamic() || declared.is_object()).then_some(Sink::Anything) + } + + /// Say that `expr` is read into a place declaring `declared`. + fn reads_into_declared(&mut self, expr: &Expr, declared: Type<'db>) { + if let Some(sink) = self.declared_sink(declared) { + self.reads_into(expr, sink); + } + } + + /// Record that the value at `path` was used in a way no requirement can state. + fn mark_unstatable(&mut self, path: &MemberPath<'db>) { + self.uses.entry(path.clone()).or_default().unstatable = true; + } + + /// Say that each argument of a call whose shape was recorded is accounted for. + /// + /// The shape was written down *from these very arguments* — each position takes what the + /// argument there already is, or takes anything where its type could not be written down — so + /// every one of them fits it by construction. + fn account_for_recorded_call_shape(&mut self, call: &ast::ExprCall) { + for argument in call.arguments.iter_source_order() { + match argument { + ast::ArgOrKeyword::Arg(argument) => self.accounted_for(argument), + ast::ArgOrKeyword::Keyword(keyword) => self.accounted_for(&keyword.value), + } + } + } + + /// Say that `expr` is read into wherever the expression it is a possible value of was read + /// into. + /// + /// `x or y`, `x if c else y` and `(a := x)` all hand on whatever they are given, so an arm of + /// one sits in the position the whole expression sits in — including, when that position was + /// one this analysis did not account for, in having no position at all. + fn hands_on(&mut self, expr: &Expr, sink: Option>) { + if let Some(sink) = sink { + self.reads_into(expr, sink); + } + } + + /// Say that the parts of a display or a slice are accounted for, when the display or slice + /// itself was asked nothing. + /// + /// A display builds a container this analysis does not describe, so it can say nothing about + /// what a place expecting a particular container asks of the elements. Where the whole was + /// asked nothing, though, neither is any part of it. + fn parts_accounted_for<'e>( + &mut self, + parts: impl IntoIterator, + sink: Option>, + ) { + if matches!(sink, Some(Sink::Anything)) { + self.all_accounted_for(parts); + } + } + + /// Record that whatever tracked value `expr` names was used in a way no requirement can state. + /// + /// A name a test narrowed counts too, though not as the same thing: nothing here can say what + /// the argument would have to be for such a use to pass either, but what the use is really + /// about is the narrower value, so it is recorded separately. + fn cannot_state(&mut self, expr: &Expr) { + match self.path(expr) { + Some(path) => self.mark_unstatable(&path), + None => self.mark_narrowed(expr), } } @@ -865,7 +1610,7 @@ where /// Record `x.name` read as a member the value at `path` has to have, whose own value has to /// fit wherever it was read into. fn record_read(&mut self, path: &MemberPath<'db>, name: &Name, sink: Option>) { - self.record_member(path, name, None); + self.record_member(path, name, None, MemberReach::ByName); if let Some(sink) = sink { self.record_value(path.member(name), sink); } @@ -873,45 +1618,59 @@ where /// Record `x.name(...)` as a method the value at `path` has to have, shaped like the call and /// returning something that fits wherever the result was read into. + /// + /// Answers whether the call was one that could be written down as a shape at all. A splatted + /// argument is not: `x.foo(*args)` passes however many elements `args` turns out to have, and + /// no fixed parameter list says that. Nothing is recorded then — not even that `foo` exists — + /// because the caller marks the whole value unstatable instead, and a member requirement built + /// from the *rest* of the body would be checked against this call and fail against it. fn record_call( &mut self, path: &MemberPath<'db>, name: &Name, call: &ast::ExprCall, sink: Option>, - ) { + reach: MemberReach, + ) -> bool { let mut parameters = vec![Parameter::positional_only(Some(Name::new_static("self")))]; for argument in &call.arguments.args { if argument.is_starred_expr() { - return; + return false; } let mut parameter = Parameter::positional_only(None); - if let Some(ty) = self.portable((self.expression_type)(argument)) { + if let Some(ty) = self.argument_type(argument) { parameter = parameter.with_annotated_type(ty); } parameters.push(parameter); } for keyword in &call.arguments.keywords { let Some(argument_name) = keyword.arg.as_ref() else { - return; + return false; }; let mut parameter = Parameter::keyword_only(argument_name.id.clone()); - if let Some(ty) = self.portable((self.expression_type)(&keyword.value)) { + if let Some(ty) = self.argument_type(&keyword.value) { parameter = parameter.with_annotated_type(ty); } parameters.push(parameter); } - self.record_member(path, name, Some(Parameters::standard(parameters))); + self.record_member(path, name, Some(Parameters::standard(parameters)), reach); if let Some(sink) = sink { self.record_value(path.member(name), sink); } + true } /// The value a *name* stands for: the parameter's own hole, or a local a value at some path /// was assigned to. fn name_path(&self, expr: &Expr) -> Option> { let db = self.db; + // a name being written to is not a read of the value it is about to hold + if let Expr::Name(name) = expr + && !name.ctx.is_load() + { + return None; + } if let Some(parameter) = self.hole(expr) { return Some(MemberPath::parameter(parameter)); } @@ -935,8 +1694,37 @@ where .then(|| path.clone()) } + /// The parameter `expr` is a *narrowing* of, when a test narrowed the name written there. + /// + /// Only a parameter's own name is answered for. A narrowing applies to a place, and the other + /// places it can apply to — `x.foo`, `x[0]` — are ones [`Self::path`] already answers for + /// whether or not they were narrowed, so their uses are recorded rather than lost. A *local* + /// stands for a value reached through the parameter rather than for the parameter, so a + /// narrowing of one says nothing about the argument even where the local's type is written in + /// terms of it. + fn narrowed_parameter(&self, expr: &Expr) -> Option> { + let name = expr.as_name_expr()?; + if !name.ctx.is_load() { + return None; + } + let parameter = *self.parameters.get(&name.id)?; + // the name still stands for the argument itself, so its uses are recorded as usual + if self.name_path(expr).is_some() { + return None; + } + self.narrows_hole(expr, parameter, &name.id) + .then_some(parameter) + } + + /// Record that the body did something with `parameter` narrowed. + fn mark_narrowed(&mut self, expr: &Expr) { + if let Some(parameter) = self.narrowed_parameter(expr) { + self.narrowed.insert(parameter); + } + } + /// The value `expr` produces, when the body says which one it is: a name for one, or a - /// member read or method call on one — the `x.foo()` of `a = x.foo()`. + /// member read, method call or subscript on one — the `x.foo()` of `a = x.foo()`. fn path(&self, expr: &Expr) -> Option> { let attribute = match expr { Expr::Call(call) => call.func.as_attribute_expr(), @@ -944,29 +1732,41 @@ where Expr::Attribute(_) => return None, _ => None, }; - let Some(attribute) = attribute else { - return self.name_path(expr); - }; - Some(self.path(&attribute.value)?.member(&attribute.attr.id)) - } - - /// Visit `expr` knowing the declared type it is being read into. - fn visit_into(&mut self, expr: &Expr, sink: Option>) { - let outer = std::mem::replace(&mut self.sink, sink); - self.visit_expr(expr); - self.sink = outer; + if let Some(attribute) = attribute { + return Some(self.path(&attribute.value)?.member(&attribute.attr.id)); + } + // the value of `x[k]` is what `x`'s `__getitem__` handed back, and the value of `f(...)` + // where `f` is itself a hole is what its `__call__` did, in exactly the way the value of + // `x.foo()` is what `foo` did + match expr { + Expr::Subscript(subscript) if subscript.ctx.is_load() => Some( + self.path(&subscript.value)? + .member(&Name::new_static("__getitem__")), + ), + Expr::Call(call) => self + .name_path(expr) + .or_else(|| Some(self.path(&call.func)?.member(&Name::new_static("__call__")))), + _ => self.name_path(expr), + } } - /// Visit `call`'s arguments, each knowing the parameter type it was matched to. + /// Say what each of `call`'s arguments is read into: the parameter it was matched to. /// /// That parameter type serves twice: an argument that *is* a hole has to fit it, and an /// argument that reads a member off a hole makes that member's value have to fit it. - fn visit_call_arguments(&mut self, call: &ast::ExprCall) { + /// + /// An argument whose parameter could not be worked out — the callee is a union, or overloaded, + /// or the call does not match its signature at all — gets no sink, and a tracked value written + /// there is one this cannot state. The parameter is a real requirement whether or not this + /// analysis can read it, and a bound built as though the argument were not there would be + /// checked against this very call. + fn account_for_call_arguments(&mut self, call: &ast::ExprCall) { let env = self.env.clone(); // a splatted argument hands the callee its *elements*, or its values under their own // names, so the parameter it lands on says nothing about the argument itself. taking // that parameter as a requirement would bound a hole by what it is expected to contain - // — `def f(it): C(*it)` would require `it` to *be* an `int` rather than to yield them + // — `def f(it): C(*it)` would require `it` to *be* an `int` rather than to yield them. + // what `*it` does ask for — that it be iterable — is recorded where the splat is written let arguments: Vec<(&Expr, bool)> = call .arguments .iter_source_order() @@ -977,38 +1777,94 @@ where .collect(); // binding a call is the expensive half of this analysis, and most calls in a body have - // nothing to do with any hole. only an argument that names a value reached from one has - // anything to learn from the parameter it was matched to - let constrains_a_hole = arguments + // nothing to do with any hole. only an argument with a value reached from one anywhere + // inside it has anything to learn from the parameter it was matched to + let tracked: Vec = arguments .iter() - .any(|(argument, splatted)| !splatted && self.path(argument).is_some()); - let parameter_types = if constrains_a_hole { - let callee = (self.expression_type)(&call.func); + .map(|(argument, splatted)| !splatted && self.mentions_tracked_value(argument)) + .collect(); + if !tracked.iter().any(|tracked| *tracked) { + // no argument of this call reads a value the walk tracks, so no position in it needs + // accounting for and the expensive half below is skipped + for (argument, splatted) in &arguments { + if *splatted && argument.is_starred_expr() { + self.accounted_for(argument); + } + } + return; + } + + let callee = (self.expression_type)(&call.func); + let parameter_types = call_parameter_types(self.db, &env, callee, &call.arguments, |expr| { (self.expression_type)(expr) - }) - .unwrap_or_default() - } else { - Vec::new() - }; + }); for (index, (argument, splatted)) in arguments.into_iter().enumerate() { - let matched = if splatted { - None - } else { - parameter_types - .get(index) - .copied() - .flatten() - .and_then(|matched| self.portable(matched)) + if splatted { + // `*xs` asks that `xs` be iterable, which the splat itself records; `**xs` asks + // that it be a mapping, which nothing here can state + if argument.is_starred_expr() { + self.accounted_for(argument); + } + continue; + } + let read_into = match parameter_types.as_ref().and_then(|types| types.get(index)) { + // an unannotated parameter accepts anything + Some(None) => Some(Sink::Anything), + Some(Some(declared)) => self.declared_sink(*declared), + None => None, }; - // a member read is constrained by the sink instead, when it is visited below - if let (Some(path), Some(matched)) = (self.name_path(argument), matched) { - self.record_value(path, matched); + match read_into { + Some(sink) => self.reads_into(argument, sink), + // no parameter type was readable, or the one that was is not something this can + // write down. the position is still accounted for if it would take `object` + None if !tracked[index] || self.call_accepts_anything(call, argument.range()) => { + self.accounted_for(argument); + } + None => {} } - self.visit_into(argument, matched); } } + + /// Whether `expr` reads a value this walk tracks, anywhere inside it. + /// + /// A name a test narrowed counts. What the position asks of it is not a requirement on the + /// argument, but it still has to be *read*: a position that takes anything takes a narrowed + /// value too, and skipping it would leave a `print(x)` under an `if x:` looking like a use + /// nothing accounted for. + fn mentions_tracked_value(&self, expr: &Expr) -> bool { + let tracked = + |expr: &Expr| self.path(expr).is_some() || self.narrowed_parameter(expr).is_some(); + if tracked(expr) { + return true; + } + let mut search = TrackedValueSearch { + found: false, + is_tracked: &tracked, + }; + walk_expr(&mut search, expr); + search.found + } +} + +/// Looks for a value a walk tracks, anywhere inside a subtree. +struct TrackedValueSearch<'a> { + found: bool, + is_tracked: &'a dyn Fn(&Expr) -> bool, +} + +impl Visitor<'_> for TrackedValueSearch<'_> { + fn visit_expr(&mut self, expr: &Expr) { + if self.found { + return; + } + if (self.is_tracked)(expr) { + self.found = true; + return; + } + walk_expr(self, expr); + } } impl<'db, F> Visitor<'_> for UseCollector<'db, F> @@ -1027,15 +1883,109 @@ where record_captured_names(stmt, &mut self.captured); } + // the value of an expression statement is thrown away, so the statement asks nothing + // of it + Stmt::Expr(expr) => { + self.accounted_for(&expr.value); + walk_stmt(self, stmt); + } + // `a = x.foo()` gives a name to the value at a path, so what the body goes on to do // with that name is a requirement on that value. the value itself is recorded by // the walk below as usual Stmt::Assign(assign) => { - if let [ast::Expr::Name(target)] = assign.targets.as_slice() - && self.single_bindings.contains(&target.id) + if let [target] = assign.targets.as_slice() && let Some(path) = self.path(&assign.value) + && self.bind_local(target, path) { - self.locals.insert(target.id.clone(), path); + // the name carries every use below it, so the assignment itself asks nothing + self.accounted_for(&assign.value); + } + for target in &assign.targets { + match target { + // `x[k] = v` is a `__setitem__` call, whose second argument is the + // value being assigned rather than anything inside the subscript + Expr::Subscript(subscript) => { + if let Some(path) = self.path(&subscript.value) { + self.record_operator( + &path, + "__setitem__", + &[Some(&*subscript.slice), Some(&*assign.value)], + None, + ); + self.accounted_for(&subscript.value); + self.accounted_for(&subscript.slice); + self.accounted_for(&assign.value); + } + } + // `a, b = xs` takes `xs` apart by iterating it, the same way a `for` + // does + Expr::Tuple(_) | Expr::List(_) => { + if let Some(path) = self.path(&assign.value) { + self.record_iteration(&path); + self.accounted_for(&assign.value); + } + } + _ => {} + } + } + walk_stmt(self, stmt); + } + + // `x[k] += 1` reads `x[k]`, operates on it and stores it back, so it asks for all + // three. the operator asked for is the plain one rather than the in-place one: + // Python falls back from `__iadd__` to `__add__`, and a type that defines only the + // in-place form is far rarer than one — `int`, `str`, `tuple` — that defines only + // the plain form + Stmt::AugAssign(assign) => { + if let Some(dunder) = runtime_dunder(assign.op) { + match &*assign.target { + Expr::Subscript(subscript) => { + if let Some(path) = self.path(&subscript.value) { + let key = Some(&*subscript.slice); + self.record_operator(&path, "__getitem__", &[key], None); + self.record_operator(&path, "__setitem__", &[key, None], None); + let element = path.member(&Name::new_static("__getitem__")); + self.record_operator( + &element, + dunder, + &[Some(&*assign.value)], + None, + ); + self.accounted_for(&subscript.value); + self.accounted_for(&subscript.slice); + self.accounted_for(&assign.value); + } + } + Expr::Attribute(attribute) if attribute.ctx.is_store() => { + if let Some(path) = self.path(&attribute.value) { + self.record_read(&path, &attribute.attr.id, None); + self.record_operator( + &path.member(&attribute.attr.id), + dunder, + &[Some(&*assign.value)], + None, + ); + self.accounted_for(&attribute.value); + self.accounted_for(&assign.value); + } + } + _ => {} + } + } + walk_stmt(self, stmt); + } + + // `for x in xs` iterates `xs`, and the loop variable names an element of it. + // `async for` asks instead for an `__aiter__` whose `__anext__` hands back something + // awaitable, which is not a shape this can write down + Stmt::For(for_stmt) => { + if !for_stmt.is_async + && let Some(path) = self.path(&for_stmt.iter) + { + let element = self.record_iteration(&path); + self.accounted_for(&for_stmt.iter); + self.bind_local(&for_stmt.target, element); } walk_stmt(self, stmt); } @@ -1045,24 +1995,93 @@ where self.visit_expr(&assign.target); if let Some(value) = assign.value.as_deref() { let declared = (self.expression_type)(&assign.annotation); - self.visit_into(value, self.portable(declared)); + self.reads_into_declared(value, declared); + self.visit_expr(value); } } Stmt::Return(ret) => { if let Some(value) = ret.value.as_deref() { - self.visit_into(value, self.declared_return); + match self.declared_return { + // an *inferred* return type is read off the body, so it asks nothing of it + None => self.accounted_for(value), + Some(declared) => self.reads_into_declared(value, declared), + } + self.visit_expr(value); } } + // a test is read for its truth, which every object answers + Stmt::If(if_stmt) => { + self.accounted_for(&if_stmt.test); + for clause in &if_stmt.elif_else_clauses { + if let Some(test) = &clause.test { + self.accounted_for(test); + } + } + walk_stmt(self, stmt); + } + + Stmt::While(while_stmt) => { + self.accounted_for(&while_stmt.test); + walk_stmt(self, stmt); + } + + Stmt::Assert(assert) => { + self.accounted_for(&assert.test); + if let Some(message) = assert.msg.as_deref() { + self.accounted_for(message); + } + walk_stmt(self, stmt); + } + + // everything left says nothing about the values written in it, which is what makes + // those values ones this cannot state. `raise x` asks for a `BaseException`, `with x` + // for a pair of context-manager methods, `del x.a` for an attribute nothing recorded, + // `match x` for whatever its patterns take apart — each a requirement this analysis + // has no way to write down, and each one a bound built without it would be checked + // against _ => walk_stmt(self, stmt), } } + // an interpolation is handed to `format`, which every object answers, so it asks nothing of + // the value written there + fn visit_interpolated_string_element(&mut self, element: &ast::InterpolatedStringElement) { + if let ast::InterpolatedStringElement::Interpolation(interpolation) = element { + self.accounted_for(&interpolation.expression); + } + ruff_python_ast::visitor::walk_interpolated_string_element(self, element); + } + fn visit_expr(&mut self, expr: &Expr) { // a sink belongs to the expression it was set for; whatever is nested inside it is // read into somewhere else, or nowhere - let sink = self.sink.take(); + let sink = self.sinks.remove(&expr.range()); + match sink { + Some(Sink::Required(required)) => { + // a name that stands for a tracked value carries the requirement itself. a value + // reached through a member carries it on that member instead, which the arms + // below record + if let Some(path) = self.name_path(expr) { + self.record_value(path, required); + } else { + // what the place asks of something narrower than the argument is not what it + // asks of the argument: a value the test ruled out never reaches here, and + // holding the argument to this would reject the very calls the test was + // written for + self.mark_narrowed(expr); + } + } + Some(Sink::Anything) => {} + // nothing above accounted for this expression, so whatever tracked value it reads was + // used in a way this analysis cannot state + None => self.cannot_state(expr), + } + let required = match sink { + Some(Sink::Required(required)) => Some(required), + _ => None, + }; match expr { Expr::Lambda(lambda) => { @@ -1071,8 +2090,16 @@ where } // a comprehension binds its own names in a scope of its own, so a name written - // inside one is not the local this scope bound + // inside one is not the local this scope bound. the iterable of its first `for` + // clause is the exception: that one is evaluated where the comprehension is written Expr::ListComp(_) | Expr::SetComp(_) | Expr::DictComp(_) | Expr::Generator(_) => { + if let Some(first) = comprehension_generators(expr).first() + && !first.is_async + && let Some(path) = self.path(&first.iter) + { + self.record_iteration(&path); + self.accounted_for(&first.iter); + } let outer = std::mem::replace(&mut self.in_nested_scope, true); walk_expr(self, expr); self.in_nested_scope = outer; @@ -1080,27 +2107,234 @@ where } Expr::Call(call) => { + // what the callee's own parameters ask of each argument, which is what an + // argument that is not part of a shape recorded below is held to + self.account_for_call_arguments(call); + if let Expr::Attribute(method) = &*call.func && let Some(path) = self.path(&method.value) { - self.record_call(&path, &method.attr.id, call, sink); + if self.record_call(&path, &method.attr.id, call, required, MemberReach::ByName) + { + self.accounted_for(&method.value); + self.account_for_recorded_call_shape(call); + } else { + self.mark_unstatable(&path); + } // the callee is this call, not a member read in its own right self.visit_expr(&method.value); } else { + // calling the parameter itself is a `__call__` on it, and the call decides + // that member's shape the same way a method call does + if let Some(path) = self.path(&call.func) { + if self.record_call( + &path, + &Name::new_static("__call__"), + call, + required, + MemberReach::ThroughSyntax, + ) { + self.accounted_for(&call.func); + self.account_for_recorded_call_shape(call); + } else { + self.mark_unstatable(&path); + } + } self.visit_expr(&call.func); } - self.visit_call_arguments(call); + + for argument in call.arguments.iter_source_order() { + match argument { + ast::ArgOrKeyword::Arg(argument) => self.visit_expr(argument), + ast::ArgOrKeyword::Keyword(keyword) => self.visit_expr(&keyword.value), + } + } return; } + // a member being *written* — `x.a = 1`, `del x.a` — asks for one this analysis does + // not record, so the value it is written on is left unaccounted for Expr::Attribute(attribute) => { if attribute.ctx.is_load() && let Some(path) = self.path(&attribute.value) { - self.record_read(&path, &attribute.attr.id, sink); + self.record_read(&path, &attribute.attr.id, required); + self.accounted_for(&attribute.value); + } + } + + Expr::Subscript(subscript) => { + if subscript.ctx.is_load() { + if let Some(path) = self.path(&subscript.value) { + self.record_operator( + &path, + "__getitem__", + &[Some(&*subscript.slice)], + required, + ); + self.accounted_for(&subscript.value); + self.accounted_for(&subscript.slice); + } else if self.dunder_accepts_anything( + &subscript.slice, + &subscript.value, + "__getitem__", + ) { + self.accounted_for(&subscript.slice); + } + } + } + + // only the *left* operand carries the requirement. a tracked value on the right is + // left unaccounted for on purpose: python reaches the right operand's reflected + // dunder only when the left one returns `NotImplemented`, so which of the two routes + // the operation takes is decided by the argument, and neither route is a requirement + // on its own — `"%s" % attr` runs entirely through `str.__mod__`, and `str` has no + // `__rmod__` for `attr` to be required to have + Expr::BinOp(binary) => { + if let Some(dunder) = runtime_dunder(binary.op) { + if let Some(path) = self.path(&binary.left) { + self.record_operator(&path, dunder, &[Some(&*binary.right)], required); + self.accounted_for(&binary.left); + self.accounted_for(&binary.right); + } else if self.dunder_accepts_anything(&binary.right, &binary.left, dunder) { + // `"%s" % attr` runs entirely through `str.__mod__`, which takes anything + self.accounted_for(&binary.right); + } + } + } + + Expr::UnaryOp(unary) => { + if let Some(dunder) = unary_dunder(unary.op) + && let Some(path) = self.path(&unary.operand) + { + self.record_operator(&path, dunder, &[], required); + self.accounted_for(&unary.operand); + } else if matches!(unary.op, ast::UnaryOp::Not) { + // `not x` reads `x` for its truth, which every object answers + self.accounted_for(&unary.operand); + } + } + + // `a < b < c` is two comparisons on the same operands read pairwise, so an operand in + // the middle of a chain is only accounted for when both of them account for it + Expr::Compare(compare) => { + let operands: Vec<&Expr> = std::iter::once(&*compare.left) + .chain(&compare.comparators) + .collect(); + let mut accounted = vec![true; operands.len()]; + for (index, op) in compare.ops.iter().enumerate() { + let (Some(left), Some(right)) = (operands.get(index), operands.get(index + 1)) + else { + continue; + }; + match op { + // `==`, `!=` and the identity tests are answered by `object` itself + ast::CmpOp::Eq | ast::CmpOp::NotEq | ast::CmpOp::Is | ast::CmpOp::IsNot => { + } + // `in` can run through `__contains__`, `__iter__` *or* `__getitem__` on + // the right operand, and asking for any one of the three would demand + // something the body never needed. that disjunction is not a shape a + // protocol member can state, and neither is what whichever of them runs + // then asks of the left operand + ast::CmpOp::In | ast::CmpOp::NotIn => { + // the container it runs through still decides what it takes, and + // most of them take anything + if !self.dunder_accepts_anything(left, right, "__contains__") { + accounted[index] = false; + } + accounted[index + 1] = false; + } + _ => match (comparison_dunder(*op), self.path(left)) { + (Some(dunder), Some(path)) => { + self.record_operator(&path, dunder, &[Some(right)], None); + } + (Some(dunder), None) + if self.dunder_accepts_anything(right, left, dunder) => {} + _ => { + accounted[index] = false; + accounted[index + 1] = false; + } + }, + } + } + for (operand, accounted) in operands.iter().zip(accounted) { + if accounted { + self.accounted_for(operand); + } + } + } + + // `f(*xs)` hands the callee `xs`'s elements, which means iterating it + Expr::Starred(starred) => { + if starred.ctx.is_load() + && let Some(path) = self.path(&starred.value) + { + self.record_iteration(&path); + self.accounted_for(&starred.value); + } + } + + Expr::BoolOp(bool_op) => { + for value in &bool_op.values { + self.hands_on(value, sink); + } + } + + Expr::If(ternary) => { + self.accounted_for(&ternary.test); + self.hands_on(&ternary.body, sink); + self.hands_on(&ternary.orelse, sink); + } + + // `(a := x)` is `x` under another name and `x` again as the value of the expression, + // so the name has to be one that can carry the uses below it for either to be read + Expr::Named(named) => { + if let Some(path) = self.path(&named.value) + && self.bind_local(&named.target, path) + { + self.hands_on(&named.value, sink); + } + } + + Expr::Tuple(tuple) => self.parts_accounted_for(&tuple.elts, sink), + Expr::List(list) => self.parts_accounted_for(&list.elts, sink), + Expr::Set(set) => self.parts_accounted_for(&set.elts, sink), + Expr::Dict(dict) => self.parts_accounted_for( + dict.items + .iter() + .flat_map(|item| item.key.iter().chain(std::iter::once(&item.value))), + sink, + ), + + // the parts of a slice are handed to whatever the subscript resolved to, and where + // that was a `__getitem__` this analysis recorded, its shape was written down from + // this very slice + Expr::Slice(slice) => self.parts_accounted_for( + [ + slice.lower.as_deref(), + slice.upper.as_deref(), + slice.step.as_deref(), + ] + .into_iter() + .flatten(), + sink, + ), + + // what a generator yields becomes the yield type of the generator it returns, which + // is read off the body rather than asked of it. a declared return type does ask, and + // is not a shape this takes apart + Expr::Yield(yielded) => { + if self.declared_return.is_none() + && let Some(value) = yielded.value.as_deref() + { + self.accounted_for(value); } } + // everything left is a position this analysis does not read: `await x` asks for an + // `__await__` whose iterator yields the awaited value, `yield from x` for an + // iterable. neither is written down, so a tracked value in one of them is left + // unaccounted for _ => {} } 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 e7572099c2..5072fa8a1a 100644 --- a/crates/ty_python_semantic/src/types/set_theoretic/builder.rs +++ b/crates/ty_python_semantic/src/types/set_theoretic/builder.rs @@ -1295,6 +1295,19 @@ impl<'db> UnionBuilder<'db> { ) })); } + // basedpython: a literal without a group of its own — a float or a complex — + // is carried as an ordinary element, so it is the only place the union's + // `recursively_defined` can be recorded for it. The grouped kinds above are + // rebuilt from their value and get the flag the same way. Without this the flag + // lives only on the `UnionType` wrapper, and anything that takes the union apart + // into its elements — the constraint solver turning a union into one lower bound + // per element, say — hands those elements on with nothing left to say the union + // they came from was defined in terms of itself. + UnionElement::Type(Type::LiteralValue(literal)) if recursively_defined.is_yes() => { + types.push(Type::LiteralValue( + literal.with_recursively_defined(recursively_defined), + )); + } UnionElement::Type(ty) => types.push(ty), } } @@ -2198,6 +2211,26 @@ mod tests { KnownClass::Complex.to_instance(db, &env) ); + // Taking a recursively defined union apart and building a fresh one out of its elements + // — which is what a generic call's solve does, collecting one lower bound per element — + // must not lose the fact that it was defined in terms of itself. Nothing carries that + // here except the elements themselves, so a builder left at its default still reaches + // the same widened answer. + let under_limit = doubling().take(2).map(Type::float_literal).fold( + UnionBuilder::new(db, &env).recursively_defined(RecursivelyDefined::Yes), + UnionBuilder::add, + ); + let Type::Union(under_limit) = under_limit.build() else { + panic!("two distinct float literals should not have collapsed to one type"); + }; + let rebuilt = under_limit + .elements(db) + .iter() + .copied() + .chain(doubling().skip(2).map(Type::float_literal)) + .fold(UnionBuilder::new(db, &env), UnionBuilder::add); + assert_eq!(rebuilt.build(), KnownClass::Float.to_instance(db, &env)); + // A union nothing defines in terms of itself settles in one go, so its literals are // kept however many there are. let non_recursive = over_limit diff --git a/crates/ty_python_semantic/src/types/soundness.rs b/crates/ty_python_semantic/src/types/soundness.rs index 099d5a8af6..6374f5a0f5 100644 --- a/crates/ty_python_semantic/src/types/soundness.rs +++ b/crates/ty_python_semantic/src/types/soundness.rs @@ -289,6 +289,37 @@ pub fn runtime_check_plan<'db>( runtime_check_target(db, env, file, ty).map(CheckKind::Isinstance) } +/// [`runtime_check_plan`] for a *parameter*, whose type may be a hole rather than +/// a written type. +/// +/// An unannotated parameter opens a hole bounded by everything the function +/// requires of it, and a hole has no faithful runtime test of its own — so asking +/// [`runtime_check_plan`] about one answers nothing, and the interpreted leg wrote +/// no check where the native backend was already enforcing the bound. +/// +/// What the check is planned against is the part of that bound the source +/// *states*: the parameter's default. `def f(safe='/')` says `safe` is a `str`, +/// and if something else belonged there something else would be written. The rest +/// of the bound is recovered from how the body happens to use the parameter, and a +/// recovered requirement is not something to raise a `TypeError` over — it is a +/// sample, and it can be withdrawn. +pub fn parameter_runtime_check_plan<'db>( + db: &'db dyn Db, + env: &ProgramEnvironment<'db>, + file: File, + ty: Type<'db>, +) -> Option { + let stated = match ty { + Type::TypeVar(typevar) => { + let definition = typevar.typevar(db).definition(db)?; + crate::types::inferred_signature::stated_parameter_bound(db, definition)? + } + // a written annotation is already the stated type + written => written, + }; + runtime_check_plan(db, env, file, stated) +} + /// whether a runtime check against `ty` must silently drop a type-argument /// claim: `ty` carries a written generic specialization, but its arguments are /// erased at runtime, so only the origin class can be tested. this is what diff --git a/docs/basedpython/development/compilation/index.md b/docs/basedpython/development/compilation/index.md index b5803f0542..c0f52a2a6c 100644 --- a/docs/basedpython/development/compilation/index.md +++ b/docs/basedpython/development/compilation/index.md @@ -18,6 +18,31 @@ the observable behaviour of a compiled module and its interpreted twin must be identical. that is not an aspiration, it is the property the entire test strategy is built on ([plan](plan.md#differential-testing)) +it holds for the programs `by check` accepts, and a compiled function is the leg +that has to be *made* to hold it: it checks its arguments at its boundary, where +an interpreted one checks nothing until the opt-in `parameters` soundness gate is +turned on. that gate used to read a parameter's plan off its **annotation**, so a +type stated with a default was invisible to it: + +```python +def f(safe='/'): + return len(safe) + +f(b'abc') # compiled: TypeError. twin, before: 3 +``` + +a default is a written type. `def f(safe='/')` says `safe` is a `str` as plainly +as an annotation would — if something else belonged there, something else would +be written — and the type system already agrees: a default's requirement is +`WhenContradicted::Stands`, holding even where the body contradicts it, while +everything the body merely *samples* can be withdrawn. so the gate now plans +against the parameter, and reaches what the source stated either way. + +it deliberately stops there. the rest of an unannotated parameter's bound is +recovered from how its body happens to use it, and a recovered requirement is not +something to raise a `TypeError` over. `x=None` states nothing either — that says +the argument may be left out, not that `None` is what belongs there + ## why a second backend and not a mypyc invocation the obvious cheap move is to transpile `.by` to `.py` and hand the result to diff --git a/docs/basedpython/development/compilation/runtime.md b/docs/basedpython/development/compilation/runtime.md index 061f96dbb2..507f7e27ce 100644 --- a/docs/basedpython/development/compilation/runtime.md +++ b/docs/basedpython/development/compilation/runtime.md @@ -176,6 +176,32 @@ call reads that refusal as proof no override exists. so a base an interpreted class extends is left interpreted too, and `by compile --verbose` names the subclass that caused it +#### the twin arrives compiled + +parsing that source is most of what importing a compiled module costs — a +stdlib-sized module is milliseconds of it, enough that a compiled module could +import slower than the `.py` it came from. so the build asks the target +interpreter to compile the twin once and embeds the code object beside the +source. an import reads that instead: `argparse` goes from 6.6ms to 0.31ms and +`_pydecimal` from 11.2ms to 0.52ms + +a code object is only good for the interpreter that wrote it, so the artefact +records two things about the one that did and the runtime checks both before +using it: + +- the **bytecode magic**, cpython's own answer to the same question — it is what + makes an upgraded interpreter regenerate a `.pyc` rather than misread one. + handing 3.14 a code object 3.13 wrote segfaults the process, so this is not a + tidiness check +- the **optimization level**, because `-O` takes `assert` out of the bytecode and + `-OO` takes docstrings too. the twin has always been compiled by the importing + interpreter, so `python -O` has always meant `-O` for it, and a code object + compiled at the build's level would quietly stop meaning that + +either mismatch sends the import back to the source, which is slower and is the +same program. a code object that passes both checks and then will not read is a +broken artefact rather than a mismatched one, and fails the import + ## integers `int` is `CPyTagged`: a pointer-sized word where an even value is a small diff --git a/docs/basedpython/development/compilation/technology.md b/docs/basedpython/development/compilation/technology.md index b3c4da28c8..ffddfc4596 100644 --- a/docs/basedpython/development/compilation/technology.md +++ b/docs/basedpython/development/compilation/technology.md @@ -188,6 +188,35 @@ paying a function call for every field read is the opposite of the exercise. mypyc reached the same conclusion. this is worth revisiting only if [HPy](https://hpyproject.org) stabilizes with a performant CPython ABI mode +### an artefact is pinned to one minor version, and says so itself + +taking the full API means the runtime header reads layouts that move between +versions, so `by.h` is full of `#if PY_VERSION_HEX` branches — and those are +decided by the headers the *build* compiled against. an artefact loaded by a +different minor version therefore runs branches written for a layout that +interpreter does not have, which is a crash rather than a wrong answer + +cpython does not prevent this. the version tag lives in the **file name**, and +every 3.x also lists a bare `.so` in `EXTENSION_SUFFIXES` — so an artefact that +is renamed, or copied out of a wheel built elsewhere, is offered to whatever is +running. that is not hypothetical: `argparse` built for 3.13 and renamed +segfaults inside a type construction under 3.14, in a build with no marshalled +code object in it at all + +so every emitted module refuses one itself. `PyInit_` calls +`By_InterpreterMatches` before it hands its module definition over — before +anything of the build's own layout is read — and a mismatch is an `ImportError` +naming both versions. the reading is `Py_GetVersion` rather than the newer +`Py_Version`, because it is the one every version this header compiles against +exports: a module built against newer headers naming a symbol the running +interpreter lacks would be the same failure by another road + +this is the general form of the check the marshalled fallback makes for itself. +that one compares the bytecode magic, which moves for a different reason and can +move within a micro release, and a disagreement there *declines* to the embedded +source rather than refusing the import — the code object is a cache, while the +compiled code is the module + ### free-threading is a design constraint now, not a migration later cpython 3.13 introduced free-threaded builds and 3.14 made them supported. diff --git a/docs/basedpython/features/sound-types.md b/docs/basedpython/features/sound-types.md index 200db76001..c014e8b169 100644 --- a/docs/basedpython/features/sound-types.md +++ b/docs/basedpython/features/sound-types.md @@ -134,6 +134,39 @@ def f(x): a synthesized bound is spelled as the [inline protocol](inline-protocol.md) that would declare it, so the recovered signature is something you could have written by hand +an operator, a subscript, iteration and calling the parameter itself are members like any other, +so each of those is a requirement too + +```python +def f(s): + a = s.rstrip("\r\n") + b = s[:5] +# def f(s: some protocol(def __getitem__(self, slice[None, 5, None], /) -> Unknown; def rstrip(self, str, /) -> object)) +``` + +iterating asks for two members at once — `__iter__`, and a `__next__` on whatever that hands back — +so what a loop body does with the loop variable is a requirement on what the argument *yields* + +```python +def total(xs): + for x in xs: + x.bit_length() +# def total(xs: some protocol(def __iter__(self, /) -> protocol(def __next__(self, /) -> protocol(def bit_length(self, /) -> object)))) +``` + +only the *left* operand of a binary operation carries the requirement. python reaches the right +operand's reflected dunder only when the left one returns `NotImplemented`, and which of the two an +operation takes is decided by the argument, so `2 * x` asks nothing of `x` — see +[a bound has to type the body it came from](#a-bound-has-to-type-the-body-it-came-from) for what +that then means for `x` + +a member the body reached this way reads back as `Unknown` rather than `object`, because recording +a requirement is about what the *call site* has to supply and it should not change what the body +itself reads. so does a member the body named: the requirement is that it **exist**, and nothing +about naming it says what it holds. `object` would not describe such a value, it would forbid every +use of it — and that claim travels, because a member's type becomes the recovered return type of the +function that read it + a parameter the argument is forwarded into is a requirement too ```python @@ -145,14 +178,19 @@ def f(x): f("a") # error: invalid-argument-type ``` -and reading a member into somewhere that says what it holds constrains that member, not just the -parameter. an annotated assignment, a call argument and a declared return type are all such places +and reading a value into somewhere that says what it holds is a requirement on that value. an +annotated assignment, a call argument and a declared return type are all such places, and each of +them constrains whichever value was read into it — a member, or the parameter itself ```python def f(x): a: int = x.foo() return x # def f(x: some protocol(def foo(self, /) -> int)) -> x + +def g(x) -> str: + return x +# def g(x: some str) -> str ``` an *inferred* return type is not one of them: it is read off the body, so it cannot also constrain @@ -174,6 +212,26 @@ that a member's value was given to is read the same way: a name bound more than for one value, and a use under a narrowing is about something narrower than the value it was bound to +saying nothing is not the same as costing nothing. a narrowed *parameter* still carries the hole, so +the body goes on being checked against whatever bound the rest of it recovers — through a use no +requirement was ever built from, which is what +[a bound has to type the body it came from](#a-bound-has-to-type-the-body-it-came-from) rules out. so +a use like that takes the recovered bound away, exactly as the unnarrowed form of it would + +```python +def bits(x): + x.bit_length() + if x: + return 2 * x + return 0 +# def bits(x) +``` + +a narrowing the bound already implies is not one of these, which is what keeps the `assert` below +working. once `int` is `x`'s bound, `x` narrowed to an `int` *is* the hole, so the uses under such an +`assert` are recorded like any other — and what they record is then checked against the very bound +the `assert` put there + a parameter its own body rebinds keeps nothing at all, for the same reason. the reads above the rebinding are not enough on their own: walking a linked structure asks only that the argument have the member it walks along, so the rebinding lands on that member — whose value nothing described — @@ -203,12 +261,60 @@ f("a") # error: invalid-argument-type the same test inside an `if` says nothing — the author plainly meant the other branch to be reachable +### a bound has to type the body it came from + +every requirement above is read off the body, so the body is checked against the bound they add up +to. that makes one thing non-negotiable: a bound that the body's *own* code would fail against is +worse than no bound at all, because it makes the checker report an error in the very function it +claims to have understood + +so a use the analysis cannot state does not get passed over — it takes the bound away + +```python +def area(r): + a = r.bit_length() + return 2 * r +# def area(r) +``` + +reading `r.bit_length()` on its own asks for a protocol. `2 * r` two lines later is exactly what such +a protocol cannot answer: python reaches `r`'s reflected dunder only when `int.__mul__` returns +`NotImplemented`, and which of the two routes an operation takes is decided by the argument, so there +is nothing here to ask of `r`. keeping the protocol would make `area`'s own body stop compiling +against `area`'s own signature, so the protocol goes + +the same rule applies one member deep: a use nothing can be said about takes away only the value it +is about. `x.foo` still has to be there, and still has to be callable the way the body called it + +```python +def held(x): + return 1 + x.foo() +# def held(x: some protocol(def foo(self, /) -> Unknown)) +``` + ### what is left out -nothing is invented from a use that was not understood, so a body keeps type-checking exactly as it -did and its call sites stay unchecked. a forwarded type that mentions a type variable is left out -too: it is bound to the callee's own scope, and the same rule stops two functions that forward into -each other from each defining the other +the uses that cannot be stated are the ones where python's own answer is a disjunction, or a shape an +inline protocol has no way to write: + +- an operand on the *right* of an operation whose left operand does not take anything, as above +- `in`, which runs through `__contains__`, `__iter__` *or* `__getitem__` on the container +- `await`, `async for`, `with`, `raise`, `del x.a`, `match`, and `**x` in a call +- writing a member rather than reading one — `x.a = 1` +- a call whose arguments are splatted, since no fixed parameter list says how many there are +- an argument whose parameter cannot be worked out, or is one this cannot write down +- anything at all written on a name a test narrowed, unless the position takes anything: the branch + was written because the author meant the other one to be reachable, so holding every argument to + what this one does would reject the very calls the test exists for + +the other side of that rule is what keeps most code unaffected: wherever a position accepts `object` +it accepts whatever bound the body recovers, so nothing has to be recorded and nothing is lost. +printing a value, formatting one, reading one for its truth, looking a key up in a mapping and +`"%s" % x` are all positions like that + +a forwarded type that mentions a type variable is left out too: it is bound to the callee's own +scope, and the same rule stops two functions that forward into each other from each defining the +other a value the body reached *through* a parameter is left out on the same grounds. reading a member off one leaves the shape this analysis invented for that member, so requiring a method to accept it @@ -378,10 +484,21 @@ a type variable a call leaves *unsolved* is a separate matter, and is covered by these are known gradual-guarantee costs that `sound-types` does **not** currently address -- **a use the body analysis cannot read** contributes nothing: `def f(x): return x + 1` leaves `x` - gradual. only attribute reads, method calls, forwarding into an annotated parameter and a - top-level `assert` are read. operators, subscripting, iteration and calling the parameter itself - are not +- **a use the body analysis cannot read** contributes nothing: `def f(x): return 1 in x` leaves `x` + gradual, because `in` runs through `__contains__`, `__iter__` *or* `__getitem__` and asking for + any one of the three would demand something the body never needed. an `async for`, a `with` and + an operation whose left operand is not the parameter are left out for the same kind of reason +- **an operation gives way to what the program states**: where a default value, an `assert` or a + forwarded parameter type rules the operation out, the statement wins and the operation is + reported in the body where it lives rather than at every call site. `def g(x=0): x + "foo"` is + an error in `g` +- **a call recorded twice needs one signature**: two calls of the same member union position by + position, so `m.group("a")` and `m.group("b")` ask for a `group` that takes `str`. two calls of + different *shape* would need an overload, which cannot be written here, so such a member + degrades to asking only that it exist and be callable +- **a call site whose argument is itself an unsolved hole** is where most of the remaining noise + lives: the callee's requirement cannot be propagated onto the caller's own hole, because it is + a shape this analysis invented, so the forward is reported instead - **`*args` / `**kwargs` do not open a hole**: one type parameter cannot name a run of arguments, so they stay `tuple[Unknown, ...]` / `dict[str, Unknown]`, and a `target(**kwargs)` forward is entirely unchecked diff --git a/scripts/bg.sh b/scripts/bg.sh index eb58781312..4758261436 100755 --- a/scripts/bg.sh +++ b/scripts/bg.sh @@ -30,16 +30,41 @@ set -u +# where a job's markers live: outside the project, and *per checkout* +# +# keeping them out of the project matters because `by compile` compiles every +# source it finds beside the file it was given. keeping them per checkout matters +# because `$TMPDIR` is per *user* on macos, not per session — several worktrees +# are worked in at once here, and two of them starting a job under the same +# obvious name (`relbuild`, `tests`) shared one set of markers. the second +# session's `start` deleted the first's, so the first waited on a log another +# process was writing and read a status that was never its own. that really +# happened +# +# the checkout's own path is used rather than a hash of it: it is exact, so two +# trees can never land on one directory, and each component is short enough that +# the nesting costs nothing dir() { - # the scratchpad if the harness gave us one, else a temp dir. keeping the - # markers out of the project matters: `by compile` compiles every source it - # finds beside the file it was given - printf '%s\n' "${BY_BG_DIR:-${TMPDIR:-/tmp}/by-bg}" + if [ -n "${BY_BG_DIR:-}" ]; then + printf '%s\n' "$BY_BG_DIR" + return + fi + local root + root=$(git rev-parse --show-toplevel 2>/dev/null) || root=$PWD + printf '%s\n' "${TMPDIR:-/tmp}/by-bg$root" } start() { local name="$1"; shift local d; d=$(dir); mkdir -p "$d" + # a name still in use is the caller's mistake, not something to paper over: + # clearing the markers under a live job orphans it, and the log it goes on + # writing then belongs to a job nobody is waiting for + local state; state=$(status "$name") + if [ "$state" = running ]; then + printf '%s is already running — pick another name or wait for it\n' "$name" >&2 + return 1 + fi rm -f "$d/$name.done" "$d/$name.log" "$d/$name.pid" # the marker is written by the same shell that runs the command, after it # exits, so it cannot be missed and it carries the real status diff --git a/scripts/native-sweeps/isoconstruct.sh b/scripts/native-sweeps/isoconstruct.sh index 55a6daaeca..fdac21512d 100755 --- a/scripts/native-sweeps/isoconstruct.sh +++ b/scripts/native-sweeps/isoconstruct.sh @@ -125,8 +125,8 @@ for b in $(sweep_modules "$LIB" "$@"); do cp "$root/drive.py" "$SWEEP_RUN_I/drive.py"; cp "$root/drive.py" "$SWEEP_RUN_C/drive.py" # a traceback names the file it came from, and the two legs run from different # directories — so the *path* would read as a difference the module never had - i=$(leg "$SWEEP_RUN_I") - c=$(leg "$SWEEP_RUN_C") + i=$(sweep_canonical "$(leg "$SWEEP_RUN_I")") + c=$(sweep_canonical "$(leg "$SWEEP_RUN_C")") if [ "$i" = "$c" ]; then # a module that cannot be imported here agrees on both legs and exercises nothing — # kept apart from `same` so the denominator stays honest diff --git a/scripts/native-sweeps/isoimport.sh b/scripts/native-sweeps/isoimport.sh index 2b1e1df01b..bf7ef75e06 100755 --- a/scripts/native-sweeps/isoimport.sh +++ b/scripts/native-sweeps/isoimport.sh @@ -37,7 +37,7 @@ leg() { out=$(cd "$dir" && "$PY" -c "$probe" 2>&1); LEG_STATUS=$? # the two legs run from different directories, so a message that names a file would # read as a difference the module never had. each is made relative to its own root - LEG_TEXT=$(printf '%s' "$out" | tail -1 | sed "s|$dir/||g") + LEG_TEXT=$(sweep_canonical "$(printf '%s' "$out" | tail -1 | sed "s|$dir/||g")") } for b in $(sweep_modules "$LIB" "$@"); do diff --git a/scripts/native-sweeps/isoinstance.sh b/scripts/native-sweeps/isoinstance.sh new file mode 100755 index 0000000000..18e59d18a9 --- /dev/null +++ b/scripts/native-sweeps/isoinstance.sh @@ -0,0 +1,418 @@ +#!/bin/bash +# each module in a project of its own, built, imported, constructed — and then the +# instance *used* +# +# `isoconstruct` calls every class with no arguments and compares the outcome, so it +# sees a constructor that binds its arguments wrongly. it stops there: the instance it +# made is thrown away. `isosurface` asks the *class* what it contains, which is a +# question about the type object and not about anything the type was used for. so a +# whole class of divergence falls between them — the constructor succeeds, the class +# carries every name it should, and the object is wrong the first time a program touches +# it: +# +# tempfile._RandomNameSequence.rng a @property whose body does `self._rng = ...` +# interpreted +# compiled +# +# every other rung scores that module `same`, because none of them reads an attribute +# off an instance. this one does: it constructs each class and then touches what a +# program would touch — every property, every class-level attribute, and `repr`, `str`, +# `bool`, `len`, `hash`, `iter` and `next` where the class defines them — and compares +# the answer, value or exception, between the two legs. +# +# three things make a *value* comparison trustworthy where a name comparison did not +# need to be: +# +# - the two legs must touch the same things. a compiled class publishes a getset for +# every field its twin keeps on the instance, and a compiled property is not a +# `property` object — so a leg left to choose for itself would probe a different set +# and, worse, would skip the very member above. so the *plan* is drawn up once, from +# the interpreted class, and both legs are handed it +# - a value that moves on its own is not evidence. `next()` on the sequence above +# returns eight random characters; a property can read the clock. so each leg is run +# twice and any probe that does not agree with *itself* is dropped from the +# comparison and counted as `unstable`, rather than reported as a difference. a +# difference that survives that gets a third run of each leg before it is reported, +# because a probe with few possible answers — a `__bool__` that flips a coin — +# repeats itself often enough to pass a two-run filter by luck +# - what is left still has to be rendered so that two processes agree when nothing is +# wrong: `sweep_write_renderer` does that, and the reasons are with it +# +# usage: isoinstance.sh SP BY PY OUT [MODULE...] +SP="$1"; BY="$2"; PY="$3"; OUT="$4"; shift 4 +# shellcheck source=scripts/native-sweeps/sweeplib.sh +. "$(dirname "$0")/sweeplib.sh" +LIB=$(sweep_lib "$PY") +root="$SP/isoinst.$$"; rm -rf "$root"; mkdir -p "$root" +trap 'rm -rf "$root"' EXIT +: > "$OUT" + +# how long one construction or one probe is given. it is the same two seconds +# `isoconstruct` allows a constructor, for the same reason: a member that blocks must +# not stall the corpus +export SWEEP_PROBE_BOUND=${SWEEP_PROBE_BOUND:-2} +# a class's members are read in sorted order and this many are taken. the cap applies to +# both legs identically — it is applied when the plan is drawn up, and both legs run the +# same plan — so it can hide a difference past the cap and can never invent one. it is +# here because a class deriving from `dict` inherits about forty names that all render +# as ``, and a handful of modules define hundreds of classes +export SWEEP_MEMBER_LIMIT=${SWEEP_MEMBER_LIMIT:-40} + +cat > "$root/plan.py" <<'PYEOF' +"""what both legs will touch, decided once by the interpreted class + +a leg that chose for itself would choose differently — a compiled class's members are +getset descriptors where its twin's are properties and functions — and the difference +would fall exactly where the defect is. so this runs against the interpreted module and +its answer is handed to both +""" + +import importlib +import os +import signal + +# the staging says what to import: `m` for a top-level module, `pkg.m` for a package +# member, which is the only name its relative imports resolve against +MOD = os.environ['SWEEP_MOD'] +SELF = (MOD, MOD.rpartition('.')[2]) +LIMIT = int(os.environ['SWEEP_MEMBER_LIMIT']) + + +def _ring(signum, frame): + raise TimeoutError('timed out') + + +signal.signal(signal.SIGALRM, _ring) +try: + signal.alarm(int(os.environ['SWEEP_IMPORT_BOUND'])) + try: + m = importlib.import_module(MOD) + finally: + signal.alarm(0) +except BaseException as error: + print('@IMPORT-FAILED\t%s\t%s' % (type(error).__name__, error), flush=True) + raise SystemExit(0) + +# what a compiled type does not carry by construction rather than by defect, and what +# 3.13's compiler writes into a class statement's namespace from a code object a spec +# has none of. reading any of these off an instance compares the two builds' plumbing +# rather than the module's meaning +BY_DESIGN = { + '__dict__', '__weakref__', '__firstlineno__', '__static_attributes__', '__module__', +} + +for name in sorted(vars(m)): + cls = vars(m)[name] + if not isinstance(cls, type) or getattr(cls, '__module__', None) not in SELF: + continue + try: + members = dir(cls) + except BaseException: + members = [] + lines = [(name, 'repr', '-'), (name, 'str', '-'), (name, 'bool', '-')] + # a default `__hash__` is derived from the address, so it differs between two + # processes of the same build and says nothing. only a hash the class wrote is asked + if getattr(cls, '__hash__', None) not in (None, object.__hash__): + lines.append((name, 'hash', '-')) + for dunder, kind in (('__len__', 'len'), ('__iter__', 'iter'), ('__next__', 'next')): + if dunder in members: + lines.append((name, kind, '-')) + # `hasattr(object, ...)` drops what every object has; the dunder test drops the rest + # of the protocol surface, which is compared above as behaviour rather than read as + # a value — a method read off an instance is a bound method on one leg and a builtin + # on the other, and that is spelling, not meaning + attrs = [member for member in sorted(members) + if not (member.startswith('__') and member.endswith('__')) + and member not in BY_DESIGN + and not hasattr(object, member)] + for member in attrs[:LIMIT]: + lines.append((name, 'attr', member)) + for line in lines: + print('@PLAN\t%s\t%s\t%s' % line, flush=True) +PYEOF + +cat > "$root/probe.py" <<'PYEOF' +"""construct each class the plan names, then touch it the way a program would""" + +import importlib +import os +import signal +import sys + +from sweepcanon import Canon + +MOD = os.environ['SWEEP_MOD'] +SELF = (MOD, MOD.rpartition('.')[2]) +BOUND = int(os.environ['SWEEP_PROBE_BOUND']) + + +class _Slow(Exception): + pass + + +def _ring(signum, frame): + raise _Slow('timed out') + + +def load(): + try: + signal.alarm(int(os.environ['SWEEP_IMPORT_BOUND'])) + try: + return importlib.import_module(MOD) + finally: + signal.alarm(0) + except BaseException as error: + print('@IMPORT-FAILED\t%s\t%s' % (type(error).__name__, error), flush=True) + raise SystemExit(0) + + +def aliases(m): + """the module prefix the two builds spell differently, and nothing else + + an emitted class takes its `__module__` from the last component of the file name, so + it answers `m` where its twin answers `by_stage.pkg.m`. that spelling is inside every + `repr`, every `` and every AttributeError message a probe produces, so + left alone it would be the only thing this rung ever reported. `isosurface` compares + `__module__` outright and reports the defect once per class, which is where it + belongs. here both spellings are removed, by exact class name rather than by pattern: + a blanket `m.` substitution would also rewrite `m.py` inside a message + """ + out = [] + quals = set() + for name in sorted(vars(m)): + cls = vars(m)[name] + if not isinstance(cls, type) or getattr(cls, '__module__', None) not in SELF: + continue + quals.add(name) + quals.add(getattr(cls, '__qualname__', name)) + try: + inner_values = list(vars(cls).values()) + except BaseException: + inner_values = [] + for inner in inner_values: + if isinstance(inner, type): + quals.add(inner.__name__) + quals.add(getattr(inner, '__qualname__', inner.__name__)) + for qual in quals: + for prefix in SELF: + out.append(('%s.%s' % (prefix, qual), qual)) + return out + + +def instance(m, cache, name): + """one instance per class, made once and reused by every probe against it + + a construction that fails is not this rung's finding — `isoconstruct` compares that + outcome, wording and all — so only the fact is recorded, and the class's probes all + answer with it. that keeps one line per plan entry, which is what lets a leg killed + mid-probe be restarted at the right place + """ + if name in cache: + return cache[name] + cls = getattr(m, name, None) + if not isinstance(cls, type): + made = (None, '') + else: + try: + signal.alarm(BOUND) + try: + made = (cls(), None) + finally: + signal.alarm(0) + except BaseException as error: + made = (None, '' % type(error).__name__) + cache[name] = made + return made + + +def touch(canon, obj, kind, member): + if kind == 'attr': + return canon.render(getattr(obj, member)) + if kind == 'repr': + return canon.scrub(repr(obj)) + if kind == 'str': + return canon.scrub(str(obj)) + if kind == 'bool': + return repr(bool(obj)) + if kind == 'len': + return repr(len(obj)) + if kind == 'hash': + return repr(hash(obj)) + if kind == 'iter': + return canon.render(type(iter(obj))) + if kind == 'next': + return canon.render(next(obj)) + return '' % kind + + +def main(): + signal.signal(signal.SIGALRM, _ring) + m = load() + canon = Canon(aliases(m)) + with open('plan.txt') as handle: + plan = [line.rstrip('\n').split('\t') for line in handle if line.strip()] + start = int(sys.argv[1]) if len(sys.argv) > 1 else 0 + cache = {} + for index in range(start, len(plan)): + name, kind, member = plan[index] + obj, failure = instance(m, cache, name) + if failure is not None: + answer = failure + else: + try: + signal.alarm(BOUND) + try: + answer = touch(canon, obj, kind, member) + finally: + signal.alarm(0) + except BaseException as error: + answer = canon.raised(error) + print('@P%d\t%s\t%s\t%s\t%s' % (index, name, kind, member, answer), flush=True) + + +# `multiprocessing` starts a worker by re-running this interpreter and importing this +# file, as `__mp_main__`. a constructor that makes a pool would otherwise have every +# worker import the module and probe every class in it again — pool included +if __name__ == '__main__': + main() +PYEOF + +# run one leg to completion, restarting past whatever killed it +# +# a probe that segfaults truncates the leg, and a truncated leg reads as an ordinary +# difference — which is exactly how 56 modules' crashes went unreported in +# `isoconstruct` before it grew this loop. the death is written as a line of its own +# carrying the index it happened at, so it both survives into the comparison and +# consumes the probe that caused it +# +# only the probe lines and an import failure are kept. a leg also writes whatever the +# module wrote — a traceback, an `Exception ignored in __del__` — and those lines carry +# none of the four fields the comparison pairs rows by, so keeping them would pair rows +# that are not the same probe. nothing is lost by dropping them: a leg that failed +# outright leaves a non-zero status, which becomes a `DIED` row of its own +leg() { + local dir="$1" start=0 attempts=0 text="" out="" status=0 done_lines=0 + while [ "$attempts" -lt 40 ]; do + attempts=$((attempts+1)) + out=$(cd "$dir" && "$PY" probe.py "$start" 2>&1 | grep -E '^@(P[0-9]+|IMPORT-FAILED)\t' + exit "${PIPESTATUS[0]}"); status=$? + # *both* legs' directories are replaced, in both legs, and the compiled one first + # because it sits inside the interpreted one. scrubbing only the leg's own directory + # would leave the other leg's spelling of the same idea standing: `wsgiref.handlers` + # publishes `os.environ` as a class attribute, and the two legs necessarily run from + # different directories, so `PWD` alone made that module differ + out=$(printf '%s' "$out" | sed -e "s|$SWEEP_RUN_C||g" -e "s|$SWEEP_RUN_I||g") + [ -n "$out" ] && text="$text$out"$'\n' + [ "$status" -eq 0 ] && break + done_lines=$(printf '%s' "$text" | grep -c '^@P') + text="$text@P$done_lines"$'\t'"DIED"$'\t'"signal=$status"$'\t'"-"$'\t'"died"$'\n' + start=$((done_lines + 1)) + done + printf '%s' "$text" +} + +# the probes a leg agrees with itself about. anything else moved on its own — a clock, a +# random draw, an address this rung did not scrub — and cannot be evidence either way +stable() { + comm -12 <(printf '%s' "$1" | LC_ALL=C sort) <(printf '%s' "$2" | LC_ALL=C sort) +} + +# keep the lines whose key — index, class, probe, member — is in the given key list +keep() { + LC_ALL=C join -t $'\t' -j 1 \ + <(printf '%s' "$1" | awk -F'\t' 'NF {print $1 "\x01" $2 "\x01" $3 "\x01" $4 "\t" $0}' | LC_ALL=C sort -t $'\t' -k1,1) \ + <(printf '%s' "$2" | LC_ALL=C sort) \ + | cut -f2- +} + +keys() { + printf '%s' "$1" | awk -F'\t' 'NF {print $1 "\x01" $2 "\x01" $3 "\x01" $4}' | LC_ALL=C sort -u +} + +for b in $(sweep_modules "$LIB" "$@"); do + f="$LIB/$b" + [ -f "$f" ] || continue + d="$root/w"; sweep_stage "$d" "$LIB" "$b" + sweep_compile "$b" "$d" "$PY" "$BY" + if ! sweep_built "$d"; then printf '%s\tno-artifact\n' "$b" >> "$OUT"; continue; fi + sweep_place "$d" + for run in "$SWEEP_RUN_I" "$SWEEP_RUN_C"; do + cp "$root/plan.py" "$run/plan.py"; cp "$root/probe.py" "$run/probe.py" + sweep_write_renderer "$run" + done + # the plan is drawn up on the interpreted leg and handed to both. a module the + # interpreter cannot import here exercises nothing, and says so rather than agreeing + # + # importing a module runs it, and a module may print: `this.py` prints the whole zen of + # python at import. so the plan is read off marked lines rather than off the output, + # and so is everything the probe program says. without that, twenty lines of poetry + # became twenty malformed plan entries, the probe program raised on the first of them, + # and the module was reported as a crash on both legs + said=$(cd "$SWEEP_RUN_I" && "$PY" plan.py 2>&1) + plan=$(printf '%s' "$said" | grep '^@PLAN'$'\t' | cut -f2-) + if printf '%s' "$said" | grep -q '^@IMPORT-FAILED'$'\t'; then + printf '%s\timport-failed\t%s\n' "$b" \ + "$(printf '%s' "$said" | grep '^@IMPORT-FAILED'$'\t' | head -1)" >> "$OUT"; continue + fi + if [ -z "$plan" ]; then + # no class this module owns, or none with anything to touch. a rung that scored this + # `same` would be reporting agreement it never looked for + printf '%s\tnothing-to-probe\n' "$b" >> "$OUT"; continue + fi + printf '%s\n' "$plan" > "$SWEEP_RUN_I/plan.txt" + printf '%s\n' "$plan" > "$SWEEP_RUN_C/plan.txt" + i1=$(leg "$SWEEP_RUN_I"); i2=$(leg "$SWEEP_RUN_I") + c1=$(leg "$SWEEP_RUN_C"); c2=$(leg "$SWEEP_RUN_C") + steady=$(comm -12 <(keys "$(stable "$i1" "$i2")") <(keys "$(stable "$c1" "$c2")")) + i=$(keep "$i1" "$steady"); c=$(keep "$c1" "$steady") + if [ "$i" != "$c" ]; then + # two runs are enough to expose a clock or a random draw, and not enough to expose a + # probe with only a few possible answers: a `__bool__` that flips a coin repeats + # itself half the time, and a rung that stopped here would report the coin as a + # defect. a difference is therefore confirmed with a third run of each leg before it + # is reported, which costs a run only on the modules that were going to be looked at + # by hand anyway + i3=$(leg "$SWEEP_RUN_I"); c3=$(leg "$SWEEP_RUN_C") + steady=$(comm -12 <(keys "$(stable "$(stable "$i1" "$i2")" "$i3")") \ + <(keys "$(stable "$(stable "$c1" "$c2")" "$c3")")) + i=$(keep "$i1" "$steady"); c=$(keep "$c1" "$steady") + else + i3=""; c3="" + fi + # a leg killed by an alarm has not answered, and an empty answer must never be read as + # agreement — so this is decided before any verdict is written + if printf '%s%s%s%s%s%s' "$i1" "$i2" "$c1" "$c2" "$i3" "$c3" | grep -q '_Slow timed out'; then + printf '%s\ttimed-out\n' "$b" >> "$OUT"; continue + fi + # a compiled leg that cannot be imported at all has no probes to compare, and its one + # line would read as a wholesale difference. it is a real finding, but it is + # `isoimport`'s, so it is named rather than diffed + if printf '%s' "$c1" | grep -q '^@IMPORT-FAILED'$'\t'; then + printf '%s\tcompiled-import-failed\t%s\n' "$b" \ + "$(printf '%s' "$c1" | grep '^@IMPORT-FAILED'$'\t' | head -1)" >> "$OUT"; continue + fi + compared=$(printf '%s' "$steady" | grep -c '') + unstable=$(( $(printf '%s' "$plan" | grep -c '') - compared )) + if printf '%s\n%s\n%s\n%s\n%s\n%s\n' "$i1" "$i2" "$c1" "$c2" "$i3" "$c3" | grep -q $'\tDIED\t'; then + # a leg that was killed answered nothing, and two legs killed the same way answer + # nothing in the same words — which is not agreement. this was not hypothetical: a + # broken copy of this script made both legs die identically forty times, and the + # first version scored the pair `same`. so a death is its own verdict, and the diff + # goes with it so the probe that caused it is named + { printf '%s\tCRASHED\t%s\tunstable=%s\n' "$b" "$compared" "$unstable" + diff <(printf '%s\n' "$i") <(printf '%s\n' "$c") | awk -v b="$b" '{print b "\t| " $0}' + } >> "$OUT" + elif [ "$compared" -eq 0 ]; then + # every probe moved on its own, so the two legs were never compared. this is the + # reading `instancecensus` once gave silently as `0 of 0`, and it is not agreement + printf '%s\tnothing-stable\t%s\n' "$b" "$(printf '%s' "$plan" | grep -c '')" >> "$OUT" + elif [ "$i" = "$c" ]; then + printf '%s\tsame\t%s\tunstable=%s\n' "$b" "$compared" "$unstable" >> "$OUT" + else + { printf '%s\tDIFFERS\t%s\tunstable=%s\n' "$b" "$compared" "$unstable" + diff <(printf '%s\n' "$i") <(printf '%s\n' "$c") | awk -v b="$b" '{print b "\t| " $0}' + } >> "$OUT" + fi +done +echo "walked: $(grep -cE $'\t(same|DIFFERS|CRASHED|timed-out|import-failed|compiled-import-failed|nothing-to-probe|nothing-stable|no-artifact)\t?' "$OUT") exercised: $(grep -cE $'\t(same|DIFFERS)\t' "$OUT") differing: $(grep -c $'\tDIFFERS\t' "$OUT") crashed: $(grep -c $'\tCRASHED\t' "$OUT") nothing-stable: $(grep -c $'\tnothing-stable' "$OUT") nothing-to-probe: $(grep -c $'\tnothing-to-probe' "$OUT") timed-out: $(grep -c $'\ttimed-out' "$OUT") import-failed: $(grep -c $'\timport-failed' "$OUT") compiled-import-failed: $(grep -c $'\tcompiled-import-failed' "$OUT") no-artifact: $(grep -c $'\tno-artifact' "$OUT")" diff --git a/scripts/native-sweeps/isosubclass.sh b/scripts/native-sweeps/isosubclass.sh index 4a432a921a..ebbfc41f35 100755 --- a/scripts/native-sweeps/isosubclass.sh +++ b/scripts/native-sweeps/isosubclass.sh @@ -137,8 +137,8 @@ for b in $(sweep_modules "$LIB" "$@"); do if ! sweep_built "$d"; then printf '%s\tno-artifact\n' "$b" >> "$OUT"; continue; fi sweep_place "$d" cp "$root/drive.py" "$SWEEP_RUN_I/drive.py"; cp "$root/drive.py" "$SWEEP_RUN_C/drive.py" - i=$(leg "$SWEEP_RUN_I") - c=$(leg "$SWEEP_RUN_C") + i=$(sweep_canonical "$(leg "$SWEEP_RUN_I")") + c=$(sweep_canonical "$(leg "$SWEEP_RUN_C")") if [ "$i" = "$c" ]; then # a module that cannot be imported here agrees on both legs and exercises nothing — # kept apart from `same` so the denominator stays honest diff --git a/scripts/native-sweeps/sweeplib.sh b/scripts/native-sweeps/sweeplib.sh index 7451368ec8..60d625b2bc 100755 --- a/scripts/native-sweeps/sweeplib.sh +++ b/scripts/native-sweeps/sweeplib.sh @@ -280,9 +280,185 @@ sweep_warm() { return 0 } +# canonicalise a set literal in a leg's output, so its *order* is not read as a difference +# +# a set has none. `__abstractmethods__` is a frozenset, and cpython's own +# `DeprecationWarning: Unimplemented abstract methods {...}` prints it in whatever order +# the hashes fell — which differed between the two legs consistently, and the rungs compare +# text, so it read as `DIFFERS` for a module where both legs name the same two methods +# +# ⚠️ only a *set*. a dict prints in insertion order and that order is meaningful, so a +# `{...}` that parses as a dict is left exactly as written. the span is parsed with +# `ast.literal_eval` rather than matched with a regex, precisely so the two cannot be +# confused — a nested `{'a': 1}` inside a set of tuples would defeat any "contains a colon" +# test. anything that does not parse as a literal is left alone +sweep_canonical() { + # an address is never a difference: two processes print two addresses for the same + # object, so a leg that merely *mentions* one differs from itself. cpython prints them + # unbidden — `Exception ignored in: ` reaches a + # comparison from the garbage collector, and `tempfile` scored a difference on that line + # alone, which a perfect compiler would also have scored + set -- "$(printf '%s' "$1" | sed -E 's/0x[0-9a-fA-F]+/0xX/g')" + case $1 in *'{'*) ;; *) printf '%s' "$1"; return 0 ;; esac + printf '%s' "$1" | "$PY" -c ' +import ast, sys +text = sys.stdin.read() +out, i = [], 0 +while i < len(text): + if text[i] != "{": + out.append(text[i]); i += 1; continue + depth, j = 0, i + while j < len(text): + if text[j] == "{": depth += 1 + elif text[j] == "}": + depth -= 1 + if depth == 0: break + j += 1 + span = text[i:j+1] + try: + value = ast.literal_eval(span) + except (ValueError, SyntaxError, TypeError, MemoryError, RecursionError): + value = None + if isinstance(value, (set, frozenset)): + out.append("{" + ", ".join(sorted(repr(v) for v in value)) + "}") + else: + out.append(span) + i = j + 1 +sys.stdout.write("".join(out)) +' +} + # true when the build actually left an extension module behind. `-d o` is not enough: # a build that fails halfway leaves the directory and no artefact, and the compiled leg # then fails to import for a reason that is not the defect the sweep is looking for sweep_built() { sweep_artifact "$1" >/dev/null } + +# write the shared value renderer next to a leg, as `sweepcanon.py` +# +# named apart from `sweep_canonical` on purpose: that one takes *text* a leg already +# printed and normalises it, and three rungs call it that way. this one takes a +# *directory* and writes a python module into it. one name for both would have been +# resolved by bash in favour of whichever was defined last, and the text callers would +# have silently stopped canonicalising +# +# a rung that compares *values* rather than names has to answer one question first: what +# in a repr moves between two runs of the same program, and is therefore never a defect? +# two things do. an address — `` — differs between +# any two processes. and the order a `set` or a `frozenset` prints in differs with the +# hash seed and with insertion history, so the same set prints two ways. a `dict`'s order +# is *not* in that class: it is insertion order, it is part of the answer, and a compiled +# module that built one in a different order has a real defect. so sets are sorted and +# dicts are left exactly as they came +# +# the renderer also takes a list of aliases — text to substitute before comparing. the +# rungs use it for the one difference that is already reported elsewhere: an emitted +# class in a package answers `m` for its `__module__` where its twin answers `pkg.m`, +# and that spelling is inside every repr and every AttributeError message a probe +# produces. `isosurface` reports that defect once per class, which is where it belongs; +# repeating it inside every value would leave nothing else visible +sweep_write_renderer() { + cat > "$1/sweepcanon.py" <<'PYEOF' +"""turn a value, or the exception reading it raised, into text two processes agree on""" + +import re +import types + +_ADDR = re.compile(r'0x[0-9a-fA-F]+') + +# a container is rendered element by element rather than repr'd, so these bound the work +# and the output. they apply to both legs identically, so a cap can hide a difference +# past the cap but can never invent one +ELEMENTS = 50 +CHARACTERS = 200 +DEPTH = 3 +# past this a set is described rather than rendered: sorting is what makes a set +# comparable, and sorting has to see all of it +SET_LIMIT = 2000 + +# read off an instance, a method is a `bound method` on the interpreted leg and a +# `builtin_function_or_method` on the compiled one — the same member, two spellings. +# so a callable is rendered by its name and its kind is dropped +_CALLABLE = ( + types.FunctionType, types.MethodType, types.BuiltinFunctionType, + types.MethodDescriptorType, types.WrapperDescriptorType, + types.MethodWrapperType, types.ClassMethodDescriptorType, + types.GetSetDescriptorType, types.MemberDescriptorType, + staticmethod, classmethod, +) + + +class Canon: + def __init__(self, aliases=()): + # longest first: `pkg.m.Outer.Inner` must not be half-replaced by `pkg.m.Outer` + self.aliases = sorted(aliases, key=lambda pair: len(pair[0]), reverse=True) + + def scrub(self, text): + text = _ADDR.sub('0xX', text) + for old, new in self.aliases: + text = text.replace(old, new) + # a rung compares its two legs line by line, so a rendered value has to stay on + # one line: `str()` of an object and of an exception both readily contain a + # newline, and one that reached the output would split a probe's answer into + # rows the comparison could pair up wrongly + text = text.replace('\\', '\\\\').replace('\n', '\\n').replace('\t', '\\t') + if len(text) > CHARACTERS: + text = text[:CHARACTERS] + '...' + return text + + def render(self, value, depth=0): + try: + return self._render(value, depth) + except BaseException as error: + return '' % type(error).__name__ + + def _render(self, value, depth): + if depth > DEPTH: + return '...' + if value is None or value is True or value is False: + return repr(value) + kind = type(value) + if kind in (int, float, complex, str, bytes, bytearray): + return self.scrub(repr(value)) + if kind in (list, tuple): + return self._sequence(value, depth, '[%s]' if kind is list else '(%s)') + if kind in (set, frozenset): + if len(value) > SET_LIMIT: + return '<%s of %d>' % (kind.__name__, len(value)) + # the whole set is rendered before anything is dropped: capping first would + # cap an arbitrary slice, which is the very thing sorting exists to defeat + items = sorted(self.render(item, depth + 1) for item in value) + return '{%s}' % ', '.join(self._cap(items, len(value))) + if kind is dict: + items = ['%s: %s' % (self.render(key, depth + 1), self.render(item, depth + 1)) + for key, item in list(value.items())[:ELEMENTS]] + return '{%s}' % ', '.join(self._cap(items, len(value))) + if isinstance(value, type): + return '' % self.scrub( + '%s.%s' % (getattr(value, '__module__', '?'), + getattr(value, '__qualname__', value.__name__))) + if isinstance(value, types.ModuleType): + return '' % self.scrub(getattr(value, '__name__', '?')) + if isinstance(value, _CALLABLE): + return '' % self.scrub(str(getattr(value, '__name__', '?'))) + return self.scrub(repr(value)) + + def _sequence(self, value, depth, shape): + items = [self.render(item, depth + 1) for item in list(value)[:ELEMENTS]] + return shape % ', '.join(self._cap(items, len(value))) + + def _cap(self, items, total): + if total > ELEMENTS: + return items[:ELEMENTS] + ['...+%d' % (total - ELEMENTS)] + return items + + def raised(self, error): + """an exception is an answer too, so its type *and* its wording are compared""" + try: + text = str(error) + except BaseException: + text = '' + return '' % (type(error).__name__, self.scrub(text)) +PYEOF +}