Skip to content

correct builtins type under function#6745

Merged
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:correct-builtins-type
Jan 16, 2026
Merged

correct builtins type under function#6745
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:correct-builtins-type

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Jan 16, 2026 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Refactor

    • Improved internal handling of builtin references during function initialization and argument processing, enhancing consistency and reliability of builtin access throughout the function lifecycle.

  • Tests

    • Added test validation to verify correct behavior and type checking of function-level builtin attributes.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

The changes refactor PyFunctionNewArgs struct field signatures, converting OptionalArg types to Option, renaming defaults to argdefs, and enhance builtins resolution logic to fallback to vm.builtins.dict() with module unwrapping. The py_new constructor is updated to validate closure length against code.freevars. A test is added to verify that function \builtins resolves to a dict.

Changes

Cohort / File(s) Summary
PyFunction signature and constructor updates
crates/vm/src/builtins/function.rs		 PyFunctionNewArgs struct fields refactored: defaults renamed to argdefs, types changed from OptionalArg to Option; closure and kwdefaults similarly updated. py_new constructor adds closure length validation against code.freevars. Builtins resolution improved with fallback to vm.builtins.dict() and module \dict unwrapping.
Test coverage
extra_tests/snippets/builtins_module.py		 New test function added with runtime assertion that test_func.__builtins__ is a dict instance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 Hoppy defaults find their way,
From OptionalArg to Option's ray,
Closure validation stands so tall,
Builtins unwrapped—dict for all!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'correct builtins type under function' accurately summarizes the main changes: fixing builtins type handling in function initialization, including struct field changes from OptionalArg to Option types and builtins resolution logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 16, 2026

Code has been automatically formatted

The code in this PR has been formatted using:

  • ruff format
    Please pull the latest changes before pushing again:
git pull origin correct-builtins-type

@youknowone youknowone marked this pull request as ready for review January 16, 2026 13:57
@youknowone youknowone changed the title correct buitins type correct builtins type under function Jan 16, 2026
@youknowone youknowone merged commit e9a57d1 into RustPython:main Jan 16, 2026
13 checks passed
@youknowone youknowone deleted the correct-builtins-type branch January 16, 2026 15:00
@coderabbitai coderabbitai bot mentioned this pull request Feb 5, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone