GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Share code construction and comparison controls by youknowdot · Pull Request #9713 · RustPython/RustPython

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 4, 2026 •
edited by youknowone
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Summary

Align code-object construction, replacement, comparison and hashing controls with Python 3.15 through shared validation and existing binding primitives in two Rust production files.

  • Compare constant keys before names and tables, releasing temporary keys left-to-right on both success and error.
  • Compare retained Python name/local-plus/table objects through their protocols, preserving tuple/bytes-subclass callbacks instead of comparing normalized internal arrays.
  • Hash the retained objects in CPython's order, preserving callback errors and the cached hash of the original constants tuple. Remove the local-partition count: code objects with identical combined locals-plus names can compare equal despite different local/cell partitions.
  • Share CodeUnits' existing atomic deoptimization/cache traversal between byte serialization and hashing. Exclude inline caches and adaptive counters; hold the monitoring lock only after Python hash callbacks finish.

Validation

Compared against pinned CPython d220409a13b77de7a6c30abe16ea90db01528edc codeobject.c. The original four reference groups and nine expanded groups pass, covering callback order, early false/error exits, retained-field identity, local/cell partitions, original tuple hash caching and stable hashing after ordinary specialization. Constructed or modified code objects were not executed.

Independent source review passed, including bounds/traversal preservation, atomic reads, monitoring-lock scope and equality/hash consistency. Final threaded cargo check passed (1m04s), all-target Clippy passed (2m17s) with exactly the 12 unchanged baseline diagnostics, and normal commit hooks passed.

Limits and integration

The initial validation used external reference checks and source review. Current native validation is recorded in the shared-binding follow-up section; hosted CI remains necessary. No repository tests, markers, Lib files or workflows are changed.

The goal includes observable protocol callbacks and equality/hash consistency, not a promise of identical numerical hashes across distinct interpreter implementations. Signed constructor first-line representation differences remain outside this patch. #9688's filename repr work and #8939's constant-borrow ownership work remain separate and should be preserved on composition.

Integration with #9439 (head 95a4916) requires explicit reconciliation, not a clean automatic composition: retain that PR's removal of per-code monitoring data and its original_bytes-based deopt_code; omit this patch's extracted monitoring resolver and lock; hash through original_instructions_with with identity resolution. Its bounded base-instruction analysis helper has different stop semantics and should not replace serialization/hash traversal blindly. The retained-object comparison/hash controls are not duplicated by #9439.

Shared construction and replacement controls

  • Share signed C-int conversion, original argument retention, audit-event construction, structural validation, name interning and CodeObject assembly. Derived signatures, KwArgs and the shared Clinic validation in bind_for handle binding. The owner-retaining wrapper forwards binding metadata without a separate keyword parser or nested bind.
  • Accept the constructor's optional freevars/cellvars tuples and positional-only signature. Convert replacement controls in declaration order and reject iterable substitutes for tuple fields without calling their iteration hooks.
  • Emit the nine-value code.new audit event with original object identities and propagate vetoes at the constructor/replacement-specific phase. Retain numeric arguments through later callbacks. Snapshot the replacement's default filename before index conversion callbacks can mutate it.
  • Keep constructor name normalization separate from replacement's exact-string validation, matching their distinct error precedence. Share argument/local-capacity validation and retain positive unknown flag bits. Preserve the original comparison/hash changes.

The constructor's negative firstlineno still uses the existing unsigned internal line representation. In-place interning of externally referenced tuples, constant deduplication and exact interned filename/name identity, malformed bytecode parity, generic binder TypeError wording, and broader locals/line representation changes remain outside scope. Existing safe bytecode checks and location-array behavior are retained; no constructed or replaced code is executed in reference probes. #6894's old locals representation redesign is not duplicated, but would require source reconciliation if revived.

The current group changes 887 lines (+511/-376) in two production Rust files. Constructor-specific name diagnostics use native type names and the verified UTF-8-safe 500-byte bound. The added validation includes 103 focused and 103 independent pinned CPython cases, covering audit identity/veto/order, conversion and destruction order, structural errors, tuple subclass controls, mutable filename snapshots, flag retention and diagnostic bounds. Independent source review also confirmed that the original comparison/hash section is unchanged. These are reference checks and source review, not a claim that all oracle wording passes verbatim on RustPython.

Final follow-up validation: threaded host-env/VM/stdlib cargo check passed in 1m02s and all-targets Clippy in 2m20s, with exactly the 12 unchanged baseline diagnostics. The first compile exposed three borrowed-reference ownership conversions; those and two new lint findings were corrected, independently reviewed, and both final commands rerun. Normal commit hooks passed. Those earlier runs did not include full workspace, C-API or native runtime tests. Current results follow; separate WASM/non-threaded graphs remain unexecuted.

Shared-binding follow-up

