Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Dec 11, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling with more accurate error messages in various internal code paths.
  • Refactor

    • Internal code cleanup and reorganization of module imports for improved maintainability.
  • Style

    • Added lint suppression annotations to enhance code quality and reduce compiler warnings.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 11, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This PR systematically replaces unreachable!() macros with unimplemented!() in py_new/slot_new implementations across builtin and stdlib modules, removes identifier from prelude imports in several files, and adds targeted lint-suppression attributes for existing code patterns.

Changes

Cohort / File(s) Summary
Replace unreachable! → unimplemented! in py_new/slot_new implementations
crates/stdlib/src/contextvars.rs, crates/stdlib/src/ssl.rs, crates/vm/src/builtins/bool.rs, crates/vm/src/builtins/classmethod.rs, crates/vm/src/builtins/int.rs, crates/vm/src/builtins/object.rs, crates/vm/src/builtins/staticmethod.rs, crates/vm/src/builtins/type.rs, crates/vm/src/builtins/weakref.rs, crates/vm/src/exceptions.rs, crates/vm/src/stdlib/ast/python.rs, crates/vm/src/stdlib/ctypes/array.rs, crates/vm/src/stdlib/ctypes/base.rs, crates/vm/src/stdlib/ctypes/function.rs, crates/vm/src/stdlib/ctypes/pointer.rs, crates/vm/src/stdlib/ctypes/structure.rs, crates/vm/src/stdlib/ctypes/union.rs, crates/vm/src/stdlib/typevar.rs
Systematically changed panic macro from unreachable!("use slot_new") to unimplemented!("use slot_new") in constructor methods, altering error signaling from internal-invariant violations to explicit unimplemented paths
Remove identifier import from prelude
crates/vm/src/builtins/bool.rs, crates/vm/src/builtins/complex.rs, crates/vm/src/builtins/type.rs, crates/vm/src/class.rs, crates/vm/src/function/protocol.rs, crates/vm/src/stdlib/itertools.rs, crates/vm/src/stdlib/operator.rs, crates/vm/src/types/slot.rs, crates/vm/src/vm/vm_object.rs
Removed identifier from prelude imports; resolves references through alternate import paths
Lint suppression and formatting adjustments
crates/codegen/src/unparse.rs, crates/stdlib/src/os.rs
Added clippy lint-allow attributes wrapping existing code patterns (const assertions in unparse.rs, optional pointer mappings in os.rs)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

  • Attention areas: Verify that the identifier import removals do not break compile-time macro resolution in affected files (especially builtins/complex.rs, builtins/type.rs, function/protocol.rs, stdlib/operator.rs); confirm that moving identifier resolution to alternate import paths does not introduce unintended symbol shadowing or missing references.
  • Cross-check that the unreachable!unimplemented! swaps maintain consistent error handling patterns and do not mask legitimate control-flow assumptions elsewhere in the codebase.

Possibly related PRs

  • Fix nightly clippy warnings #5803 — Modifies overlapping files and the same functions (e.g., PyBaseObject::py_new) with stylistic and diagnostic changes to panic messages.
  • ctypes struct/union/array #6309 — Affects multiple ctypes module files (array.rs, base.rs, pointer.rs, structure.rs, union.rs) with similar py_new constructor modifications.
  • Fix import ctypes #6304 — Modifies ctypes module constructor code (PyCArray::py_new, etc.) alongside other py_new slot implementations.

Suggested reviewers

  • ShaharNaveh
  • arihant2math
  • coolreader18

Poem

🐰 Hops through the code with careful care,
Swapping unreachable for paths more fair;
Unimplemented shines bright and true,
Imports trimmed down, control flow renewed!
With lint flags placed and logic preserved,
This refactor's rewards are well-deserved!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f379ea8 and 6df0374.

📒 Files selected for processing (27)
  • crates/codegen/src/unparse.rs (1 hunks)
  • crates/stdlib/src/contextvars.rs (2 hunks)
  • crates/stdlib/src/ssl.rs (1 hunks)
  • crates/vm/src/builtins/bool.rs (1 hunks)
  • crates/vm/src/builtins/classmethod.rs (1 hunks)
  • crates/vm/src/builtins/complex.rs (0 hunks)
  • crates/vm/src/builtins/int.rs (1 hunks)
  • crates/vm/src/builtins/object.rs (1 hunks)
  • crates/vm/src/builtins/staticmethod.rs (1 hunks)
  • crates/vm/src/builtins/type.rs (1 hunks)
  • crates/vm/src/builtins/weakref.rs (1 hunks)
  • crates/vm/src/class.rs (0 hunks)
  • crates/vm/src/exceptions.rs (1 hunks)
  • crates/vm/src/function/protocol.rs (0 hunks)
  • crates/vm/src/stdlib/ast/python.rs (1 hunks)
  • crates/vm/src/stdlib/ctypes/array.rs (2 hunks)
  • crates/vm/src/stdlib/ctypes/base.rs (1 hunks)
  • crates/vm/src/stdlib/ctypes/function.rs (1 hunks)
  • crates/vm/src/stdlib/ctypes/pointer.rs (1 hunks)
  • crates/vm/src/stdlib/ctypes/structure.rs (2 hunks)
  • crates/vm/src/stdlib/ctypes/union.rs (2 hunks)
  • crates/vm/src/stdlib/itertools.rs (1 hunks)
  • crates/vm/src/stdlib/operator.rs (0 hunks)
  • crates/vm/src/stdlib/os.rs (1 hunks)
  • crates/vm/src/stdlib/typevar.rs (2 hunks)
  • crates/vm/src/types/slot.rs (0 hunks)
  • crates/vm/src/vm/vm_object.rs (0 hunks)

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone marked this pull request as ready for review December 11, 2025 16:13
@youknowone youknowone merged commit bfd873a into RustPython:main Dec 11, 2025
11 of 13 checks passed
@youknowone youknowone deleted the clippy branch December 11, 2025 16:13
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.

1 participant