Skip to content

Propagate module initialization and attribute control errors - #9712

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix/module-controls-20261104
Nov 5, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix/module-controls-20261104

Conversation

@youknowdot

Copy link
Copy Markdown
Contributor

Summary

Align Python 3.15 ModuleType initialization, attribute and annotation controls in nine Rust source files (+161/-149).

  • Propagate dictionary callback failures during module initialization instead of expecting writes to succeed. Preserve the original string name and arbitrary doc object.
  • Reuse one annotation/dir dictionary loader that honors overridden dict, while performing subsequent dictionary operations on raw storage. Preserve callback errors distinctly from missing keys and return direct dir hook results unchanged.
  • Let descriptor AttributeError fall through to module getattr. Optional lookups skip missing-attribute diagnostics and their metadata callbacks.
  • Share fallible import-spec initialization checks, propagate origin/state callback errors, and consult the parent's _uninitialized_submodules for circular-import diagnostics. Preserve metadata lookup and temporary-owner order.
  • Add try_new_module for fallible initialization, propagate native-definition initialization errors, and use the fallible path for WebAssembly injection after user code has populated its globals.

Validation

Compared against pinned CPython d220409a13b77de7a6c30abe16ea90db01528edc moduleobject.c and import.c. All 45 external reference checks pass, including collisions in all five initialization writes, annotation dictionary overrides, dictionary-subclass bypass, callback failures, descriptor fallback and optional lookup behavior. Independent source/lifetime review passed, including the native initialization callers and generated module helper.

Final threaded cargo check passed (1m12s), all-target Clippy passed (3m11s) with exactly the 12 unchanged baseline diagnostics, and normal commit hooks passed. The final lifetime oracle also passed independently in three fresh CPython processes.

Limits and integration

The initialization panic path is source-established, not a native RustPython crash reproduction. Native execution remains unperformed because its required safety guard is not verified. No repository tests, markers, Lib files or workflows were changed. Hosted runtime CI remains necessary.

The changed WebAssembly call site was source-reviewed but its separate wasm/non-threaded dependency graph was not compiled locally due limited disk space. The threaded VM/stdlib check does not substitute for that target validation.

The existing public new_module wrapper remains infallible for callback-free dictionaries. External embedders supplying user-controlled dictionaries must use try_new_module. All in-tree pre-initialization dictionary callers were reviewed; WebAssembly's executed-globals path is migrated.

#9700's Unicode annotation-result diagnostic change remains separate and should be preserved on composition. Existing import-from name/sys.modules suppression and file fallback differences, super error controls, and broader import behavior are outside this patch. The shared spec changes do not claim full import diagnostic parity.

Assisted-by: OpenAI Codex:model version not exposed
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 7e9a375 and 3d2e10b.

Files selected for processing: 9

  • crates/derive-impl/src/pymodule.rs
  • crates/vm/src/builtins/dict.rs
  • crates/vm/src/builtins/module.rs
  • crates/vm/src/import.rs
  • crates/vm/src/lazy_import.rs
  • crates/vm/src/stdlib/_imp.rs
  • crates/vm/src/vm/mod.rs
  • crates/vm/src/vm/vm_new.rs
  • crates/wasm/src/vm_class.rs
  • 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

@youknowone
youknowone marked this pull request as ready for review November 5, 2026 03:11
@youknowone
youknowone enabled auto-merge (squash) November 5, 2026 03:11
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-05T03:11:28.317199Z
We couldn’t complete this review. Comment @codex review to try again.
3d2e10b 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.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@youknowone
youknowone merged commit fb553a6 into RustPython:main Nov 5, 2026
22 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants