Add sys._stdlib_dir#6798
Conversation
📝 WalkthroughWalkthroughIntroduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
Sorry, something went wrong.
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin stdlib_dir |
Sorry, something went wrong.
f42eaaf to
22569fa
Compare
January 19, 2026 14:03
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/pylib/build.rs`:
- Around line 14-31: The build script currently calls
canonicalize(...).to_str().unwrap(), which can panic on non‑UTF‑8 Windows paths;
change the emission to use a lossy conversion from the canonicalized Path (e.g.,
Path::to_string_lossy on the value returned by std::fs::canonicalize(&lib_path))
and print that into the cargo:rustc-env=win_lib_path value, and if
std::fs::canonicalize(&lib_path) returns an Err emit a visible build warning via
println!("cargo:warning=...") instead of silently skipping so missing
win_lib_path is obvious; update the code paths referencing canonicalized_path
and to_str().unwrap() accordingly.
Sorry, something went wrong.
7371d30 to
b212d9a
Compare
January 19, 2026 15:32
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/codegen/src/compile.rs`:
- Around line 6983-6986: The DictMerge path currently uses the first mapping as
the accumulator which can mutate a caller-provided **kwargs dict; change
compile.rs so that when merging multiple mappings (the loop around emit!(self,
Instruction::DictMerge { index: 1 })), you first allocate a fresh empty
accumulator and merge each mapping into that fresh map instead of using the
first input as the target; specifically, before the for-loop that emits
Instruction::DictMerge, emit an instruction to create a new empty dict (so the
accumulator is new) and then merge all incoming `**` groups into that fresh
accumulator (update the code that emits Instruction::DictMerge and the
surrounding logic in the same block to use the new accumulator).
🧹 Nitpick comments (1)
crates/vm/src/frame.rs (1)
962-962: Inconsistent downcast pattern withDictUpdate.
DictUpdate(line 934) uses a safe downcast with.expect():let dict = dict.downcast_ref::<PyDict>().expect("exact dict expected");But
DictMergeuses an unsafe downcast without aSAFETYcomment explaining the invariant. Consider using the same safe pattern asDictUpdatefor consistency and defensive coding, or at minimum add aSAFETYcomment like the other unsafe downcasts in this file (e.g.,ListAppend,ListExtend).♻️ Suggested fix for consistency
- let dict: &Py<PyDict> = unsafe { dict_ref.downcast_unchecked_ref() }; + let dict: &Py<PyDict> = unsafe { + // SAFETY: compiler guarantees correct type + dict_ref.downcast_unchecked_ref() + };Or use the safe pattern:
- let dict: &Py<PyDict> = unsafe { dict_ref.downcast_unchecked_ref() }; + let dict = dict_ref.downcast_ref::<PyDict>().expect("exact dict expected");
Sorry, something went wrong.
c9e25ea to
3867a0a
Compare
January 19, 2026 17:12
3867a0a to
99f3e3e
Compare
January 19, 2026 17:32
b76906a
into
RustPython:main
Jan 19, 2026
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.