Rebased independently onto main at fb57e68 and validated commit 886c794. The four existing commits are patch-identical after rebase. Later main changes through ea5fbcc affect ctypes and zlib, not the code-object or binding files used here.

  • Forward PARAMS and BINDING_PARAMS from the typed arguments so shared Clinic validation runs once before conversion. Remove the duplicate arity/keyword preflight, temporary keyword-name vector, and private specification trait.
  • Keep the original argument owners through numeric conversions and audit callbacks. Preserve the replacement filename snapshot before conversion and existing callee diagnostics.
  • Independent review and normal commit hooks passed. No tests, expected-failure markers, or unrelated production files were changed by this follow-up.

Validation on this exact commit:

  • Full workspace tests with the documented WASM, venvlauncher and C-API exclusions: 1,513 passed, 20 ignored across unit and documentation suites.
  • Full workspace all-target Clippy: passed with zero warnings. This was ordinary Clippy, without -D warnings.
  • Separate C-API all-target Clippy: passed with zero warnings. Separate C-API tests: 149 passed, 2 ignored, using CARGO_PROFILE_TEST_DEBUG=0.
  • Fresh release RustPython -m test test_code: SUCCESS, 37 tests run and 15 skipped.

The host PyO3 interpreter was the existing CPython 3.12.14, verified in the fresh build configuration. C-API checks used the repository's RustPython 3.15 configuration without an external interpreter. The pinned CPython 3.15 comparison is source-only; no CPython 3.15 executable was run for this follow-up. Hosted CI for the updated head remains pending.

AI assistance is disclosed in the commit trailers.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Explain Change →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

  • X
  • Mastodon
  • Reddit
  • LinkedIn
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@youknowdot youknowdot changed the title Preserve code comparison and hashing callbacks Share code construction and comparison controls Nov 4, 2026
youknowone
youknowone reviewed Nov 6, 2026
View reviewed changes
Comment thread crates/vm/src/builtins/code.rs Outdated
_owners: FuncArgs,
}

impl<T: CodeArgumentSpec> FromArgs for CodeArgs<T> {

@youknowone youknowone Nov 6, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

check if we still need this after #9563

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@youknowdot
youknowdot force-pushed the fix/code-controls-20261104 branch from 4e49d67 to 4c33098 Compare November 6, 2026 14:00
youknowdot and others added 5 commits November 7, 2026 09:24
@youknowdot @codex
Preserve code comparison and hashing callbacks
91aa222
Assisted-by: OpenAI Codex:model version not exposed
@youknowdot @codex
Share code construction and replacement controls
5777aed
Preserve argument owners, signed validation, audit and name-conversion order through shared construction. Match optional closure tuples and replacement controls without invoking tuple subclass iteration.

Assisted-by: OpenAI Codex:model version not exposed
@codex
Use canonical code constructor argument names
7808947
Remove redundant pyarg name overrides by naming the FromArgs fields codestring and constants. Preserve constructor storage mapping and Python signatures.

Assisted-by: OpenAI: undisclosed
@codex
Rely on scope cleanup for redundant explicit drops
b0aecbb
Preserve callback ownership, release ordering, and retained argument owners.

Assisted-by: Codex:model-version-unavailable
@codex
Use shared binding validation for retained code arguments
886c794
Forward the derived binding metadata and let bind_for validate once before converting. Preserve original argument owners through conversion and audit callbacks.

Assisted-by: OpenAI Codex:GPT-6
@youknowdot
youknowdot force-pushed the fix/code-controls-20261104 branch from 4c33098 to 886c794 Compare November 7, 2026 01:53
youknowone
youknowone approved these changes Nov 8, 2026
View reviewed changes
@youknowone
youknowone marked this pull request as ready for review November 8, 2026 04:00
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 8, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-08T04:10:21.847300Z 886c794 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
youknowone merged commit f98f70c into RustPython:main Nov 8, 2026
20 checks passed

Uh oh!

There was an error while loading. Please reload this page.

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Nov 8, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 886c794856

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread crates/vm/src/builtins/code.rs
// replace inputs reach this check only after structural validation.
let names = intern_name_tuple(&self.names, vm)?;
let varnames = intern_name_tuple(&self.varnames, vm)?;
let cellvars = intern_name_tuple(&self.cellvars, vm)?;

@chatgpt-codex-connector chatgpt-codex-connector Bot Nov 8, 2026

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

P2 Badge Validate cell names after merging them with locals

When replacing an existing cell variable with a str subclass of the same name—for example, co_cellvars=(S('x'),) where class S(str): pass and x is also a local—this now raises SystemError because intern_name_tuple requires exact strings before the cell can be merged with its local. The previous implementation accepted this replacement, and a reference probe on CPython 3.14.4 also accepts it: the merged cell entry is discarded before exact-string interning of the combined locals-plus names. Perform the cell/local merge before enforcing exactness on the retained names, so this otherwise unchanged, valid closure remains replaceable.

Useful? React with 👍 / 👎.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments
@youknowone youknowone youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@youknowdot @youknowone @codex
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.