crates/stdlib/src/_sqlite3.rs (2)

2916-2923: Same surrogate-safety concern as register_converter.

This expect_str() usage has the same potential panic risk; please apply the same UTF‑8‑safe handling here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/_sqlite3.rs` around lines 2916 - 2923, The code currently
calls isolation_level.expect_str().bytes(), which can panic on invalid UTF-8 the
same way register_converter did; change the conversion to a UTF-8-safe form
(e.g., use to_string_lossy() and then extend with its as_bytes()) so the string
bytes are produced without panicking, e.g. replace
isolation_level.expect_str().bytes() with
isolation_level.expect_str().to_string_lossy().as_bytes() (and keep the
surrounding s.extend(...) and self._exec(&s, vm) logic).

---

2192-2202: Same surrogate-safety concern as register_converter.

This expect_str() usage has the same potential panic risk; please apply the same UTF‑8‑safe handling here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/_sqlite3.rs` around lines 2192 - 2202, The code uses
expect_str() on both name and obj which can panic on invalid UTF-8; replace
these with a UTF-8-safe conversion (use the PyStr->to_str / try-to-str API that
returns Result<&str, _> or an Option) for the variables used in the comparison,
handle the error by skipping/breaking the comparison instead of panicking, and
then use the resulting &str to build a_iter and b_iter as before; locate the
occurrences in the loop that iterates self.description (the block that uses
description.iter().zip(0..), name, obj, a_iter, b_iter and returns
self.data.getitem_by_index(vm, i)) and change the expect_str() calls to the safe
conversion and appropriate error handling.

crates/stdlib/src/binascii.rs (1)

851-853: Optional: tighten the new_binascii_error wrapper signature to match the migrated API

Now that vm.new_exception_msg accepts Into<Wtf8Buf>, the wrapper can take impl Into<Wtf8Buf> (or just Wtf8Buf) directly, which lets every call site drop the unnecessary .to_owned() allocation on string literals.

♻️ Proposed refactor

