Repository navigation
posix, compile, display: take filesystem names as bytes across the path, compile, repr and environment boundaries - #1027
Conversation
WalkthroughThe interpreter preserves filesystem path bytes and original path objects across ChangesFilesystem and representation boundaries
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 239b169). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patch
2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44bd9b23fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return Ok(pyre_object::w_str_get_wtf8(r) | ||
| .to_string_lossy() | ||
| .into_owned()); |
There was a problem hiding this comment.
When a code object with a byte-only filename is nested in a container, such as repr((compile("", b"\xff.py", "exec"),)), py_repr_wtf8 delegates the tuple to py_repr, which reaches this generic code-object __repr__ dispatch and replaces the filename's \udcff with U+FFFD via to_string_lossy(). Direct repr(code) remains exact because of the new special case, but tuple/list/dict representations diverge from PyPy and no longer contain the actual co_filename; the recursive representation path needs to carry WTF-8 rather than convert nested results to String.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/pyre-interpreter/src/module/posix/interp_posix.rs`:
- Around line 498-503: The `as_bytes` flag is determined from the initial `arg`
before `__fspath__` resolution, but a `PathLike` object may return a different
type after resolution. Replace the separate calls to
`pyre_object::bytesobject::is_bytes_like(arg)` and
`crate::gateway::fsencode_bytes_w(arg)?` with a single call to
`fsencode_bytes_w_with_kind(arg)?` that returns both the bytes and a
discriminator indicating the actual resolved type. Use that discriminator to set
`as_bytes` instead of checking the input directly, ensuring `bytes_mode`
reflects the actual resolved path type.
- Around line 2420-2422: Update the path conversion in the stat/lstat
implementations around fsencode_bytes_w to propagate converter errors directly
with ?, removing the blanket map_err TypeError conversion. Preserve the
converter’s original ValueError, RuntimeError, UnicodeEncodeError, and other
exception behavior while maintaining structural parity between stat and lstat.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7fc490b0-97be-46a4-bcfb-842c39d45114
📒 Files selected for processing (10)
pyre/extra_tests/parity_tests/compile_filename_boundary.pypyre/extra_tests/parity_tests/os_boundary_surrogateescape.pypyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/display.rspyre/pyre-interpreter/src/gateway.rspyre/pyre-interpreter/src/host_seam.rspyre/pyre-interpreter/src/module/_random/macro_smoke.rspyre/pyre-interpreter/src/module/posix/interp_posix.rspyre/pyre-interpreter/src/pycode.rspyre/pyre-macros/src/lib.rs
|
Acted on the Codex parity review. §1 is real and it was mine.
The bytes and the resolved object now travel together in one record, the shape §3 — three of the four are pyre against both runtimes, so they are fixed here
The
Also folded in: Re-verified at — commented by Claude |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbcaa14e4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Some(env_ptrs.as_ptr()), | ||
| ) as i32; | ||
| Err(errno_err(errno, &command)) | ||
| Err(errno_err_with_filename(errno, args[0])) |
There was a problem hiding this comment.
When path is an os.PathLike and the executable is missing, this changed line stores the original wrapper object in OSError.filename, rather than the resolved str/bytes path used by the syscall. More importantly for PyPy parity, pypy/module/posix/interp_posix.py:1810-1817 catches the failed path-based execve with wrap_oserror, which leaves filename unset, just like the newly corrected execv path above; use the filename-less error conversion here as well.
AGENTS.md reference: AGENTS.md:L231-L233
Useful? React with 👍 / 👎.
…ailure names `gateway::fsencode_w` ended in `String::from_utf8_lossy`, and the posix module used it for every path argument, so a path byte with no UTF-8 spelling folded to U+FFFD before the syscall was made and distinct names aliased onto one another: `os.stat(b"\xffvictim")` answered for the file named `"�victim"`, and `os.path.exists` on it returned True. Take the bytes through to the call instead, converting to a `Path` with `OsStr::from_bytes` on unix and keeping the text form only where the platform has no byte spelling. Delete `fsencode_w` and `fsencode_w_with_kind` rather than leaving the lossy converters reachable; the `PyPath` / `PyPathOrNone` macro bindings, which stand for `gateway.py:365` `unwrap_spec` `'fsencode'`, now bind `Vec<u8>`. `OSError.filename` carries the argument object the call was given, matching `interp_posix.py:332 wrap_oserror2(space, e, w_path)`, so a bytes path reports bytes and a str keeps its surrogate escapes instead of both being respelled through a lossy decode. Assisted-by: Claude
… one `code_repr` formatted `source_path`, the compiler-side spelling, so after `c.replace(co_filename=...)` it reported the name the object was compiled under rather than the current `co_filename`. Build it from `code_filename_bytes` as WTF-8, the way `pycode.py:570 repr` reaches for the filename's `utf8_w`, and give the code type the same exact `py_repr_wtf8` branch the frame type has. That makes a code object the second thing whose repr can carry a lone surrogate, which the plain-`String` readers abort on: `repr([obj])` for any `__repr__` returning one already panicked in `w_str_get_value` where CPython returns the string. Those readers now drop the surrogate the way every other `String` consumer in this file does; `py_repr_wtf8` remains the accessor that keeps the result exact. Assisted-by: Claude
`compiling.py:13` declares `unwrap_spec(filename='fsencode', mode='text')`,
but `builtin_compile` bound both with `if is_str { ... } else { <default> }`,
so anything else was swallowed: `compile(src, b"ascii.py", "exec")` recorded
`"<string>"` — no surrogate needed to lose the name — and bytes, `__fspath__`,
`int` and `bytearray` did the same, a surrogate str raised UnicodeEncodeError,
an embedded NUL was accepted, and a non-str mode became "exec". Route them
through `fsencode_bytes_w` and `text_w`, which already carry the deprecation
warning, the two TypeError spellings and the NUL rejection.
The filename names the whole compilation unit, so the nested constants the
code object still holds unrealized inherit it when they are boxed; that is
what separates it from `pycode.py:431`, whose constructor and `replace`
filenames rename only the object being built. A SyntaxError out of the same
call reports the filename the successful compile would have recorded.
Assisted-by: Claude
…, and three parity fixes
`OSError.filename` retained the argument object, which is right for a `str` or
`bytes` path but not for an `os.PathLike`: `interp_posix.py:211-219 _unwrap_path`
calls `__fspath__` and keeps its *result* as `Path.w_path`, so
`os.stat(P("absent"))` names `"absent"`, not `P`. Carry the encoded bytes and the
resolved object together, as that record does, so the single `__fspath__` call
serves both.
The record owns its root scope, since a resolved `PathLike` name is freshly
allocated and — unlike the caller's argument array — nothing else keeps it
reachable while the call runs.
Alongside, three divergences from both runtimes:
- `execv` attached a filename where `interp_posix.py:1814-1817` wraps with none.
`execve` is left as it is: there CPython reports the path and PyPy reports
None, so pyre is not in the wrong against both.
- `posix_spawn` rejected every `=` in an environment key, while
`interp_posix.py:1762-1769` rejects it only after index zero, which is what
the `execve` path in the same file already implements.
- `pycode.py:570-572` reports a zero `co_firstlineno` as line -1.
`os.putenv` / `os.unsetenv` take their two halves as bytes for the same reason
paths do: `posix.environ` already hands entries back as bytes, so a value
spelling a byte with no UTF-8 form must survive the write.
Assisted-by: Claude
`py_repr_wtf8` / `py_str_wtf8` carried a prelude for the few types known to carry a lone surrogate and delegated everything else to `py_repr` / `py_str`, whose `String` cannot hold one. Move the whole dispatch into the WTF-8 functions and make `py_repr` / `py_str` lossy views of them, which drops the prelude's per-type carve-outs for frames and code objects. `dict_repr`, `list_repr`, `tuple_repr`, `array_repr_wtf8`, the deque, set, frozenset, slice, range, structseq and exception formatting, and the `!r` conversion in f-strings, `str.format` and the descriptor slots now build a `Wtf8Buf` and recurse through `py_repr_wtf8`, so `repr([C()])` for a `__repr__` returning `"s\udcff"` answers `[s\udcff]` rather than `[s�]`. `ReprGuard::enter` and the `stack_check()` are unchanged, and each slot keeps the allocation kind it had. Adds pyre/extra_tests/parity_tests/repr_surrogate_wtf8.py. Assisted-by: Claude
youknowone
changed the title
There was a problem hiding this comment.
💡 Codex Review
https://github.com/youknowone/pyre/blob/a67823489e4a33d599e390b5c2698252df131720/pyre-interpreter/src/display.rs#L243-L244
When an element's valid __repr__ returns "", appending it leaves out.len() at 1, so the next element is treated as the first: repr([E(), 1]) renders as [1] rather than [, 1]. tuple_repr has the identical condition, and multi-argument exception formatting similarly checks an accumulated buffer; use explicit item/first state rather than rendered buffer length to preserve separators.
AGENTS.md reference: AGENTS.md:L231-L233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ck out of execve's error
Building a container's repr in a buffer replaced the `parts.join(", ")` the
list, tuple and exception paths used with a separator emitted when the buffer
was already non-empty. An item whose `__repr__` answers `""` writes nothing,
so the next item lost its separator: `repr([E(), E()])` gave `[]` rather than
`[, ]`, and `repr(ValueError(E(), E()))` gave `ValueError()`. Emit by position
instead, the way `listobject.py:213-223`, `tupleobject.py:114`,
`dictmultiobject.py:388` and `interp_exceptions.py:135-147` join theirs.
`list_repr` also stops rather than skips when an item is gone, per
`listobject.py:217-221`.
`os.execve` reported the program path as `OSError.filename`;
`interp_posix.py:1812,1817` wraps the failure with `wrap_oserror`, which names
no file, as `execv` already did.
Documents `py_repr_wtf8` against `listobject.py:206-225`, whose
`rutf8.Utf8StringBuilder` the `Wtf8Buf` stands in for.
Adds pyre/extra_tests/parity_tests/repr_empty_item_separator.py.
Assisted-by: Claude
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/extra_tests/parity_tests/os_boundary_surrogateescape.py`:
- Around line 183-188: Update the subprocess validation around read_back so a
nonzero returncode fails the test and reports the child’s captured output; only
perform the existing stdout equality assertion after confirming the child
completed successfully.
In `@pyre/pyre-interpreter/src/_structseq.rs`:
- Line 175: Replace each transient representation or formatting call to
w_str_from_wtf8 with w_str_from_wtf8_managed in
pyre/pyre-interpreter/src/_structseq.rs:175,
pyre/pyre-interpreter/src/module/_collections/mod.rs:1305,
pyre/pyre-interpreter/src/module/array/mod.rs:959,
pyre/pyre-interpreter/src/type_methods.rs:1676, and both dictionary-view and
union representation sites in pyre/pyre-interpreter/src/typedef.rs:6728-6730 and
9114-9116.
In `@pyre/pyre-interpreter/src/module/posix/interp_posix.rs`:
- Around line 2447-2449: Update stat_impl’s fsencode_path_w call to propagate
the converter result directly with ?, removing the blanket map_err that rewrites
errors as TypeError; preserve the converter’s ValueError for embedded NULs and
unchanged exceptions from user __fspath__. Match the existing chown_entry
handling pattern.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4c1f80ea-e483-4407-abcf-2c5b3e9b5c01
📒 Files selected for processing (18)
pyre/extra_tests/parity_tests/compile_filename_boundary.pypyre/extra_tests/parity_tests/os_boundary_surrogateescape.pypyre/extra_tests/parity_tests/repr_empty_item_separator.pypyre/extra_tests/parity_tests/repr_surrogate_wtf8.pypyre/pyre-interpreter/src/_structseq.rspyre/pyre-interpreter/src/builtins.rspyre/pyre-interpreter/src/display.rspyre/pyre-interpreter/src/gateway.rspyre/pyre-interpreter/src/host_seam.rspyre/pyre-interpreter/src/module/_collections/mod.rspyre/pyre-interpreter/src/module/_random/macro_smoke.rspyre/pyre-interpreter/src/module/array/mod.rspyre/pyre-interpreter/src/module/posix/interp_posix.rspyre/pyre-interpreter/src/pycode.rspyre/pyre-interpreter/src/runtime_ops.rspyre/pyre-interpreter/src/type_methods.rspyre/pyre-interpreter/src/typedef.rspyre/pyre-macros/src/lib.rs
| read_back = subprocess.run( | ||
| [sys.executable, "-c", "import os,sys; sys.stdout.buffer.write(os.environb[b'%s'])" % NAME], | ||
| capture_output=True, | ||
| ) | ||
| if read_back.returncode == 0: | ||
| assert read_back.stdout == b"v\xff", read_back.stdout |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Report the child failure instead of skipping the assertion.
The assertion runs only when returncode == 0. If the child process fails for any reason, the test passes without checking the environment value, so a regression in putenv byte handling stays hidden. Surface the child output when the run fails.
♻️ Proposed change
read_back = subprocess.run(
[sys.executable, "-c", "import os,sys; sys.stdout.buffer.write(os.environb[b'%s'])" % NAME],
capture_output=True,
+ check=False,
)
- if read_back.returncode == 0:
- assert read_back.stdout == b"v\xff", read_back.stdout
+ assert read_back.returncode == 0, read_back.stderr
+ assert read_back.stdout == b"v\xff", read_back.stdout📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| read_back = subprocess.run( | |
| [sys.executable, "-c", "import os,sys; sys.stdout.buffer.write(os.environb[b'%s'])" % NAME], | |
| capture_output=True, | |
| ) | |
| if read_back.returncode == 0: | |
| assert read_back.stdout == b"v\xff", read_back.stdout | |
| read_back = subprocess.run( | |
| [sys.executable, "-c", "import os,sys; sys.stdout.buffer.write(os.environb[b'%s'])" % NAME], | |
| capture_output=True, | |
| check=False, | |
| ) | |
| assert read_back.returncode == 0, read_back.stderr | |
| assert read_back.stdout == b"v\xff", read_back.stdout |
🧰 Tools
🪛 Ruff (0.16.1)
[error] 183-183: subprocess call: check for execution of untrusted input
(S603)
[warning] 183-183: subprocess.run without explicit check argument
Add explicit check=False
(PLW1510)
[warning] 184-184: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/os_boundary_surrogateescape.py` around lines
183 - 188, Update the subprocess validation around read_back so a nonzero
returncode fails the test and reports the child’s captured output; only perform
the existing stdout equality assertion after confirming the child completed
successfully.
| parts.join(", ") | ||
| ))) | ||
| out.push_str(")"); | ||
| Ok(pyre_object::w_str_from_wtf8(out)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use the managed constructor for transient representation strings.
w_str_from_wtf8 allocates the Unicode object outside the managed GC path. These runtime paths create a new temporary string on each representation or formatting call. Repeated calls can retain allocations and increase process memory.
pyre/pyre-interpreter/src/_structseq.rs#L175-L175: replacew_str_from_wtf8withw_str_from_wtf8_managed.pyre/pyre-interpreter/src/module/_collections/mod.rs#L1305-L1305: use the managed constructor for the deque representation.pyre/pyre-interpreter/src/module/array/mod.rs#L959-L959: use the managed constructor for the array representation.pyre/pyre-interpreter/src/type_methods.rs#L1676-L1676: use the managed constructor for the temporary!rconversion result.pyre/pyre-interpreter/src/typedef.rs#L6728-L6730: use the managed constructor for dictionary-view representations.pyre/pyre-interpreter/src/typedef.rs#L9114-L9116: use the managed constructor for union representations.
📍 Affects 5 files
pyre/pyre-interpreter/src/_structseq.rs#L175-L175(this comment)pyre/pyre-interpreter/src/module/_collections/mod.rs#L1305-L1305pyre/pyre-interpreter/src/module/array/mod.rs#L959-L959pyre/pyre-interpreter/src/type_methods.rs#L1676-L1676pyre/pyre-interpreter/src/typedef.rs#L6728-L6730pyre/pyre-interpreter/src/typedef.rs#L9114-L9116
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/_structseq.rs` at line 175, Replace each transient
representation or formatting call to w_str_from_wtf8 with
w_str_from_wtf8_managed in pyre/pyre-interpreter/src/_structseq.rs:175,
pyre/pyre-interpreter/src/module/_collections/mod.rs:1305,
pyre/pyre-interpreter/src/module/array/mod.rs:959,
pyre/pyre-interpreter/src/type_methods.rs:1676, and both dictionary-view and
union representation sites in pyre/pyre-interpreter/src/typedef.rs:6728-6730 and
9114-9116.
| let path = crate::gateway::fsencode_path_w(args[0]).map_err(|_| { | ||
| crate::PyError::type_error("stat: path should be string, bytes, os.PathLike") | ||
| })?; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Propagate the path-converter error in stat_impl instead of rewriting it to TypeError.
fsencode_path_w reports an embedded NUL as ValueError, and it lets an exception raised inside a user __fspath__ out unchanged, including RuntimeError and UnicodeEncodeError. The blanket map_err turns all of them into TypeError, so os.stat and os.lstat lose those results. The same file states this rule for chown_entry at lines 4118-4123 and uses ? there.
This repeats a previous review comment that was reported as addressed.
🐛 Proposed fix
- let path = crate::gateway::fsencode_path_w(args[0]).map_err(|_| {
- crate::PyError::type_error("stat: path should be string, bytes, os.PathLike")
- })?;
+ let path = crate::gateway::fsencode_path_w(args[0])?;As per coding guidelines, when porting RPython/PyPy code, maintain strict line-by-line structural parity; do not take shortcuts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-interpreter/src/module/posix/interp_posix.rs` around lines 2447 -
2449, Update stat_impl’s fsencode_path_w call to propagate the converter result
directly with ?, removing the blanket map_err that rewrites errors as TypeError;
preserve the converter’s ValueError for embedded NULs and unchanged exceptions
from user __fspath__. Match the existing chown_entry handling pattern.
Source: Coding guidelines
1 participant
Four independent fixes at the filesystem-name boundary. The first is a
wrong-behaviour bug, not a reporting one.
posix paths aliased onto one another
gateway::fsencode_wended inString::from_utf8_lossy, and the posix moduleused it for every path argument, so a path byte with no UTF-8 spelling folded to
U+FFFD before the syscall was made. Two names differing only in such a byte
became one name:
Upstream never folds:
space.fsencode_w(baseobjspace.py:1970) answers with abyte string and
dispatch_filename(interp_posix.py:56-115) hands those bytesto the call unchanged. The extraction now yields bytes, and
OsStr::from_bytescarries them to the syscall on unix; the text form stays only where the platform
has no byte spelling of a path.
fsencode_wandfsencode_w_with_kindare deleted rather than left reachable,and the
PyPath/PyPathOrNonemacro bindings — which stand forgateway.py:365unwrap_spec'fsencode'— bindVec<u8>.OSError.filenamenow carries the path object the call was given(
interp_posix.py:332 wrap_oserror2(space, e, w_path)), so a bytes path reportsbytes and a str keeps its surrogate escapes. For an
os.PathLike, that object isthe result of the single
__fspath__call, not the wrapper that supplied it(
interp_posix.py:211-219 _unwrap_path), so the two travel together in onerecord. All eight rows across
os.stat/os.open/open/io.FileIOmatch CPython and PyPy.
compile() dropped its filename, silently
compiling.py:13declaresunwrap_spec(filename='fsencode', mode='text'), butbuiltin_compilebound both asif is_str { ... } else { <default> }:compile(s, b"ascii.py", "exec").co_filename'ascii.py''ascii.py''<string>'compile(s, b"\xff.py", "exec").co_filename'\udcff.py''\udcff.py''<string>'compile(s, '\udcff.py', "exec")'\udcff.py''\udcff.py'__fspath__'\udcff.py''\udcff.py''<string>'compile(s, 42, "exec")'<string>'compile(s, bytearray(b"ba.py"), "exec")'ba.py'+ warning'<string>'compile(s, "a\x00b.py", "exec")compile(s, "a.py", 42)execThe first row needs no exotic encoding: a plain ASCII
bytesfilename was lost,and every traceback, warning and frame built from that code object pointed at
<string>. Routing filename throughfsencode_bytes_wand mode throughtext_wyields the whole table, since the converter already carries thedeprecation warning, both TypeError spellings and the NUL rejection. The
bytearrayrow is where CPython and PyPy differ; we follow PyPy.The filename names the whole compilation unit, so nested constants inherit it
when they are boxed — otherwise
outer.co_filenamewas exact whileouter.co_consts[0]and the function's__code__stayed lossy. That is whatseparates it from
pycode.py:431, whose constructor andreplacefilenamesrename only the object being built, so the two get separate entry points.
repr built in a buffer that could not hold what it was given
code_reprformattedsource_path, so afterc.replace(co_filename=...)itreported the name the object was compiled under, not the current one. It now
builds from
code_filename_bytesas WTF-8, the waypycode.py:570 reprreachesfor the filename's
utf8_w.That made a code object the second thing whose repr can carry a lone surrogate,
and
py_repr_wtf8was a hand-maintained prelude listing such types beforedelegating to
py_repr, whoseStringcannot hold one. Frames were on the listand code objects were not, so
repr(code)aborted inw_str_get_value.Any new such type repeats that, and the delegation also lost the surrogate for
every container — reachable on
maintoday, independent of this change:So the two are inverted:
py_repr_wtf8/py_str_wtf8carry the wholedispatch,
py_repr/py_strare lossy views of them for the ~109 callers thatgenuinely want a Rust
String(error-message interpolation and the like), andthe prelude's per-type carve-outs are deleted rather than extended.
dict_repr,list_repr,tuple_repr,array_repr_wtf8, the deque / set / frozenset /slice / range / structseq / exception formatting, and the
!rconversion inf-strings,
str.formatand the descriptor slots build aWtf8Bufand recursethrough
py_repr_wtf8— the same shape aslistobject.py:206-225 _listrepr_inner, which appendsspace.utf8_len_w(space.repr(w_item))into arutf8.Utf8StringBuilder.No repr's text changes.
ReprGuard::enterand thestack_check()areuntouched, so
[[...]],{1: {...}}and theRecursionErroron a200000-deep list all still hold, and every slot keeps the allocation kind it
had (a per-call
__repr__result stays GC-managed rather than becoming animmortal
malloc_rawbuffer).Buffer assembly did cost one thing, caught by the Codex parity review: the
parts.join(", ")it replaced separates by position, while emitting aseparator whenever the buffer is non-empty separates by what was written. An
item whose
__repr__answers""writes nothing, so the next item lost itsseparator —
repr([E(), E()])gave[], andrepr(ValueError(E(), E()))gave
ValueError(). All four sites now go by index,dict_reprincluded(which was accidentally safe, since its
": "always lands).putenv wrote its value through a lossy decode
os.putenv/os.unsetenvtake both halves through the same'fsencode'converter, and
posix.environalready hands entries back as bytes, so a valuespelling a byte with no UTF-8 form has to survive the write. They now carry
Vec<u8>toset_var/remove_var— which already acceptimpl AsRef<OsStr>,so no change outside pyre was needed — with the NUL rejection and the
interp_posix.py:1762-1769key rule (=illegal only after index 0, so the=C:form Windows uses for a working directory stays legal).Verification
pyre/extra_tests/parity_tests(176): nodynasm=FAIL. The one failure istype_members_python314.py cpython=FAIL, the system CPython, not pyre.pyre/extra_testsdynasm: 235/241, up from 234; the remaining six are asubset of the previous seven.
cargo test -p pyre-interpreter --features dynasm: 490 passed, 0 failed.compile_filename_boundary.py,repr_surrogate_wtf8.py,repr_empty_item_separator.pyand the extendedos_boundary_surrogateescape.pypass under CPython and PyPy too; thebytearrayfilename row is left out because CPython and PyPy disagree there.Not in this change
_path_splitrootand the Windowsarg_pathstill go through text. Both aretext-level operations on a name rather than a syscall boundary, and each
carries the reason in place.
os.execvereports no filename, which is whatinterp_posix.py:1812,1817does and what this branch's base did. CPython reports the program path there;
the divergence predates this change.
String-returning#[pymethod]is wrapped withw_str_new, which is theimmortal
malloc_rawpath — sodeque.__repr__and its neighbours leak percall. That predates this change and is a macro-level fix.
Summary by CodeRabbit
compile()filename handling, including path-like values, invalid inputs, syntax errors, and code-object representations.