-fn new_binascii_error(msg: String, vm: &VirtualMachine) -> PyBaseExceptionRef {
-    vm.new_exception_msg(decl::error_type(vm), msg.into())
+fn new_binascii_error(msg: impl Into<rustpython_vm::common::wtf8::Wtf8Buf>, vm: &VirtualMachine) -> PyBaseExceptionRef {
+    vm.new_exception_msg(decl::error_type(vm), msg.into())
 }

Call sites can then pass &str directly:

-    return Err(super::new_binascii_error("Odd-length string".to_owned(), vm));
+    return Err(super::new_binascii_error("Odd-length string", vm));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/binascii.rs` around lines 851 - 853, Change the signature
of new_binascii_error to accept impl Into<Wtf8Buf> (or Wtf8Buf) instead of
String and forward that into vm.new_exception_msg (which now accepts
Into<Wtf8Buf>); update the function declaration new_binascii_error(msg: impl
Into<Wtf8Buf>, vm: &VirtualMachine) -> PyBaseExceptionRef and call
vm.new_exception_msg(decl::error_type(vm), msg) so call sites can pass &str
without .to_owned().

crates/vm/src/macros.rs (1)

191-199: Extract metavariable expansion outside the unsafe block instead of suppressing the lint.

#[allow(clippy::macro_metavars_in_unsafe)] is a workaround for a lint whose intended fix is to bind the expression before the unsafe block. Doing so keeps the safe-code/unsafe-code boundary explicit and removes the need for the suppression entirely:

♻️ Proposed refactor

     ($as_ctx:expr, $name:ident) => {{
         // Safety: All known identifiers are ascii strings.
-        #[allow(clippy::macro_metavars_in_unsafe)]
-        unsafe {
-            $crate::builtins::PyUtf8StrInterned::from_str_interned_unchecked(
-                $as_ctx.as_ref().names.$name
-            )
-        }
+        let interned = $as_ctx.as_ref().names.$name;
+        unsafe { $crate::builtins::PyUtf8StrInterned::from_str_interned_unchecked(interned) }
     }};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/macros.rs` around lines 191 - 199, The macro currently silences
clippy::macro_metavars_in_unsafe; instead bind the metavariable expansion
outside the unsafe block: evaluate and store the expression
$as_ctx.as_ref().names.$name into a local let (e.g. names_val) before entering
unsafe, then call
$crate::builtins::PyUtf8StrInterned::from_str_interned_unchecked(names_val)
inside the unsafe block and remove the
#[allow(clippy::macro_metavars_in_unsafe)] attribute; this keeps the safe/unsafe
boundary explicit for the macro arm that uses $as_ctx and $name.

crates/vm/src/stdlib/posix.rs (1)

2374-2383: Unnecessary inner block in SysconfName::TryFromObject.

The { let name = s.as_str(); ... } scope is redundant — s is in scope through the entire Err arm and the lifetime of the borrow doesn't require restriction.

♻️ Proposed simplification

-                    {
-                        let name = s.as_str();
-                        name.parse::<SysconfVar>().or_else(|_| {
-                            if name == "SC_PAGESIZE" {
-                                Ok(SysconfVar::SC_PAGESIZE)
-                            } else {
-                                Err(vm.new_value_error("unrecognized configuration name"))
-                            }
-                        })? as i32
-                    }
+                    let name = s.as_str();
+                    name.parse::<SysconfVar>().or_else(|_| {
+                        if name == "SC_PAGESIZE" {
+                            Ok(SysconfVar::SC_PAGESIZE)
+                        } else {
+                            Err(vm.new_value_error("unrecognized configuration name"))
+                        }
+                    })? as i32

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/posix.rs` around lines 2374 - 2383, In
SysconfName::TryFromObject, remove the unnecessary inner block and local
shadowing of name; directly call s.as_str().parse::<SysconfVar>().or_else(|_| {
if s.as_str() == "SC_PAGESIZE" { Ok(SysconfVar::SC_PAGESIZE) } else {
Err(vm.new_value_error("unrecognized configuration name")) } })? as i32 so the
borrow of s lasts naturally without the extra scope and the code is simplified
while keeping the same logic for SysconfVar and the SC_PAGESIZE special-case.

crates/stdlib/src/unicodedata.rs (1)

128-139: to_str() chain is correct; minor note on the error-message Display for surrogate inputs.

When name.to_str() returns None (lone-surrogate string), the chain falls through to the KeyError at line 134 — which is the correct Python behavior since every valid Unicode character name is ASCII. No correctness concern there.

The let_chains syntax (lines 128–129) is fully supported: the project's MSRV is 1.93.0, which exceeds the Rust 1.88.0 requirement where let_chains stabilized (June 2025), and the project uses the 2024 edition where the feature is available.

One small point: the error message on line 136 formats name via its Display impl:

format!("undefined character name '{name}'")

If name contains lone surrogates, Display for PyStrRef will likely substitute U+FFFD replacement characters, so the error message won't faithfully reproduce the raw surrogate codepoints. CPython would surface the exact code units. This is well within acceptable range for an improvement, but if tighter CPython parity is desired down the line, the error path could use name.as_wtf8() with an escaped representation instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/unicodedata.rs` around lines 128 - 139, The error message
currently formats the PyStrRef `name` via its Display implementation
(format!("undefined character name '{name}'")) which may replace lone surrogates
with U+FFFD; modify the KeyError formatting in the branch that returns
vm.new_key_error so it emits an escaped representation preserving raw code units
(e.g., use name.as_wtf8() and produce a diagnostic-escaped string) instead of
relying on Display; update the call site that builds the message passed into
vm.ctx.new_str(...) so the message shows exact code units for
surrogate-containing inputs while leaving the rest of the logic (name.to_str(),
unicode_names2::character, self.check_age) unchanged.

crates/stdlib/src/ssl.rs (1)

3913-3921: Consider aligning setter arg type with PyUtf8StrRef.

This would make the setter consistent with the updated call sites and remove the extra try_into_utf8 step.

♻️ Suggested adjustment

-        fn set_server_hostname(
-            &self,
-            value: Option<PyStrRef>,
-            vm: &VirtualMachine,
-        ) -> PyResult<()> {
+        fn set_server_hostname(
+            &self,
+            value: Option<PyUtf8StrRef>,
+            vm: &VirtualMachine,
+        ) -> PyResult<()> {
             // Check if handshake is already done
             if *self.handshake_done.lock() {
                 return Err(
                     vm.new_value_error("Cannot set server_hostname on socket after handshake")
                 );
             }

             // Validate hostname
             let hostname_string = value
                 .map(|s| {
-                    let utf8 = s.try_into_utf8(vm)?;
-                    validate_hostname(utf8.as_str(), vm)?;
-                    Ok::<String, _>(utf8.as_str().to_owned())
+                    validate_hostname(s.as_str(), vm)?;
+                    Ok::<String, _>(s.as_str().to_owned())
                 })
                 .transpose()?;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/ssl.rs` around lines 3913 - 3921, The setter currently
accepts an Option<PyObjectRef> (or similar) and calls try_into_utf8 inside the
closure; change the setter signature to accept Option<PyUtf8StrRef> (or
Option<&PyUtf8StrRef> as appropriate) so callers can pass already-validated
UTF-8 refs, then remove the try_into_utf8 call in the closure and use the
PyUtf8StrRef's .as_str() directly for validate_hostname and when creating the
owned String; update uses around server_hostname.write() assignment to take the
Option<PyUtf8StrRef> and set hostname_string = value.map(|s| {
validate_hostname(s.as_str(), vm)?; Ok::<String, _>(s.as_str().to_owned())
}).transpose()? accordingly.

crates/stdlib/src/socket.rs (1)

2911-2916: Simplify by accepting PyUtf8StrRef directly (like sethostname/inet_aton/inet_pton).

gethostbyaddr, gethostbyname, and gethostbyname_ex still take PyStrRef and explicitly call .try_into_utf8(vm).map_err(IoOrPyException::from)?. Since IoOrPyException: From<PyBaseExceptionRef>, the #[pyfunction] macro can propagate a TryFromObject error as IoOrPyException::Py automatically — the same end result as the current explicit conversion.

♻️ Proposed simplification (shown for `gethostbyaddr`; apply identically to `gethostbyname` and `gethostbyname_ex`)

 #[pyfunction]
 fn gethostbyaddr(
-    addr: PyStrRef,
+    addr: PyUtf8StrRef,
     vm: &VirtualMachine,
 ) -> Result<(String, PyListRef, PyListRef), IoOrPyException> {
-    let addr = addr.try_into_utf8(vm).map_err(IoOrPyException::from)?;
     let addr = get_addr(vm, addr, c::AF_UNSPEC)?;

Also applies to: 2928-2930, 2941-2943

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/socket.rs` around lines 2911 - 2916, Change the functions
gethostbyaddr, gethostbyname, and gethostbyname_ex to accept PyUtf8StrRef (like
sethostname/inet_aton/inet_pton) instead of PyStrRef and remove the explicit
.try_into_utf8(vm).map_err(IoOrPyException::from)? conversions; let the
#[pyfunction] macro propagate TryFromObject errors as IoOrPyException::Py
automatically, then use the received PyUtf8StrRef directly in the existing
get_addr calls (e.g., replace the local addr conversion in gethostbyaddr and the
analogous conversions in gethostbyname and gethostbyname_ex).

crates/vm/src/stdlib/ctypes/array.rs (1)

4-5: Run rustfmt & clippy before merge.

Please confirm cargo fmt and cargo clippy were run for these Rust changes.
As per coding guidelines: "Follow the default rustfmt code style by running cargo fmt to format Rust code" and "Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints that are introduced by your changes."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/ctypes/array.rs` around lines 4 - 5, Run rustfmt and
clippy for the VM crate and fix any formatting or lints introduced by these
changes: run cargo fmt for crates/vm (ensure files like
crates/vm/src/stdlib/ctypes/array.rs are formatted) and run cargo clippy --fix
or cargo clippy to surface warnings; address all clippy suggestions and warnings
(including any issues related to imports such as LazyLock and SaturatedSliceIter
or unused/incorrect imports) before merging.

crates/vm/src/stdlib/ctypes/union.rs (1)

620-624: Silent skip when field name isn't valid UTF-8 — verify this is intentional.

If downcast_ref::<PyUtf8Str>() fails (e.g., a field name contains surrogates), the positional argument is silently skipped without error. In practice, process_fields (line 220) already validates field names as PyUtf8Str, so this path shouldn't be reachable with malformed names. However, the structure.rs counterpart at line 751 has the identical pattern — so at least they're consistent.

If you want extra safety, consider logging or returning an error when the downcast fails, rather than silently skipping.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/ctypes/union.rs` around lines 620 - 624, The code
currently silently skips a field when tuple.first().downcast_ref::<PyUtf8Str>()
fails (i.e., non-UTF-8 names); update the handling in the union field-processing
block (the branch using PyTuple and downcast_ref::<PyUtf8Str>()) to treat a
failed downcast as an explicit error instead of silently skipping: return or
propagate a descriptive error (e.g., invalid field name encoding) from the
surrounding function (see process_fields and the analogous handling in
structure.rs) so callers get notified when a field name is not valid UTF-8.

crates/vm/src/utils.rs (1)

1-2: Pre-merge hygiene: please run rustfmt + clippy for this refactor-heavy PR.

These changes touch a lot of core codepaths; it’s worth ensuring no formatting drift and no new clippy warnings slipped in.

As per coding guidelines “Follow the default rustfmt code style by running cargo fmt” and “Always run clippy (cargo clippy) before completing tasks and fix any warnings or lints that are introduced by your changes”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/utils.rs` around lines 1 - 2, This PR introduced formatting and
potential lints in core code; run rustfmt and clippy and fix any resulting
issues: run cargo fmt (or rustfmt) to apply default formatting to
crates/vm/src/utils.rs and the repo, then run cargo clippy --all-targets
--all-features -- -D warnings to surface lints and address them (fix unused
imports like Wtf8/Wtf8Buf if unused, correct style issues, replace
unwraps/expect where appropriate, and resolve any borrow/ownership or
performance warnings reported for functions in utils.rs). Ensure the file
symbols (Wtf8, Wtf8Buf and any functions in utils.rs) compile cleanly with no
clippy warnings before merging.

crates/vm/src/builtins/memory.rs (1)

808-830: Good: memoryview.cast() now requires UTF‑8-safe format strings (PyUtf8StrRef).

This change removes the previous “may panic on surrogates” footgun by ensuring cast_to_1d() can safely use format.as_str() and still keep BufferDescriptor.format in sync.

If you want, I can help draft a small regression test that passes a surrogate-containing format and asserts an exception (not a panic).

Also applies to: 957-963

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/memory.rs` around lines 808 - 830, Change calls that
accept raw strings to use PyUtf8StrRef and then use format.as_str() so
BufferDescriptor.format stays UTF‑8 safe: in the cast_to_1d implementation
replace any use of a raw format string with a PyUtf8StrRef parameter, call
Self::parse_format(format.as_str(), vm) and set BufferDescriptor.format to
format.as_str().to_owned().into(); ensure the same pattern (use PyUtf8StrRef,
parse_format(format.as_str(), vm), and assign format via
format.as_str().to_owned().into()) is applied to the other memoryview cast
routine(s) that construct BufferDescriptor (the other function that currently
calls parse_format/uses format.as_str()/builds dim_desc), keeping the rest of
the struct fields (buffer.clone(), AtomicCell::new(false), OnceCell::new(),
etc.) unchanged.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

crates/vm/src/stdlib/msvcrt.rs (2)

83-90: ⚠️ Potential issue | 🟠 Major

Avoid expect_str() to prevent surrogate-triggered panics in putwch.

PyStrRef can contain surrogates; expect_str() can panic and crash the VM. Use WTF‑8 code points instead.

✅ Suggested surrogate-safe change

-        let c = s
-            .expect_str()
-            .chars()
-            .exactly_one()
-            .map_err(|_| vm.new_type_error("putch() argument must be a string of length 1"))?;
-        unsafe { suppress_iph!(_putwch(c as u16)) };
+        let c = s
+            .as_wtf8()
+            .code_points()
+            .exactly_one()
+            .map_err(|_| vm.new_type_error("putch() argument must be a string of length 1"))?;
+        unsafe { suppress_iph!(_putwch(c.to_u32() as u16)) };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/msvcrt.rs` around lines 83 - 90, The putwch function
uses PyStrRef::expect_str(), which can panic on surrogate-containing strings and
crash the VM; update putwch to extract the string via a surrogate-safe
WTF‑8/UTF‑8 accessor (do not call expect_str()), iterate or decode WTF‑8 code
points to ensure exactly one Unicode scalar or code unit as intended, and map
errors to vm.new_type_error("putch() argument must be a string of length 1")
instead of panicking; locate the logic in putwch and replace the
expect_str()/chars().exactly_one() path with a surrogate-safe WTF‑8
check/iterator that returns a controlled PyResult error when the length is not
one, then call _putwch with the correct u16 value.

---

108-114: ⚠️ Potential issue | 🟠 Major

Avoid expect_str() to prevent surrogate-triggered panics in ungetwch.

Same issue as putwch: expect_str() can panic on surrogate-containing strings.

✅ Suggested surrogate-safe change

-        let c =
-            s.expect_str().chars().exactly_one().map_err(|_| {
-                vm.new_type_error("ungetwch() argument must be a string of length 1")
-            })?;
-        let ret = unsafe { suppress_iph!(_ungetwch(c as u32)) };
+        let c = s
+            .as_wtf8()
+            .code_points()
+            .exactly_one()
+            .map_err(|_| vm.new_type_error("ungetwch() argument must be a string of length 1"))?;
+        let ret = unsafe { suppress_iph!(_ungetwch(c.to_u32())) };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/msvcrt.rs` around lines 108 - 114, In ungetwch, avoid
calling s.expect_str() (which can panic on surrogate-containing strings);
instead obtain the Python string without expect and iterate its characters
safely (as in the putwch fix) so you call .chars().exactly_one().map_err(...) on
the surrogate-safe string view, then pass the resulting code point to the unsafe
suppress_iph!(_ungetwch(c as u32)) call; update the code around ungetwch and the
character-extraction logic to mirror the surrogate-safe approach used for
putwch.

crates/vm/src/stdlib/_wmi.rs (1)

563-565: ⚠️ Potential issue | 🔴 Critical

expect_str() still panics on surrogate-containing strings — PR objective unmet

Replacing as_str() with expect_str() is a rename-only change; expect_str() still calls .expect(…) and will panic on any PyStr that holds lone surrogates. This file needs to complete the migration to to_str() with proper error propagation, which is exactly what the rest of the PR does elsewhere.

🐛 Proposed fix

-        let query_str = query.expect_str();
+        let query_str = query.to_str().map_err(|e| vm.new_value_error(e.to_string()))?;

query_str is only used for the ASCII prefix check (lines 566-569) and the wide_str(query_str) call (line 573), both of which accept &str, so no further changes are needed downstream.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/_wmi.rs` around lines 563 - 565, The code still calls
expect_str() which can panic on surrogate-containing PyStr; change exec_query to
call query.to_str(vm)? (propagating the PyErr) and bind query_str as &str for
the ASCII prefix check and wide_str(query_str) call so errors are returned
instead of panicking; update the usage of query_str accordingly in exec_query.

crates/wasm/src/browser_module.rs (1)

66-68: ⚠️ Potential issue | 🔴 Critical

Replace expect_str() with fallible conversion to avoid WASM panics.

expect_str() will panic on surrogate-containing Python strings; these inputs are user-controlled (URLs, headers, selectors, module names) and will abort the WASM runtime instead of raising a Python error. Please switch to to_str() (propagate a Python error) or to_string_lossy() where a lossy fallback is acceptable, and map errors through the existing convert::js_py_typeerror / py_err_to_js_err paths.

#!/bin/bash
# Inspect PyStrRef conversion APIs and error types to wire correct fallible conversion paths.
rg -n "fn expect_str|fn to_str|fn to_string_lossy" crates -g '*.rs' -C3
rg -n "impl .*PyStrRef|impl .*PyStr" crates -g '*.rs' -C3

Also applies to: 73-73, 79-80, 87-88, 95-96, 171-172, 206-208, 215-216, 227-228, 244-245

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/wasm/src/browser_module.rs` around lines 66 - 68, Replace all uses of
expect_str() that can panic (e.g., in the response_format match where
FetchResponseFormat::from_str(vm, s.expect_str()) is called) with a fallible
conversion and propagate a Python error: call s.to_str() (or s.to_string_lossy()
if a lossy fallback is acceptable) and map the Err through the existing
conversion helpers (convert::js_py_typeerror or py_err_to_js_err) before passing
the resulting &str/String into FetchResponseFormat::from_str and other callers;
apply the same change to the other occurrences flagged (headers, url, selector,
module-name conversions) so no user-controlled PyStrRef can trigger a WASM
panic.

crates/vm/src/stdlib/functools.rs (1)

480-492: Lossy format! on &Wtf8 reference still present — surrogates replaced via Display.

The format!("{module_name}.{qualname_str}") at line 486 (and subsequently Wtf8Buf::from(format!("{qualified_name}(")) at line 492) triggers the Display implementation for Wtf8, which substitutes unpaired surrogates with U+FFFD. This was flagged in a previous review and the proposed fix (build the prefix directly with Wtf8Buf::push_wtf8 / push_str) was not applied.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/functools.rs` around lines 480 - 492, The code uses
format! on Wtf8 values (format!("{module_name}.{qualname_str}") and
format!("{qualified_name}(")) which invokes Display and loses unpaired
surrogates; instead construct the strings with Wtf8-aware APIs: avoid format!
and build a Wtf8Buf directly (use Wtf8Buf::new / Wtf8Buf::from and call
push_wtf8 or push_str) to append module_name, a literal ".", qualname_str, and
finally "(" so surrogates are preserved; update the qualified_name creation and
the result initialization (the variables/match arm around
module.downcast::<crate::builtins::PyStr>(), module_name, qualname_str,
qualified_name, and result) to use these push_* methods rather than format!.

crates/vm/src/intern.rs (1)

160-168: as_str() can still panic on interned strings containing surrogates.

StringPool::intern accepts Wtf8Buf and &Wtf8 (lines 292–305), so interned strings aren't guaranteed to be valid UTF-8. The unwrap_or_else(|| panic!(...)) reintroduces exactly the kind of panic this PR aims to eliminate.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/intern.rs` around lines 160 - 168, The current as_str() on
PyInterned<PyStr> calls self.inner.to_str().unwrap_or_else(...) which can still
panic for WTF-8 data with surrogates; replace this panic-prone API by returning
a non-panicking string view (e.g., change as_str() to return Cow<'_, str> and
use self.inner.to_string_lossy() instead of to_str().unwrap_or_else(...)), or
alternatively add a new as_wtf8() that returns &Wtf8 and keep a lossy
as_str_lossy() that returns Cow<'_, str>; update uses of as_str accordingly to
avoid any unwrap/panic on invalid UTF-8.

crates/vm/src/stdlib/_winapi.rs (3)

298-302: ⚠️ Potential issue | 🔴 Critical

expect_str() panics on surrogate-containing strings — contradicts PR objective.

The wstr closure calls s.expect_str() which panics when the PyStr contains surrogates. On Windows, file paths and command lines can legitimately contain unpaired surrogates (e.g., filenames created via UTF-16 APIs). This reintroduces the exact class of panics the PR aims to eliminate.

Consider using s.as_wtf8().to_wide_with_nul() directly (bypassing the WideCString::from_str path) since the Windows APIs accept UTF-16 including surrogates, or use s.to_str() with proper error propagation.

Suggested approach using WTF-8 → wide conversion

         let wstr = |s: PyStrRef| {
-            let ws = widestring::WideCString::from_str(s.expect_str())
-                .map_err(|err| err.to_pyexception(vm))?;
-            Ok(ws.into_vec_with_nul())
+            Ok(s.as_wtf8().to_wide_with_nul())
         };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/_winapi.rs` around lines 298 - 302, The wstr closure
currently calls s.expect_str(), which can panic on PyStrRef values containing
surrogate code points; replace that path with a surrogate-safe conversion by
using s.as_wtf8().to_wide_with_nul() (or an equivalent WTF-8 → UTF-16
conversion) and avoid WideCString::from_str/from_str error branches that assume
valid UTF-8; update the closure (wstr) to take PyStrRef, call as_wtf8(), convert
to a wide Vec<u16> including NUL with to_wide_with_nul(), and propagate any
conversion errors via map_err to the vm rather than using expect_str().

---

399-400: ⚠️ Potential issue | 🔴 Critical

expect_str() in CreateJunction will panic on surrogate paths.

Same issue as in the wstr closure. Windows junction paths can contain surrogates. Use to_str() with error handling or convert via WTF-8 → wide string.

Suggested fix using to_str()

-        let src_path = std::path::Path::new(src_path.expect_str());
-        let dest_path = std::path::Path::new(dest_path.expect_str());
+        let src_str = src_path.to_str().ok_or_else(|| {
+            vm.new_value_error("src_path contains surrogates")
+        })?;
+        let dest_str = dest_path.to_str().ok_or_else(|| {
+            vm.new_value_error("dest_path contains surrogates")
+        })?;
+        let src_path = std::path::Path::new(src_str);
+        let dest_path = std::path::Path::new(dest_str);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/_winapi.rs` around lines 399 - 400, The CreateJunction
implementation currently calls expect_str() when building src_path and dest_path
(and similarly uses the wstr closure), which will panic on Windows paths
containing UTF-16 surrogates; change these to avoid expect_str(): call to_str()
and handle the None case (return a proper error) or convert the OsStr via a
WTF-8 → wide-string path conversion before passing to Windows APIs, updating the
CreateJunction function and the wstr helper to accept and handle
surrogate-containing paths safely instead of unwrapping.

---

419-430: ⚠️ Potential issue | 🔴 Critical

expect_str() in getenvironment will panic on surrogate env vars.

Windows environment variables can contain unpaired surrogates. Both k.expect_str() and v.expect_str() at lines 421 and 423 will panic in that case.

Additionally, line 424 uses k.contains('\0') and v.contains('\0') on &str — this is correct only if expect_str() succeeds, but the code will panic before reaching the null check if surrogates are present.

Consider converting to wide strings via WTF-8 directly rather than going through &str.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/_winapi.rs` around lines 419 - 430, The code in
getenvironment uses PyStrRef::expect_str() for k and v which will panic on
unpaired surrogates; replace the expect_str() calls and subsequent &str
operations with a WTF-8/raw wide-string path (e.g. obtain the WTF-8/wide
representation from the PyStrRef returned by PyStrRef::try_from_object instead
of calling expect_str()), perform the null byte check against the raw/wtf8
bytes, validate the name emptiness and '=' rule using the WTF-8-aware view
(checking the first scalar/code-unit appropriately rather than slicing &str),
and then perform the uppercase/normalization step on a Unicode-safe
representation (or normalize to wide/uppercase after conversion) so no
unpaired-surrogate panics can occur; update references to expect_str(), the
temporary k/v &str usage, and key_upper creation to use the new
WTF-8/wide-string handling.

crates/vm/src/types/slot.rs (1)

1715-1724: ⚠️ Potential issue | 🟠 Major

Break the default repr recursion to avoid stack overflow.
repr_wtf8 and repr_str still call each other when neither is overridden.

Suggested fix (one example)

-    fn repr_str(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<String> {
-        Self::repr_wtf8(zelf, vm).map(|w| w.to_string())
-    }
+    fn repr_str(_zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<String> {
+        Err(vm.new_type_error("repr_str or repr_wtf8 must be implemented"))
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/types/slot.rs` around lines 1715 - 1724, The two methods
repr_wtf8 and repr_str mutually call each other when neither is overridden,
causing infinite recursion; break this by giving repr_wtf8 a non-recursive
default implementation that builds a fallback representation (e.g., using the
object's class/type name and address/ID) instead of calling repr_str; keep
repr_str converting the Wtf8Buf to String, and ensure repr (which currently
calls repr_wtf8) still uses that fallback when no override exists so the
recursion is resolved (update the repr_wtf8 implementation and leave repr_str as
the simple .to_string() conversion).

crates/vm/src/stdlib/builtins.rs (1)

566-575: ⚠️ Potential issue | 🟠 Major

Guard expect_str() to avoid surrogate panics.
This still panics on non‑UTF‑8 prompts; switch to try_into_utf8(vm) and fall back to the std‑I/O path when conversion fails.

🛠️ Suggested safe prompt conversion

-        if use_rustyline {
-            let prompt = prompt.as_ref().map_or("", |s| s.expect_str());
+        let (prompt_utf8, use_rustyline) = match prompt.as_ref() {
+            Some(s) => {
+                let prompt_utf8 = s.try_into_utf8(vm).ok();
+                (prompt_utf8, use_rustyline && prompt_utf8.is_some())
+            }
+            None => (None, use_rustyline),
+        };
+        if use_rustyline {
+            let prompt = prompt_utf8.as_ref().map_or("", |s| s.as_str());
             let mut readline = Readline::new(());
             match readline.readline(prompt) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/builtins.rs` around lines 566 - 575, The code
unconditionally calls expect_str() to build the prompt before using Readline
(inside the use_rustyline branch), which panics on non‑UTF‑8 prompts; replace
that call with a safe conversion using try_into_utf8(vm) (or equivalent) and if
the conversion returns an Err, skip the rustyline branch and fall back to the
normal std‑IO path. Specifically, change the prompt extraction around
expect_str() so the prompt is attempted via try_into_utf8(vm) and only proceed
to create Readline and call readline.readline(prompt) when conversion succeeds;
otherwise treat use_rustyline as false (or jump to the existing std I/O
handling).

crates/vm/src/vm/mod.rs (1)

639-660: ⚠️ Potential issue | 🟡 Minor

Add a separator after msg in unraisable output.
Without it, output concatenates; same issue as the earlier review.

✏️ Suggested tweak

-        let msg_str = if let Some(msg) = msg {
-            format!("Exception ignored {}", msg)
+        let msg_str = if let Some(msg) = msg {
+            format!("Exception ignored {msg}: ")
         } else {
             "Exception ignored in: ".to_owned()
         };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/vm/mod.rs` around lines 639 - 660, The unraisable output
concatenates object repr because the Some(msg) branch of msg_str lacks a
separator; update the construction of msg_str (where msg_str is built from msg
in the let msg_str = if let Some(msg) { ... } else { ... } block) to append a
separator (for example ": " or " in: ") after the interpolated msg so it matches
the None branch's separator before the subsequent write_to_stderr and
repr_result/repr_wtf8 output; ensure the change affects the same msg_str/ msg
handling before write_to_stderr is called.

crates/vm/src/stdlib/ctypes/function.rs (2)

753-759: ⚠️ Potential issue | 🟠 Major

Avoid panic if _type_ is mutated to non‑UTF‑8.

to_str().expect(...) can panic if Python mutates _type_ to a non‑UTF‑8 value; treat invalid UTF‑8 as a non‑match instead.

🛠️ Suggested fix

-        let c = s
-            .to_str()
-            .expect("_type_ is validated as ASCII at type creation");
-        if c.len() == 1 && "sPzUZXO".contains(c) {
-            return true;
-        }
+        if let Some(c) = s.to_str() {
+            if c.len() == 1 && "sPzUZXO".contains(c) {
+                return true;
+            }
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/ctypes/function.rs` around lines 753 - 759, The code
currently calls s.to_str().expect(...) which can panic if the Python-side
`_type_` string contains invalid UTF‑8; change the logic to safely handle
invalid UTF‑8 by attempting to convert with to_str() and only proceeding when it
returns Ok(&str) (treat Err as a non‑match), so replace the expect-based
conversion in the block that reads ctype.get_attr("_type_", vm) and downcasts to
PyStr with a guarded to_str() check before using c (referencing the PyStr value
and the existing conditional that checks c.len() == 1 && "sPzUZXO".contains(c)).

---

282-289: ⚠️ Potential issue | 🟠 Major

Use platform‑aware wchar_t conversion (UTF‑16 here breaks non‑Windows).

encode_wide() yields UTF‑16; on Unix, libc::wchar_t is typically 32‑bit, so this path will mis-encode wide strings.

🛠️ Suggested fix

-        // Convert to null-terminated UTF-16, preserving lone surrogates
-        let wide: Vec<u16> = s
-            .as_wtf8()
-            .encode_wide()
-            .chain(core::iter::once(0))
-            .collect();
-        let wide_bytes: Vec<u8> = wide.iter().flat_map(|&x| x.to_ne_bytes()).collect();
-        let keep = vm.ctx.new_bytes(wide_bytes);
+        // Convert to platform wchar_t bytes (UTF-16 on Windows, UTF-32 on Unix)
+        let wide_bytes = super::base::str_to_wchar_bytes(s.as_wtf8());
+        let keep = vm.ctx.new_bytes(wide_bytes);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/ctypes/function.rs` around lines 282 - 289, The current
branch in the PyStr handling uses encode_wide() and treats wchar_t as 16-bit
which breaks on platforms where libc::wchar_t is 32-bit; change the conversion
in the value.downcast_ref::<PyStr>() branch (symbols: s, wide, wide_bytes) to be
platform-aware: check std::mem::size_of::<libc::wchar_t>() (or cfg(target_os =
"windows")) and if it's 2 bytes keep the existing UTF‑16 flow
(s.as_wtf8().encode_wide() + 0u16), otherwise produce UTF‑32 code units by
iterating the WTF-8 string into Unicode scalar values (e.g. s.as_wtf8().chars()
or equivalent that preserves lone surrogates semantics), append a 0u32
terminator, and then convert those u32 units to native-endian bytes for
wide_bytes so the produced buffer matches the platform's wchar_t size.

crates/vm/src/builtins/interpolation.rs (1)

205-223: ⚠️ Potential issue | 🟠 Major

Avoid push_str on PyStr reprs to prevent surrogate panics.
expr_repr / spec_repr are PyStr values; push_str(&expr_repr) routes through AsRef<str> and can panic on WTF‑8 surrogates. Use push_wtf8(..) like the other parts.

🐛 Proposed fix

-        result.push_str(&expr_repr);
+        result.push_wtf8(expr_repr.as_wtf8());
         result.push_str(", ");
         if vm.is_none(&zelf.conversion) {
             result.push_str("None");
         } else {
             result.push_wtf8(zelf.conversion.repr(vm)?.as_wtf8());
         }
         result.push_str(", ");
-        result.push_str(&spec_repr);
+        result.push_wtf8(spec_repr.as_wtf8());

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/interpolation.rs` around lines 205 - 223, In repr_wtf8
replace the uses of push_str(&expr_repr) and push_str(&spec_repr) — which call
AsRef<str> and can panic on WTF‑8 surrogates — with push_wtf8 using the
PyStr/WTF‑8 backing (e.g. call result.push_wtf8(expr_repr.as_wtf8()) and
result.push_wtf8(spec_repr.as_wtf8())); keep the other push_wtf8 usages and the
conversion handling in the repr_wtf8 function unchanged.

crates/vm/src/stdlib/ctypes/base.rs (1)

366-373: Clarify the _type_ ASCII validation scope in the comment.
The to_str().expect(...) assumption is safe only for the simple-type _type_ path; consider noting that explicitly so the invariant is clear to future readers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/ctypes/base.rs` around lines 366 - 373, Clarify that the
ASCII validation referenced by the expect() call applies only to the simple-type
`_type_` path: update the comment near the field_type.get_attr("_type_", vm) /
downcast_ref::<PyStr>() branch to explicitly state that `_type_` was validated
as ASCII at type creation for simple types so that the to_str().expect("_type_
is validated as ASCII at type creation") invariant is clearly scoped to this
code path (the simple-type `_type_` attribute handling in this block).

crates/vm/src/builtins/str.rs (1)

1363-1390: ⚠️ Potential issue | 🟠 Major

Do not swallow non-KeyError exceptions in translate.
Errors other than KeyError from table.__getitem__ should propagate; treating them as “missing mapping” can mask failures.

🐛 Suggested fix

-            match table.get_item(&*cp.to_u32().to_pyobject(vm), vm) {
+            match table.get_item(&*cp.to_u32().to_pyobject(vm), vm) {
                 Ok(value) => {
                     if let Some(text) = value.downcast_ref::<Self>() {
                         translated.push_wtf8(text.as_wtf8());
                     } else if let Some(bigint) = value.downcast_ref::<PyInt>() {
                         let mapped = bigint
                             .as_bigint()
                             .to_u32()
                             .and_then(CodePoint::from_u32)
                             .ok_or_else(|| {
                                 vm.new_value_error("character mapping must be in range(0x110000)")
                             })?;
                         translated.push(mapped);
                     } else if !vm.is_none(&value) {
                         return Err(
                             vm.new_type_error("character mapping must return integer, None or str")
                         );
                     }
                 }
-                _ => translated.push(cp),
+                Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {
+                    translated.push(cp)
+                }
+                Err(e) => return Err(e),
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/str.rs` around lines 1363 - 1390, The translate method
currently treats any Err from table.get_item as a missing mapping; change the
Err arm to capture the exception (e.g., Err(err)) and only swallow it if it is a
KeyError, otherwise propagate it by returning Err(err). Use the VM helper to
test the exception type (e.g., vm.is_instance(&err,
&vm.ctx.exceptions.key_error) or equivalent) so non-KeyError exceptions from
table.__getitem__ are returned to the caller instead of being treated as "no
mapping".

crates/stdlib/src/zlib.rs (1)

468-471: Optional: collapse the two-step into() chain by widening the parameter bound.

The intermediate String binding exists solely to bridge impl Into<String> → Wtf8Buf. Since every call site already passes &str or String — both of which implement Into<Wtf8Buf> after this PR's additions — the signature can be updated directly, eliminating the extra allocation on already-String inputs.

♻️ Proposed simplification

-    fn new_zlib_error(message: impl Into<String>, vm: &VirtualMachine) -> PyBaseExceptionRef {
-        let msg: String = message.into();
-        vm.new_exception_msg(vm.class("zlib", "error"), msg.into())
+    fn new_zlib_error(message: impl Into<Wtf8Buf>, vm: &VirtualMachine) -> PyBaseExceptionRef {
+        vm.new_exception_msg(vm.class("zlib", "error"), message.into())
     }

Wtf8Buf would also need to be brought into scope at the top of the inner zlib module:

use rustpython_vm::common::wtf8::Wtf8Buf; // adjust path as needed

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/zlib.rs` around lines 468 - 471, Change new_zlib_error to
accept message: impl Into<Wtf8Buf> (instead of impl Into<String>), bring Wtf8Buf
into scope in the inner zlib module, and pass message.into() directly to
vm.new_exception_msg to avoid the intermediate String allocation; update the
signature and the call site in fn new_zlib_error accordingly so the function
uses Wtf8Buf conversion once.

crates/vm/src/stdlib/time.rs (1)

103-104: Consider s.as_wtf8().as_bytes() for API consistency

s.as_bytes() is correct here (ASCII is identical under WTF-8 and UTF-8), but the rest of this PR consistently uses as_wtf8() to make the WTF-8 nature explicit. Using the same pattern here keeps the migration surface uniform.

🔧 Suggested change

-               && s.as_bytes() == b"negative duration"
+               && s.as_wtf8().as_bytes() == b"negative duration"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/time.rs` around lines 103 - 104, Replace the UTF-8 byte
conversion call to maintain consistency with the rest of the PR: in the
conditional that extracts the first argument string from e.args() (the closure
using arg.str(vm).ok()), change s.as_bytes() to s.as_wtf8().as_bytes() so the
WTF-8 representation is used consistently; keep the same comparison to
b"negative duration".

crates/vm/src/stdlib/sre.rs (2)

856-856: Prefer .expect() over .unwrap() to document the invariant.

get_slice(0, …) returns None only when regs[0] contains a negative span. Since regs[0] is always set from a usize in Match::new, the unwrap() is safe, but the invariant is silent. Using expect() documents it clearly.

♻️ Suggested change

-                let match_repr = zelf.get_slice(0, str_drive, vm).unwrap().repr(vm)?;
+                let match_repr = zelf
+                    .get_slice(0, str_drive, vm)
+                    .expect("group 0 always has a valid span")
+                    .repr(vm)?;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/sre.rs` at line 856, Replace the silent unwrap on
zelf.get_slice(0, str_drive, vm).unwrap() with expect() to document the
invariant: change to zelf.get_slice(0, str_drive, vm).expect("regs[0] is
non-negative by construction in Match::new").repr(vm)?; this makes it explicit
that get_slice can only be None for a negative span and that regs[0] is always
set from a usize in Match::new.

---

857-860: Wtf8Buf::from(format!(…)) allocates an intermediate String.

Since only ASCII literals and integers are used, the intermediate String can be avoided by pushing pieces directly—matching the style used elsewhere in this PR (e.g. mappingproxy.rs, template.rs).

♻️ Suggested change

-                let mut result = Wtf8Buf::from(format!(
-                    "<re.Match object; span=({}, {}), match=",
-                    zelf.regs[0].0, zelf.regs[0].1,
-                ));
+                let mut result = Wtf8Buf::from("<re.Match object; span=(");
+                result.push_str(&zelf.regs[0].0.to_string());
+                result.push_str(", ");
+                result.push_str(&zelf.regs[0].1.to_string());
+                result.push_str("), match=");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/sre.rs` around lines 857 - 860, Replace the allocation
from Wtf8Buf::from(format!(...)) by building the buffer in pieces: create result
with Wtf8Buf::from("<re.Match object; span=(") and then append the integer
bounds and punctuation directly (use result.push_str(...) / result.push(...) or
the same push helpers used elsewhere in mappingproxy.rs/template.rs) for
zelf.regs[0].0, ',', ' ', zelf.regs[0].1, '), match=' to avoid the intermediate
String allocation; update the code around the Wtf8Buf::from(format!(…)) call and
the variable result accordingly.

crates/vm/src/exception_group.rs (2)

217-228: Minor: ArgIterable is heavier than a direct tuple downcast for a known-tuple slot.

The constructor guarantees arg(1) is always a PyTuple, yet slot_repr goes through ArgIterable::try_from_object, which dispatches Python's iterator protocol. A direct downcast_ref::<PyTuple>() (mirroring the pattern in __str__ at line 191 and in get_exceptions_tuple) avoids the extra allocation and dispatch:

♻️ Direct tuple downcast in slot_repr

-            if let Some(exceptions_obj) = zelf.get_arg(1) {
-                let iter: ArgIterable<PyObjectRef> =
-                    ArgIterable::try_from_object(vm, exceptions_obj.clone())?;
-                let mut first = true;
-                for exc in iter.iter(vm)? {
-                    if !first {
-                        result.push_str(", ");
-                    }
-                    first = false;
-                    result.push_wtf8(exc?.repr(vm)?.as_wtf8());
-                }
-            }
+            if let Some(exceptions_obj) = zelf.get_arg(1) {
+                if let Some(tuple) = exceptions_obj.downcast_ref::<PyTuple>() {
+                    let mut first = true;
+                    for exc in tuple.iter() {
+                        if !first {
+                            result.push_str(", ");
+                        }
+                        first = false;
+                        result.push_wtf8(exc.repr(vm)?.as_wtf8());
+                    }
+                }
+            }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/exception_group.rs` around lines 217 - 228, In slot_repr, avoid
using ArgIterable::try_from_object for the known-tuple arg(1); instead downcast
the argument to a PyTuple directly (use zelf.get_arg(1) and
downcast_ref::<PyTuple>()) and iterate its elements, mirroring the pattern used
in __str__ and get_exceptions_tuple to eliminate iterator protocol dispatch and
extra allocation; update the code paths around zelf, get_arg, and the for-loop
that builds result so it uses the PyTuple iteration and repr calls on each
element.

---

196-201: write! + .unwrap() is idiomatic but note the implicit assumption.

Wtf8Buf::write_str must be infallible (as is String's) for the two .unwrap() calls here and on line 213 to never panic. If the fmt::Write impl for Wtf8Buf can ever return Err (e.g., due to an encoding edge case when non-UTF-8 format arguments are passed), these become silent panics. Consider using .expect("Wtf8Buf write is infallible") to make the assumption explicit, or verify the impl is truly infallible.

💡 Explicit infallibility annotation

-            write!(result, " ({num_excs} sub-exception{suffix})").unwrap();
+            write!(result, " ({num_excs} sub-exception{suffix})").expect("Wtf8Buf write is infallible");

And similarly on line 213:

-            write!(result, "{class_name}(").unwrap();
+            write!(result, "{class_name}(").expect("Wtf8Buf write is infallible");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/exception_group.rs` around lines 196 - 201, The current use of
write!(result, ...) followed by .unwrap() assumes Wtf8Buf::write_str is
infallible; replace those .unwrap() calls with .expect("Wtf8Buf write is
infallible") (or otherwise assert/handle the error) for the write! invocation
that appends " ({num_excs} sub-exception{suffix})" and the similar write! call
later (the other occurrence around line 213) so the assumption is explicit;
locate the write! usages operating on the Wtf8Buf `result` and change their
error handling to .expect(...) or add an explicit check that the fmt::Write impl
cannot return Err.

crates/vm/src/builtins/bool.rs (1)

104-109: Return type String is inconsistent with peer __format__ implementations.

int.__format__, float.__format__, and complex.__format__ all return PyResult<Wtf8Buf>, but bool.__format__ returns PyResult<String>. While functionally correct (bool formatting only produces ASCII), aligning the return type would be more consistent.

♻️ Optional: align return type with siblings

-    fn __format__(obj: PyObjectRef, spec: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult<String> {
+    fn __format__(obj: PyObjectRef, spec: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult<Wtf8Buf> {
         let new_bool = obj.try_to_bool(vm)?;
         FormatSpec::parse(spec.as_str())
             .and_then(|format_spec| format_spec.format_bool(new_bool))
+            .map(Wtf8Buf::from_string)
             .map_err(|err| err.into_pyexception(vm))
     }

This would also require adding use rustpython_common::wtf8::Wtf8Buf; to the imports.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/bool.rs` around lines 104 - 109, The __format__
implementation for bool returns PyResult<String> which is inconsistent with
int/float/complex that return PyResult<Wtf8Buf>; change bool::__format__
signature to return PyResult<Wtf8Buf>, convert the result of
FormatSpec::parse(...).and_then(|fs| fs.format_bool(new_bool)) into a Wtf8Buf
(e.g. via Wtf8Buf::from_str or equivalent) and propagate errors the same way,
and add use rustpython_common::wtf8::Wtf8Buf; to the imports so the function
matches its peers.

crates/vm/src/import.rs (1)

528-534: Downcasting __package__ to PyUtf8StrRef rejects surrogate-containing package names.

Previously this would have accepted any PyStr (and potentially panicked on as_str()). Now it returns TypeError("package must be a string") for surrogate-containing strings. The error message is slightly misleading — the value is a string, it's just not a valid UTF-8 string. That said, surrogate package names are pathological and this aligns with CPython behavior where import resolution operates on str values.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/import.rs` around lines 528 - 534, The current downcast uses
PyUtf8StrRef which rejects surrogate-containing Python strings; change the
downcast to PyStrRef (i.e., downcast::<PyStrRef>()) so genuine PyStr values
(including surrogates) are accepted, and update any subsequent usage of pkg_str
to work with PyStrRef (use its APIs rather than UTF-8-only methods); keep the
TypeError for non-string types but do not treat surrogate-containing strings as
non-strings.

crates/vm/src/builtins/type.rs (1)

1564-1567: Avoid expect on slot names to keep surrogate paths panic‑free.

Using expect can still panic if upstream validation changes or if a surrogate slips through. Prefer converting to a typed error here.

♻️ Suggested fix

-                let mangled_name = mangle_name(
-                    &class_name,
-                    member.to_str().expect("slot name is always valid UTF-8"),
-                );
+                let member_name = member.to_str().ok_or_else(|| {
+                    vm.new_type_error("__slots__ must be valid UTF-8 identifiers".to_owned())
+                })?;
+                let mangled_name = mangle_name(&class_name, member_name);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/type.rs` around lines 1564 - 1567, The code calls
member.to_str().expect(...) before passing the string to mangle_name, which can
panic; replace the expect with a safe conversion that returns/propagates a typed
error instead (e.g., use member.to_str().ok_or_else(||
Error::SlotNameEncoding(member.to_os_string()))? or map to an existing error
variant), then pass the &str to mangle_name as before; update the surrounding
function (where mangle_name is invoked) to return a Result so the new error can
be propagated rather than panicking.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

src/shell.rs (1)

145-146: ⚠️ Potential issue | 🔴 Critical

Avoid panic in REPL prompt extraction (Line 145).

expect_str() can panic on surrogate-containing sys.ps1/sys.ps2, which reintroduces the failure mode this PR is meant to remove.

Suggested fix

-        let prompt = match prompt {
-            Ok(ref s) => s.expect_str(),
-            Err(_) => "",
-        };
+        let prompt = match prompt {
+            Ok(ref s) => s.to_str().unwrap_or(""),
+            Err(_) => "",
+        };

#!/bin/bash
# Read-only verification: confirm API behavior and current callsite.
rg -n "fn\\s+expect_str\\s*\\(" --type rust -A4 -B2
rg -n "fn\\s+to_str\\s*\\(" --type rust -A4 -B2
rg -n "expect_str\\(\\)" --type rust src/shell.rs -C2

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/shell.rs` around lines 145 - 146, The code calls expect_str() which can
panic on surrogate-containing prompts; replace that call with a non-panicking
conversion (e.g., call to_str() and handle the Result/Option) so failures return
an empty prompt instead of panicking; specifically update the use of
expect_str() in the REPL prompt extraction to something like
to_str().unwrap_or("") or match on to_str() / to_string_lossy() and return "" on
error, ensuring no panics from expect_str().

crates/vm/src/stdlib/msvcrt.rs (1)

83-91: ⚠️ Potential issue | 🟠 Major

expect_str() still panics on surrogate input — the PR's goal is unmet here.

Replacing as_str() with expect_str() doesn't fix the panic: both methods blow up on PyStr values that contain surrogates (valid Python strings on Windows). putwch / ungetwch are Windows wide-character APIs that are supposed to accept surrogate code units, so the safe path is to walk the WTF-8 code points directly.

🔧 Surrogate-safe fix for putwch (line 83-91) and ungetwch (lines 108-120)

 fn putwch(s: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
     let c = s
-        .expect_str()
-        .chars()
+        .as_wtf8()
+        .code_points()
         .exactly_one()
         .map_err(|_| vm.new_type_error("putch() argument must be a string of length 1"))?;
-    unsafe { suppress_iph!(_putwch(c as u16)) };
+    unsafe { suppress_iph!(_putwch(c.to_u32() as u16)) };
     Ok(())
 }

 fn ungetwch(s: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
     let c =
-        s.expect_str().chars().exactly_one().map_err(|_| {
+        s.as_wtf8().code_points().exactly_one().map_err(|_| {
             vm.new_type_error("ungetwch() argument must be a string of length 1")
         })?;
-    let ret = unsafe { suppress_iph!(_ungetwch(c as u32)) };
+    let ret = unsafe { suppress_iph!(_ungetwch(c.to_u32())) };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/msvcrt.rs` around lines 83 - 91, The current putwch and
ungetwch use expect_str() which panics on surrogate-containing PyStr; instead
iterate the string's WTF-8 code units so surrogate code units are preserved: for
both putwch and ungetwch, replace the expect_str().chars().exactly_one() path
with a WTF-8–aware extraction from the PyStrRef (walk the underlying code
units/encoded units, ensure there is exactly one 16-bit code unit, map that unit
to u16 and call _putwch/_ungetwch), and keep the same vm.new_type_error when the
argument is not exactly one code unit; ensure you reference the functions putwch
and ungetwch and remove any use of expect_str() that assumes valid UTF-8.

crates/vm/src/stdlib/nt.rs (3)

1189-1192: ⚠️ Potential issue | 🔴 Critical

expect_str() still panics on surrogate-containing strings — same defect as as_str()

The change replaced as_str() with expect_str(), but expect_str() also panics when the string contains surrogates. Because argv comes from user-supplied Python objects via extract_elements_with, untrusted data can still trigger an unconditional panic — directly contradicting this PR's objective.

Use to_str() with proper Python-level error propagation:

🐛 Proposed fix

         let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
             let arg = PyStrRef::try_from_object(vm, obj)?;
-            make_widestring(arg.expect_str())
+            make_widestring(arg.to_str().ok_or_else(|| vm.new_unicode_encode_error("surrogates not allowed"))?)
         })?;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/nt.rs` around lines 1189 - 1192, The closure passed to
vm.extract_elements_with currently calls PyStrRef::try_from_object and then
expect_str(), which still panics on surrogate-containing input; change the
closure to call arg.to_str() (or equivalent non-panicking conversion) and
propagate any Unicode decoding error out as a Python error (so the ? from the
closure returns a Python-level error) before passing the resulting &str into
make_widestring; update the closure around PyStrRef::try_from_object(vm, obj)?
to use arg.to_str() and return a proper VM/Python error instead of letting
expect_str() panic.

---

1230-1233: ⚠️ Potential issue | 🔴 Critical

Same expect_str() panic risk in execve argv extraction

Identical issue as in execv: arg.expect_str() on user-provided argv elements panics on surrogates.

🐛 Proposed fix

         let argv = vm.extract_elements_with(argv.as_ref(), |obj| {
             let arg = PyStrRef::try_from_object(vm, obj)?;
-            make_widestring(arg.expect_str())
+            make_widestring(arg.to_str().ok_or_else(|| vm.new_unicode_encode_error("surrogates not allowed"))?)
         })?;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/nt.rs` around lines 1230 - 1233, The argv extraction in
execve uses arg.expect_str() which can panic on surrogate-containing user
strings; change the closure passed to vm.extract_elements_with in execve to use
the safe string accessor (e.g. arg.as_str()) instead of expect_str(), so call
make_widestring(arg.as_str()) after obtaining arg via
PyStrRef::try_from_object(vm, obj) to avoid panics.

---

1253-1256: ⚠️ Potential issue | 🔴 Critical

expect_str() panic on surrogate env keys/values in execve

key.expect_str() and value.expect_str() at lines 1255–1256 are called on values extracted from a user-supplied ArgMapping. Surrogate-containing env strings will panic here instead of raising a Python UnicodeEncodeError.

🐛 Proposed fix

         let key = PyStrRef::try_from_object(vm, key)?;
         let value = PyStrRef::try_from_object(vm, value)?;
-        let key_str = key.expect_str();
-        let value_str = value.expect_str();
+        let key_str = key.to_str().ok_or_else(|| vm.new_unicode_encode_error("surrogates not allowed"))?;
+        let value_str = value.to_str().ok_or_else(|| vm.new_unicode_encode_error("surrogates not allowed"))?;

crates/vm/src/stdlib/_winapi.rs (1)

299-301: ⚠️ Potential issue | 🔴 Critical

expect_str() reintroduces panic paths on surrogate-containing str inputs

Line 299, Line 399, and Lines 421/423 still use expect_str(), which can panic and defeats the PR goal of removing panic-based string extraction. Please switch these callsites to fallible conversion (to_str() + error propagation) or a WTF-8-safe path where needed.

Proposed fix

@@
-        let wstr = |s: PyStrRef| {
-            let ws = widestring::WideCString::from_str(s.expect_str())
+        let wstr = |s: PyStrRef| {
+            let s = s.to_str().map_err(|e| e.to_pyexception(vm))?;
+            let ws = widestring::WideCString::from_str(s)
                 .map_err(|err| err.to_pyexception(vm))?;
             Ok(ws.into_vec_with_nul())
         };
@@
-        let src_path = std::path::Path::new(src_path.expect_str());
-        let dest_path = std::path::Path::new(dest_path.expect_str());
+        let src_path = std::path::Path::new(src_path.to_str().map_err(|e| e.to_pyexception(vm))?);
+        let dest_path =
+            std::path::Path::new(dest_path.to_str().map_err(|e| e.to_pyexception(vm))?);
@@
-            let k = k.expect_str();
+            let k = k.to_str().map_err(|e| e.to_pyexception(vm))?;
             let v = PyStrRef::try_from_object(vm, v)?;
-            let v = v.expect_str();
+            let v = v.to_str().map_err(|e| e.to_pyexception(vm))?;

#!/bin/bash
# Verify panic-prone string extraction remains in the touched file.
rg -nP 'expect_str\s*\(' crates/vm/src/stdlib/_winapi.rs -C2

As per coding guidelines: "Follow Rust best practices for error handling and memory management".

Also applies to: 399-400, 421-423

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/_winapi.rs` around lines 299 - 301, The uses of
s.expect_str() (notably where WideCString::from_str(s.expect_str()) is called
before .map_err(|err| err.to_pyexception(vm))? and similar callsites around the
other mentions) reintroduce panic paths; replace each expect_str() with a
fallible extraction: call s.to_str().map_err(|e| /* convert to a VM error via
e.to_pyexception(vm) or a descriptive error */ )? and then pass the &str into
widestring::WideCString::from_str, or if the input may contain WTF-8/surrogates
use a WTF-8-safe conversion path (e.g., obtain the raw bytes and use a WTF-8
aware API) and propagate the error via to_pyexception(vm); ensure the final
Ok(ws.into_vec_with_nul()) remains unchanged and all conversions return Result
to avoid panics in functions like the WideCString conversion sites referenced.

crates/vm/src/types/slot.rs (1)

2077-2077: format!("… {}", repr.as_wtf8()) has the same Wtf8: Display concern as line 237 in structseq.rs.

Since this is inside #[cold] unreachable!, it has no practical impact on correctness, but it is worth tracking alongside the broader question of how Wtf8 formats.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/types/slot.rs` at line 2077, The unreachable! invocation uses
format with repr.as_wtf8() which relies on Wtf8::Display; change the message to
avoid requiring Display (e.g., use debug formatting) by replacing
unreachable!("slot must be overridden for {}", repr.as_wtf8()) with something
like unreachable!("slot must be overridden for {:?}", repr.as_wtf8()), so the
code in slot.rs no longer depends on Wtf8::Display.

crates/vm/src/stdlib/functools.rs (1)

428-430: ⚠️ Potential issue | 🟠 Major

Avoid expect_str() when normalizing stored keyword keys.

Line [430] still uses expect_str(), which can panic for surrogate-containing str keys. Use to_str() and raise TypeError instead so partial calls fail safely.

🛠️ Proposed fix

             for (key, value) in &*keywords {
                 let key_str = key
                     .downcast_ref::<crate::builtins::PyStr>()
                     .ok_or_else(|| vm.new_type_error("keywords must be strings"))?;
-                final_kwargs.insert(key_str.expect_str().to_owned(), value);
+                let key = key_str
+                    .to_str()
+                    .ok_or_else(|| vm.new_type_error("keywords must be strings"))?
+                    .to_owned();
+                final_kwargs.insert(key, value);
             }

#!/bin/bash
# Verify panic-prone API usage and current call site.
rg -n "pub fn expect_str|pub fn to_str" crates/vm/src/builtins/str.rs -A8 -B4
rg -n "final_kwargs.insert|downcast_ref::<crate::builtins::PyStr>|expect_str" crates/vm/src/stdlib/functools.rs -A8 -B4

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/functools.rs` around lines 428 - 430, The code
normalizes keyword keys using expect_str(), which can panic on
surrogate-containing strings; update the normalization in the block that calls
downcast_ref::<crate::builtins::PyStr>() and final_kwargs.insert(...) to call
to_str() instead, check the Result/Option and on failure return
vm.new_type_error("keywords must be strings") so the partial/keyword handling
fails with a TypeError rather than panicking; ensure you replace
expect_str().to_owned() with a safe to_str() usage and propagate the error into
the existing vm.new_type_error path.

crates/vm/src/stdlib/os.rs (1)

455-456: ⚠️ Potential issue | 🟠 Major

Replace panic-prone expect_str() in Windows env APIs.

Line [455], Line [456], and Line [503] use expect_str(), which can panic on surrogate-containing str values and bypass PyResult error handling. Please switch these to to_str() and raise Python errors instead of panicking.

🛠️ Proposed fix

 fn putenv(key: PyStrRef, value: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
-        let key_str = key.expect_str();
-        let value_str = value.expect_str();
+        let key_str = key
+            .to_str()
+            .ok_or_else(|| vm.new_value_error("illegal environment variable name"))?;
+        let value_str = value
+            .to_str()
+            .ok_or_else(|| vm.new_value_error("illegal environment variable value"))?;
@@
 fn unsetenv(key: PyStrRef, vm: &VirtualMachine) -> PyResult<()> {
-        let key_str = key.expect_str();
+        let key_str = key
+            .to_str()
+            .ok_or_else(|| vm.new_value_error("illegal environment variable name"))?;

#!/bin/bash
# Verify panic-vs-result string APIs and current call sites.
rg -n "pub fn expect_str|pub fn to_str" crates/vm/src/builtins/str.rs -A8 -B4
rg -n "fn putenv|fn unsetenv|expect_str" crates/vm/src/stdlib/os.rs -A10 -B4

Also applies to: 503-503

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/os.rs` around lines 455 - 456, Replace the panic-prone
expect_str() calls in the Windows env API handlers (the places assigning key_str
and value_str and the other site at line ~503) with to_str() and propagate a
proper Python error instead of panicking; specifically, in the functions
handling putenv/unsetenv (and any code that currently calls expect_str()), call
key.to_str() and value.to_str(), map the Err case to an appropriate PyResult
error via the VM (e.g., return Err(vm.new_type_error(...) or
vm.new_value_error(...)) with a clear message about invalid string contents) and
proceed with the unwrapped &str on Ok. Ensure all expect_str() occurrences in
this module are replaced and error paths return a PyResult rather than panic.

crates/vm/src/vm/mod.rs (1)

639-645: ⚠️ Potential issue | 🟡 Minor

Add a separator after the unraisable message prefix.

Line 640 builds "Exception ignored {msg}" without a separator before object repr, so output gets concatenated.

✏️ Suggested fix

-        let msg_str = if let Some(msg) = msg {
-            format!("Exception ignored {}", msg)
+        let msg_str = if let Some(msg) = msg {
+            format!("Exception ignored {msg}: ")
         } else {
             "Exception ignored in: ".to_owned()
         };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/vm/mod.rs` around lines 639 - 645, The constructed unraisable
message in vm::mod.rs builds msg_str without a separator when Some(msg) is
present, resulting in concatenated output; update the msg_str construction (the
branch that sets msg_str when msg is Some) to include a separator (for example a
colon and space) between the prefix and the object repr so the string passed to
write_to_stderr(&msg_str, &stderr, self) reads like "Exception ignored: {msg}"
instead of "Exception ignored {msg}".

crates/vm/src/stdlib/builtins.rs (1)

572-575: ⚠️ Potential issue | 🟠 Major

Avoid expect_str() in the rustyline prompt path.

Line 573 can still panic when prompt contains surrogate data. Gate rustyline on UTF-8 conversion and fall back to standard I/O when conversion fails.

🛠️ Suggested fix

-        if use_rustyline {
-            let prompt = prompt.as_ref().map_or("", |s| s.expect_str());
+        let prompt_utf8 = prompt.as_ref().and_then(|s| s.try_into_utf8(vm).ok());
+        if use_rustyline && (prompt.as_ref().is_none() || prompt_utf8.is_some()) {
+            let prompt = prompt_utf8.as_ref().map_or("", |s| s.as_str());
             let mut readline = Readline::new(());
             match readline.readline(prompt) {

As per coding guidelines: "Follow Rust best practices for error handling and memory management".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/builtins.rs` around lines 572 - 575, The rustyline
branch currently calls prompt.as_ref().map_or("", |s| s.expect_str()) which can
panic on non-UTF-8/surrogate data; instead try to convert the prompt to a UTF-8
&str and only use rustyline when conversion succeeds—e.g., match
prompt.as_ref().and_then(|s| s.to_str().ok()) or equivalent—and if conversion
fails, disable/use_rustyline = false (or fall back to the standard std::io
prompt path) before constructing Readline::new() and calling readline.readline;
update the logic around use_rustyline, prompt, Readline::new, and
readline.readline to avoid any expect_str() usage and safely handle non-UTF-8
prompts.

crates/vm/src/builtins/interpolation.rs (1)

210-225: ⚠️ Potential issue | 🔴 Critical

push_str(&expr_repr) and push_str(&spec_repr) still panic on WTF-8 surrogates.

Lines 213 and 221 use push_str with &expr_repr/&spec_repr (both PyRef<PyStr>). This resolves via AsRef<str> on PyRef<PyStr>, which calls to_str().expect("str has surrogates") and panics if the expression or format_spec contains surrogates. All other repr parts (value_repr, conversion) already use the surrogate-safe push_wtf8. These two need the same treatment to be consistent with the PR's intent and to avoid panics.

🐛 Proposed fix

-        result.push_str(&expr_repr);
+        result.push_wtf8(expr_repr.as_wtf8());
         result.push_str(", ");
         if vm.is_none(&zelf.conversion) {
             result.push_str("None");
         } else {
             result.push_wtf8(zelf.conversion.repr(vm)?.as_wtf8());
         }
         result.push_str(", ");
-        result.push_str(&spec_repr);
+        result.push_wtf8(spec_repr.as_wtf8());

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/interpolation.rs` around lines 210 - 225, The repr
construction in interpolation.rs uses push_str(&expr_repr) and
push_str(&spec_repr) which call AsRef<str> on PyRef<PyStr> and will panic on
WTF-8 surrogates; change those calls to use Wtf8Buf::push_wtf8 so they accept
surrogate-containing strings just like value_repr and conversion do—replace
push_str(&expr_repr) and push_str(&spec_repr) with
result.push_wtf8(expr_repr.as_wtf8()) and result.push_wtf8(spec_repr.as_wtf8())
(referencing the Wtf8Buf type and push_wtf8 method and the PyRef<PyStr> values
expr_repr/spec_repr).

crates/vm/src/stdlib/codecs.rs (1)

52-75: Factor out shared CodeArgs normalization in encode/decode.

Line 52 through Line 75 duplicate the same destructuring and encoding-default logic; only the final registry call differs.

As per coding guidelines, "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/codecs.rs` around lines 52 - 75, Extract the repeated
CodeArgs normalization into a small helper (e.g., a private fn
normalize_code_args(args: CodeArgs) -> (PyObjectRef, &str, Option<String>) or
similar) that performs the destructuring and resolves encoding via
codecs::DEFAULT_ENCODING, then call vm.state.codec_registry.encode(...) and
vm.state.codec_registry.decode(...) with the normalized tuple from encode and
decode; reference the existing symbols CodeArgs, DEFAULT_ENCODING, encode,
decode, and vm.state.codec_registry to locate where to replace the duplicate
logic.

crates/vm/src/warn.rs (2)

410-433: to_str() returning None (surrogate-containing action) silently falls through to the "Unrecognized action" error path

The _ arm in the match action_s block handles both Some(unknown_str) and None (action contains surrogates) identically. While treating a surrogate-containing action as unrecognized is functionally correct, the conflation is non-obvious. A brief comment or explicit None arm would clarify intent, especially since this is the key behavioral change introduced by the migration from as_str() to to_str().

♻️ Proposed clarification

         let action_s = action_str.to_str();
         match action_s {
             Some("once") => { ... }
             Some("module") => { ... }
             Some("default") => false,
+            // None means the action string contains surrogates and is not
+            // a recognized UTF-8 action name; fall through to the error.
             _ => {
                 return Err(vm.new_runtime_error(format!(
                     "Unrecognized action ({action_str}) in warnings.filters:\n {action_str}"
                 )));
             }
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/warn.rs` around lines 410 - 433, The match on action_s (result
of action_str.to_str()) currently conflates None (surrogate-containing string)
with unrecognized Some values in the wildcard arm; make this explicit by adding
a separate None => { /* surrogate-containing action: treat as unrecognized or
document behavior */ } arm or a brief comment before the match clarifying that
None indicates surrogate-containing input and should be handled the same as
unknown actions; reference the symbols action_str, action_s, to_str(), and the
match block around update_registry/get_once_registry so reviewers can locate and
apply the change.

---

395-405: Inconsistent comparison style: mix of as_bytes() and as_wtf8() for equivalent ASCII checks

Lines 395 and 399 use as_bytes() to compare ASCII action strings, but line 405 switches to as_wtf8() for the same pattern. All four strings ("error", "ignore", "always", "all") are pure ASCII, so as_bytes() is sufficient and cheaper throughout. Mixing the two idioms in the same function reduces readability.

♻️ Proposed refactor for consistency

-    if action_str.as_bytes() == b"error" {
+    let action_bytes = action_str.as_bytes();
+    if action_bytes == b"error" {
         let exc = PyBaseExceptionRef::try_from_object(vm, message)?;
         return Err(exc);
     }
-    if action_str.as_bytes() == b"ignore" {
+    if action_bytes == b"ignore" {
         return Ok(());
     }
 
-    let already = if action_str.as_wtf8() != "always" && action_str.as_wtf8() != "all" {
+    let already = if action_bytes != b"always" && action_bytes != b"all" {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/warn.rs` around lines 395 - 405, The comparisons against the
action string are inconsistent: some use action_str.as_bytes() (for "error" and
"ignore") while others use action_str.as_wtf8() (for "always" and "all"); change
the latter to use action_str.as_bytes() so all four ASCII literals
("error","ignore","always","all") are compared the same and more
efficiently—update the conditional that currently uses as_wtf8() to use
as_bytes() instead (look for the variable action_str and the branching that
checks "always" and "all").

crates/vm/src/builtins/function.rs (1)

1090-1096: Simplify the __qualname__/__name__ fallback chain.

The if let … { Some(qname) } else { … } pattern re-wraps an already-Some value unnecessarily.

♻️ Proposed simplification

-        let func_name = if let Some(qname) =
-            vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __qualname__))?
-        {
-            Some(qname)
-        } else {
-            vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __name__))?
-        };
+        let func_name = match vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __qualname__))? {
+            Some(qname) => Some(qname),
+            None => vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __name__))?,
+        };

Or more idiomatically using a fallible or_else:

-        let func_name = if let Some(qname) =
-            vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __qualname__))?
-        {
-            Some(qname)
-        } else {
-            vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __name__))?
-        };
+        let func_name = vm
+            .get_attribute_opt(zelf.function.clone(), identifier!(vm, __qualname__))?
+            .map(Ok)
+            .unwrap_or_else(|| vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __name__)))?;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/function.rs` around lines 1090 - 1096, The current
if-let re-wraps an Option from vm.get_attribute_opt unnecessarily; replace the
conditional with an Option chain: first call
vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __qualname__))? to
get an Option, then call .or(...) with
vm.get_attribute_opt(zelf.function.clone(), identifier!(vm, __name__))? so
func_name is the first Some or the fallback without re-wrapping; update the
expression that assigns func_name accordingly (referencing func_name,
vm.get_attribute_opt, identifier!(vm, __qualname__), identifier!(vm, __name__),
and zelf.function).

crates/stdlib/src/zlib.rs (1)

468-471: Update the generic bound to align with codebase conventions and eliminate the intermediate variable.

The two-step conversion is correct, but it deviates from the established pattern in the stdlib. Other modules (e.g., csv.rs) use impl Into<Wtf8Buf> directly for similar functions, which works because Wtf8Buf implements From<String> and From<&str>.

Update the function to:

Recommended refactoring

-fn new_zlib_error(message: impl Into<String>, vm: &VirtualMachine) -> PyBaseExceptionRef {
-    let msg: String = message.into();
-    vm.new_exception_msg(vm.class("zlib", "error"), msg.into())
+fn new_zlib_error(message: impl Into<Wtf8Buf>, vm: &VirtualMachine) -> PyBaseExceptionRef {
+    vm.new_exception_msg(vm.class("zlib", "error"), message.into())
 }

Add to imports: use rustpython_common::wtf8::Wtf8Buf;

All call sites (string literals, e.to_string(), etc.) are compatible with Into<Wtf8Buf>.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/zlib.rs` around lines 468 - 471, Refactor new_zlib_error to
accept impl Into<Wtf8Buf> instead of impl Into<String>, remove the intermediate
msg variable and pass the converted value directly to vm.new_exception_msg (keep
vm.class("zlib", "error") as the exception class); also add the import use
rustpython_common::wtf8::Wtf8Buf; so call sites (literals and e.to_string())
which implement Into<Wtf8Buf> continue to work without extra allocations.

crates/wasm/src/convert.rs (1)

59-61: Redundant .into() on String binding

js_sys::Error::to_string() returns a Rust String (via Display; JavaScript's toString() is exposed as to_js_string() in js-sys bindings to avoid confusion with Rust's ToString trait and its to_string() method). The .into() here converts String → String (identity) since msg is typed as String; the actual String → Wtf8Buf (or equivalent) conversion happens implicitly at the new_type_error(msg) call site. The intermediate binding with .into() is unnecessary.

♻️ Suggested simplification

-    let msg: String = js_err.unchecked_into::<js_sys::Error>().to_string().into();
-    vm.new_type_error(msg)
+    vm.new_type_error(js_err.unchecked_into::<js_sys::Error>().to_string())

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/wasm/src/convert.rs` around lines 59 - 61, Remove the redundant
.into() when converting the JS error to a Rust String: in the binding where you
create msg from js_err (using js_sys::Error::to_string()), just assign the
returned String directly to msg and pass it into vm.new_type_error(msg) — e.g.,
change the binding that currently uses
js_err.unchecked_into::<js_sys::Error>().to_string().into() to use to_string()
without .into() so you avoid the no-op String→String conversion.

crates/vm/src/types/structseq.rs (1)

237-237: Wtf8::fmt() replaces unpaired surrogates with U+FFFD, corrupting module names containing such code points.

The Display impl for Wtf8 iterates through the bytes and explicitly substitutes any unpaired surrogate code points with the replacement character U+FFFD. When formatting module_str.as_wtf8() in the qualified type name, module names containing surrogates (which can occur via the surrogateescape codec handler) will silently lose that information. While this is an improvement over the panic behavior of as_str(), full WTF-8 fidelity would require operating on Wtf8Buf instead of converting to String for repr_str.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/types/structseq.rs` at line 237, The current code calls
module_str.as_wtf8() and formats it (via Wtf8::fmt) into a String, which
replaces unpaired surrogates with U+FFFD; instead preserve full WTF-8 fidelity
by using a Wtf8Buf-based representation when building the qualified type name in
repr_str (referencing module_str.as_wtf8(), Wtf8::fmt(), and Self::NAME). Change
the construction to operate on or append a Wtf8Buf (or otherwise emit WTF-8
bytes directly) instead of converting to a Rust String for formatting, so module
names with unpaired surrogates are preserved end-to-end in the qualified name.

crates/vm/src/stdlib/io.rs (1)

5863-5869: Consider deduplicating repr-fallback logic in _dealloc_warn.

The same fallback pattern appears in both fileio and winconsoleio; extracting it would reduce drift risk.

♻️ Optional extraction sketch

+fn repr_or_file_fallback(source: PyObjectRef, vm: &VirtualMachine) -> Wtf8Buf {
+    source
+        .repr(vm)
+        .map(|s| s.as_wtf8().to_owned())
+        .unwrap_or_else(|_| Wtf8Buf::from("<file>"))
+}
...
- let repr = source
-     .repr(vm)
-     .map(|s| s.as_wtf8().to_owned())
-     .unwrap_or_else(|_| Wtf8Buf::from("<file>"));
+ let repr = repr_or_file_fallback(source, vm);

Also applies to: 6415-6422

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/io.rs` around lines 5863 - 5869, The repr fallback
(calling .repr(vm).map(|s| s.as_wtf8().to_owned()).unwrap_or_else(|_|
Wtf8Buf::from("<file>"))) is duplicated in _dealloc_warn across fileio and
winconsoleio; extract that logic into a small helper (e.g., a function like
format_unclosed_file_repr(vm, obj) or repr_fallback_for_unclosed_file) and
replace the inline chain in both _dealloc_warn implementations to call the
helper, then use its result in the warn call (crate::stdlib::warnings::warn) to
eliminate duplication and keep behavior identical.

crates/vm/src/codecs.rs (1)

114-136: get_incremental_encoder/get_incremental_decoder signatures should be migrated to Option<PyUtf8StrRef> for consistency.

The encode, decode, and all CodecsRegistry wrapper methods now accept Option<PyUtf8StrRef>, but these two methods still accept Option<PyStrRef>. The only caller (in crates/vm/src/stdlib/io.rs) unnecessarily converts a PyUtf8Str to PyStrRef via into_wtf8() just to pass it to these methods. Updating the signatures and using into_wtf8().into() internally—as done in encode and decode—would eliminate this redundant widening and align the API.

♻️ Suggested alignment

     pub fn get_incremental_encoder(
         &self,
-        errors: Option<PyStrRef>,
+        errors: Option<PyUtf8StrRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
         let args = match errors {
-            Some(e) => vec![e.into()],
+            Some(e) => vec![e.into_wtf8().into()],
             None => vec![],
         };
         vm.call_method(self.0.as_object(), "incrementalencoder", args)
     }

     pub fn get_incremental_decoder(
         &self,
-        errors: Option<PyStrRef>,
+        errors: Option<PyUtf8StrRef>,
         vm: &VirtualMachine,
     ) -> PyResult {
         let args = match errors {
-            Some(e) => vec![e.into()],
+            Some(e) => vec![e.into_wtf8().into()],
             None => vec![],
         };
         vm.call_method(self.0.as_object(), "incrementaldecoder", args)
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/codecs.rs` around lines 114 - 136, Update the signatures of
get_incremental_encoder and get_incremental_decoder to accept
Option<PyUtf8StrRef> instead of Option<PyStrRef>, and inside each method convert
the optional PyUtf8StrRef to the argument expected by vm.call_method by calling
into_wtf8().into() (mirroring the encode/decode implementations) so callers no
longer need to perform an unnecessary widening conversion; keep method names
get_incremental_encoder and get_incremental_decoder and only change the
parameter types and the creation of args accordingly.

crates/vm/src/stdlib/operator.rs (1)

435-451: repr_wtf8 logic is correct; optional: replace first-flag with iterator combinators.

All three implementations produce the right output. The first: bool flag pattern works but is slightly verbose. A minor readability improvement is possible using intersperse or a separator-before-each approach:

✨ Optional refactor (attrgetter, same pattern applies to itemgetter)

-                let mut first = true;
                 for part in &zelf.attrs {
-                    if !first {
-                        result.push_str(", ");
-                    }
-                    first = false;
+                    if !std::ptr::eq(part, &zelf.attrs[0]) {
+                        result.push_str(", ");
+                    }
                     result.push_wtf8(part.as_object().repr(vm)?.as_wtf8());
                 }

Or, if the nightly iter_intersperse is not available, simply handle the first element before the loop:

-                let mut first = true;
-                for part in &zelf.attrs {
-                    if !first {
-                        result.push_str(", ");
-                    }
-                    first = false;
-                    result.push_wtf8(part.as_object().repr(vm)?.as_wtf8());
-                }
+                let mut iter = zelf.attrs.iter();
+                if let Some(first) = iter.next() {
+                    result.push_wtf8(first.as_object().repr(vm)?.as_wtf8());
+                }
+                for part in iter {
+                    result.push_str(", ");
+                    result.push_wtf8(part.as_object().repr(vm)?.as_wtf8());
+                }

Also applies to: 513-529, 601-622

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/stdlib/operator.rs` around lines 435 - 451, The repr_wtf8
function in operator.rs uses a mutable first: bool flag to insert ", " between
attributes; replace this flag-based loop with an iterator-based approach for
readability: map each part to its repr wtf8 string (using
part.as_object().repr(vm)?.as_wtf8()), then join or intersperse with ", " when
building the Wtf8Buf in repr_wtf8 (and apply the same change to the analogous
repr_wtf8 implementations for itemgetter and the other attr/item reprs
referenced). Ensure you still respect the ReprGuard early-return branch and
preserve error propagation from repr(vm)? while constructing the joined
representation.

crates/stdlib/src/ssl.rs (1)

3899-3923: set_server_hostname setter inconsistently keeps Option<PyStrRef> vs Option<PyUtf8StrRef> in constructor.

The _wrap_socket path rejects surrogate hostnames at argument-parse time (via WrapSocketArgs.server_hostname: Option<PyUtf8StrRef>), while this setter accepts Option<PyStrRef> and validates lazily via try_into_utf8. Both are safe (no panic), but the error type/timing differs: PyUtf8StrRef raises a UnicodeDecodeError at arg-parsing, whereas try_into_utf8 here raises a different error at runtime. Consider aligning to Option<PyUtf8StrRef> for consistency.

💡 Suggested alignment

-    fn set_server_hostname(
-        &self,
-        value: Option<PyStrRef>,
-        vm: &VirtualMachine,
-    ) -> PyResult<()> {
+    fn set_server_hostname(
+        &self,
+        value: Option<PyUtf8StrRef>,
+        vm: &VirtualMachine,
+    ) -> PyResult<()> {
         if *self.handshake_done.lock() {
             return Err(
                 vm.new_value_error("Cannot set server_hostname on socket after handshake")
             );
         }
-        let hostname_string = value
-            .map(|s| {
-                let utf8 = s.try_into_utf8(vm)?;
-                validate_hostname(utf8.as_str(), vm)?;
-                Ok::<String, _>(utf8.as_str().to_owned())
-            })
-            .transpose()?;
+        let hostname_string = value
+            .map(|s| {
+                validate_hostname(s.as_str(), vm)?;
+                Ok::<String, _>(s.as_str().to_owned())
+            })
+            .transpose()?;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/ssl.rs` around lines 3899 - 3923, The setter
set_server_hostname currently accepts Option<PyStrRef> and calls try_into_utf8,
producing a different error/timing than the WrapSocketArgs.server_hostname path
which uses Option<PyUtf8StrRef>; change the setter signature to accept
Option<PyUtf8StrRef> (matching WrapSocketArgs.server_hostname), remove
try_into_utf8 usage, and adjust the hostname_string construction to directly
clone/into a String from the PyUtf8StrRef before calling validate_hostname and
storing into self.server_hostname so both code paths raise the same
UnicodeDecodeError at argument-parse time.

crates/wtf8/src/concat.rs (1)

72-76: Drop unwrap() in concat formatting helpers to avoid panic paths.

Lines [75] and [84] use write!(...).unwrap(). This is avoidable panic surface; keep this path infallible by discarding the result explicitly.

♻️ Proposed fix

 impl<T: fmt::Display> Wtf8Concat for DisplayAsWtf8<T> {
     #[inline]
     fn fmt_wtf8(&self, buf: &mut Wtf8Buf) {
-        write!(buf, "{}", self.0).unwrap();
+        let _ = write!(buf, "{}", self.0);
     }
 }

 macro_rules! impl_wtf8_concat_for_int {
     ($($t:ty),*) => {
         $(impl Wtf8Concat for $t {
             #[inline]
             fn fmt_wtf8(&self, buf: &mut Wtf8Buf) {
-                write!(buf, "{}", self).unwrap();
+                let _ = write!(buf, "{}", self);
             }
         })*
     };
 }

As per coding guidelines, "Follow Rust best practices for error handling and memory management".

Also applies to: 81-85

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/wtf8/src/concat.rs` around lines 72 - 76, The concat formatting
helpers currently call write!(...).unwrap() inside the Wtf8Concat impls (notably
impl<T: fmt::Display> Wtf8Concat for DisplayAsWtf8 -> fmt_wtf8 and the analogous
impl around lines 81-85), which can panic; change these to discard the
fmt::Result instead of unwrapping (e.g., assign or ignore the return value) so
the formatting path is infallible. Locate the write!(buf, "{}", self.0).unwrap()
calls inside fmt_wtf8 (and the corresponding fmt_wtf8 in the other Display/Debug
wrapper) and replace unwrap() with an explicit ignore of the Result (e.g., let _
= write!(...)) to remove the panic path.

crates/vm/src/builtins/str.rs (1)

93-94: Use expect_str() in all AsRef<str> impls to remove duplication.

You already introduced PyStr::expect_str() on Line 482. Reusing it in these AsRef<str> impls keeps panic behavior consistent and centralizes future changes.

♻️ Proposed refactor

 impl AsRef<str> for PyStr {
     #[track_caller] // <- can remove this once it doesn't panic
     fn as_ref(&self) -> &str {
-        self.to_str().expect("str has surrogates")
+        self.expect_str()
     }
 }
@@
 impl AsRef<str> for Py<PyStr> {
     #[track_caller] // <- can remove this once it doesn't panic
     fn as_ref(&self) -> &str {
-        self.to_str().expect("str has surrogates")
+        self.expect_str()
     }
 }
@@
 impl AsRef<str> for PyStrRef {
     #[track_caller] // <- can remove this once it doesn't panic
     fn as_ref(&self) -> &str {
-        self.to_str().expect("str has surrogates")
+        self.expect_str()
     }
 }
@@
 impl AsRef<str> for PyRefExact<PyStr> {
     #[track_caller]
     fn as_ref(&self) -> &str {
-        self.to_str().expect("str has surrogates")
+        self.expect_str()
     }
 }
@@
 impl AsRef<str> for PyExact<PyStr> {
     #[track_caller]
     fn as_ref(&self) -> &str {
-        self.to_str().expect("str has surrogates")
+        self.expect_str()
     }
 }

Also applies to: 100-101, 107-108, 1893-1903, 482-487

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/str.rs` around lines 93 - 94, Replace the direct calls
to self.to_str().expect("str has surrogates") inside the AsRef<str> impls with
the centralized PyStr::expect_str() helper to remove duplication and keep panic
behavior consistent; locate the AsRef<str> impl blocks (where to_str() is
currently used) and call self.expect_str() instead, ensuring you use the
existing PyStr::expect_str() method rather than duplicating the expect message.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro