Drive zlib, binascii, and struct builtins off the __pyre_kw__ marker ABI - #1123
Conversation
WalkthroughThe PR adds positional-only signature support to the function macro. It migrates ChangesBuiltin signature binding migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 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 |
![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: 1248478ee3
ℹ️ 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".
| if w.is_null() || unsafe { is_none(w) } { | ||
| return Ok(default); |
There was a problem hiding this comment.
When a caller passes binascii.crc32(data, None), this branch treats the explicit None like an omitted slot and computes with CRC 0. Before this change, the second positional value was passed to int_w, and PyPy's oldcrc='truncatedint_w' converter likewise raises TypeError for None; only PY_NULL should select the default. Keep the null check for omission but pass an explicit None through integer conversion.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit fe5d0d8). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
1248478 to
783710e
Compare
youknowone
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/snippets/stdlib_struct.py`:
- Around line 114-120: Extend the struct.pack and struct.pack_into tests to call
each hand-registered gateway with required positional arguments omitted,
including the module and bound-method variants represented by struct.pack,
struct.pack_into, and _s.pack_into. Assert that every incomplete call raises
TypeError, while preserving the existing keyword-rejection cases.
In `@pyre/pyre-interpreter/src/module/binascii/mod.rs`:
- Around line 35-40: Update slot_bool to return Result<bool, PyError> and
propagate errors from crate::baseobjspace::is_true instead of replacing them
with the default. Adjust all migrated callers— including a2b_qp, b2a_qp,
a2b_base64, b2a_base64, and b2a_uu—to return/propagate the result with ?, while
preserving default handling for null or None values.
- Around line 42-49: Update slot_u32 and crc_hqx to use
crate::baseobjspace::truncatedint_w when reading CRC initialization integers, so
values are truncated to the low 32 bits instead of raising OverflowError.
Preserve the existing default handling for null or None arguments.
In `@pyre/pyre-interpreter/src/module/zlib/mod.rs`:
- Around line 381-386: Update the max_length handling in
_ZlibDecompressor.decompress so an omitted or None argument remains the default,
zero maps to unlimited decompression, and explicitly negative values raise
ValueError instead of being discarded by usize::try_from. Preserve the existing
conversion and downstream behavior for non-negative values.
In `@pyre/pyre-macros/src/lib.rs`:
- Around line 287-290: Add compile-time validation in the pyre_function
signature-generation flow around marker_posonly so #[posonly] appears before any
#[kwonly] parameter and before **kwargs; reject invalid ordering instead of
allowing marker_posonly() to panic. Also require an explicit trailing
positional-only boundary when the signature is entirely positional-only or
transitions from positional-only to kw-only parameters, preserving the existing
marker emission for valid non-trailing cases.
🪄 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: 383bac88-3e6a-435f-9fc9-752fcaf9dbd3
📒 Files selected for processing (6)
pyre/extra_tests/snippets/stdlib_struct.pypyre/pyre-interpreter/src/module/_random/macro_smoke.rspyre/pyre-interpreter/src/module/binascii/mod.rspyre/pyre-interpreter/src/module/struct/mod.rspyre/pyre-interpreter/src/module/zlib/mod.rspyre/pyre-macros/src/lib.rs
| # pack / pack_into (module and method) reject keyword arguments. | ||
| with assert_raises(TypeError): | ||
| struct.pack(format="ii") | ||
| with assert_raises(TypeError): | ||
| struct.pack_into("ii", bytearray(8), 0, 1, 2, extra=3) | ||
| with assert_raises(TypeError): | ||
| _s.pack_into(bytearray(8), 0, 1, 2, extra=3) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add cases for omitted required arguments.
The new tests cover keyword rejection only. The binder pads omitted positionals with PY_NULL, so the missing-argument path is the risky one for the hand-registered vararg gateways. Add calls that omit required slots.
💚 Proposed additional cases
with assert_raises(TypeError):
_s.pack_into(bytearray(8), 0, 1, 2, extra=3)
+
+# pack / pack_into reject calls that omit required positional slots.
+with assert_raises(TypeError):
+ struct.pack()
+with assert_raises(TypeError):
+ struct.pack_into("ii", bytearray(8))
+with assert_raises(TypeError):
+ _s.pack_into()
+with assert_raises(TypeError):
+ _s.pack_into(bytearray(8))📝 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.
| # pack / pack_into (module and method) reject keyword arguments. | |
| with assert_raises(TypeError): | |
| struct.pack(format="ii") | |
| with assert_raises(TypeError): | |
| struct.pack_into("ii", bytearray(8), 0, 1, 2, extra=3) | |
| with assert_raises(TypeError): | |
| _s.pack_into(bytearray(8), 0, 1, 2, extra=3) | |
| # pack / pack_into (module and method) reject keyword arguments. | |
| with assert_raises(TypeError): | |
| struct.pack(format="ii") | |
| with assert_raises(TypeError): | |
| struct.pack_into("ii", bytearray(8), 0, 1, 2, extra=3) | |
| with assert_raises(TypeError): | |
| _s.pack_into(bytearray(8), 0, 1, 2, extra=3) | |
| # pack / pack_into reject calls that omit required positional slots. | |
| with assert_raises(TypeError): | |
| struct.pack() | |
| with assert_raises(TypeError): | |
| struct.pack_into("ii", bytearray(8)) | |
| with assert_raises(TypeError): | |
| _s.pack_into() | |
| with assert_raises(TypeError): | |
| _s.pack_into(bytearray(8)) |
🤖 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/snippets/stdlib_struct.py` around lines 114 - 120, Extend
the struct.pack and struct.pack_into tests to call each hand-registered gateway
with required positional arguments omitted, including the module and
bound-method variants represented by struct.pack, struct.pack_into, and
_s.pack_into. Assert that every incomplete call raises TypeError, while
preserving the existing keyword-rejection cases.
| let max_length = match args.get(2).copied() { | ||
| Some(o) if !o.is_null() && !unsafe { is_none(o) } => { | ||
| usize::try_from(crate::baseobjspace::int_w(o)?).ok() | ||
| } | ||
| _ => None, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
CPython _ZlibDecompressor.decompress max_length negative zero behavior
💡 Result:
In CPython’s zlib.Decompress (implemented as _ZlibDecompressor.decompress in Modules/zlibmodule.c), max_length must be non-negative; a negative value (including “negative zero”) is rejected with ValueError. The relevant checks treat max_length < 0 as an error [1][2]. So there is no special “-0” behavior: if a value is truly negative, it fails the max_length < 0 check. Separately, when max_length == 0, CPython sets the effective limit to PY_SSIZE_T_MAX (i.e., “no practical limit” / whole input is decompressed), consistent with the documentation that max_length=0 decompresses the whole input and leaves unconsumed_tail empty [1][3][4]. How this maps to “negative zero”: - If you pass a real Python float -0.0, Python will convert it to an integer 0 when calling a signature expecting a Py_ssize_t, so CPython sees max_length == 0 (not negative) and proceeds with the max_length==0 path (unlimited/whole input decompressed) [1][5]. - If you somehow pass a negative integer-like value (e.g., -1), CPython detects max_length < 0 and raises ValueError [1][2]. Key evidence: - CPython C implementation: “if (max_length < 0) { PyErr_SetString(PyExc_ValueError, "max_length must be non-negative"); return NULL; } … else if (max_length == 0) hard_limit = PY_SSIZE_T_MAX; else hard_limit = max_length;” [1][2]. - Docs: “If max_length is zero then the whole input is decompressed, and unconsumed_tail is empty.” [3][4].
Citations:
- 1: https://github.com/python/cpython/blob/v3.9.4/Modules/zlibmodule.c
- 2: https://github.com/python/cpython/blob/v3.9.2/Modules/zlibmodule.c
- 3: https://github.com/python/cpython/blob/main/Doc/library/zlib.rst
- 4: https://docs.python.org/3.11/library/zlib.html
- 5: https://github.com/python/cpython/blob/main/Modules/zlibmodule.c
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant implementation without running repository code.
fd -a 'mod.rs|zlibmodule.c' . | sed 's#^\./##' | sort | head -50
echo
echo "--- pyre mod.rs around max_length ---"
sed -n '350,410p' pyre/pyre-interpreter/src/module/zlib/mod.rs
echo
echo "--- CPython Module/zlib.c max_length handling ---"
python3 - <<'PY'
# Read the relevant CPython source from upstream tags/sources without installing anything.
import urllib.request
urls = [
"https://raw.githubusercontent.com/python/cpython/main/Modules/zlibmodule.c",
"https://raw.githubusercontent.com/python/cpython/3.11/Modules/zlibmodule.c",
"https://raw.githubusercontent.com/python/cpython/v3.9.4/Modules/zlibmodule.c",
]
for url in urls:
try:
data = urllib.request.urlopen(url, timeout=20).read().decode("utf-8", "replace")
except Exception as e:
print(f"{url}: unreachable: {e}")
continue
idx = data.find("static PyObject *")
if idx != -1:
data = data[idx:]
for term in ["_zlib_decomp_decompress", "max_length"]:
i = data.find(term)
if i != -1:
start = max(0, i-1800)
end = min(len(data), i+2600)
print(f"URL: {url} found around {term}")
print(data[start:end])
print("---")
break
PYRepository: youknowone/pyre
Length of output: 6597
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- relevant int conversion ---"
rg -n "fn int_w|pub fn int_w|int_w\\(" pyre/pyre-interpreter/src -g '*.rs' -C 4
echo
echo "--- Python int conversion behavior probe ---"
python3 - <<'PY'
values = [-2**64, -2**32, -1, -0, 0, 1]
for x in values:
print(f"{x!r:20} as usize: {x.to_bytes((x.bit_length()//8)+1, 'big', signed=True).__len__()} via int(x): {x}; converted via object? {type(x)}")
try:
-1
except Exception as e:
print(repr(e))
PYRepository: youknowone/pyre
Length of output: 50374
🌐 Web query:
CPython Modules zlibmodule.c zlibmodule.c max_length decompress Decompress read
💡 Result:
In CPython, the zlib.Decompress.decompress method (defined in Modules/zlibmodule.c) uses the max_length parameter to limit the amount of uncompressed data returned in a single call [1][2]. When max_length is provided as a non-zero value, the decompress method ensures the returned bytes object does not exceed that length [2][3]. If the decompression process requires more output than max_length to consume the provided input, the operation stops, and any remaining input data is stored in the object's unconsumed_tail attribute [2][3]. This unconsumed_tail must be passed to subsequent calls to decompress to continue the process [2][3]. If max_length is zero (the default), the method attempts to decompress the entire input, and unconsumed_tail will be empty [2][3]. From an implementation perspective in Modules/zlibmodule.c, max_length dictates the allocation strategy for the output buffer [1][4]. Specifically: - Hard Limit Enforcement: The max_length is used to establish a hard_limit for the output buffer growth [4][5]. - Buffer Management: The implementation uses internal functions (such as OutputBuffer_Grow or arrange_output_buffer_with_maximum in various versions) to manage memory [4][5]. If the limit is reached, the decompressor halts output production, effectively allowing for controlled, chunked decompression of large streams [2][4]. - Input Handling: Any input data that could not be processed due to the max_length constraint is saved into self->unconsumed_tail via internal helper functions like save_unconsumed_input [4][5]. This functionality is primarily useful for memory-efficient processing of compressed data streams where the total output size may exceed available memory or when only specific-sized chunks are required [2][3].
Citations:
- 1: https://github.com/python/cpython/blob/main/Modules/zlibmodule.c
- 2: https://docs.python.org/3/library/zlib.html
- 3: https://github.com/python/cpython/blob/main/Doc/library/zlib.rst
- 4: https://github.com/python/cpython/blob/v3.9.5/Modules/zlibmodule.c
- 5: https://github.com/python/cpython/blob/3.11/Modules/zlibmodule.c
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
import ssl
ctx = ssl.SSLContext()
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
urls = [
"https://raw.githubusercontent.com/python/cpython/v3.9.5/Modules/zlibmodule.c",
"https://raw.githubusercontent.com/python/cpython/3.11/Modules/zlibmodule.c",
]
for url in urls:
try:
data = urllib.request.urlopen(url, context=ctx, timeout=30).read().decode("utf-8", "replace")
except Exception as e:
print(f"Fetch failed: {url}: {e}")
continue
for needle in ["_zlib_decomp_decompress", "z_decompress_impl"]:
i = data.find(needle)
if i != -1:
slice = data[max(0, i-2500): min(len(data), i+5500)]
print(f"=== {url} around {needle} ===")
print(slice)
break
PYRepository: youknowone/pyre
Length of output: 393
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from urllib import request
import ssl
ctx = ssl.SSLContext()
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
urls = [
"https://raw.githubusercontent.com/python/cpython/v3.9.5/Modules/zlibmodule.c",
"https://raw.githubusercontent.com/python/cpython/3.11/Modules/zlibmodule.c",
]
for url in urls:
try:
data = request.urlopen(url, context=ctx, timeout=30).read().decode("utf-8", "replace")
except Exception as e:
print(f"Fetch failed: {url}: {e}")
continue
needles = ["_zlib_decomp_decompress", "z_decompress_impl"]
found = False
for needle in needles:
i = data.find(needle)
if i != -1:
found = True
slice = data[max(0, i-3200): min(len(data), i+7200)]
print(f"=== {url} around {needle ===")
for line in slice.splitlines():
if "max_length" in line or "hard_limit" in line or "Py_ssize_t" in line or "ValueError" in line:
print(line)
break
if not found:
print(f"Naedle not found in {url}")
PY
echo
echo "--- inspect int_w declaration/implementation ---"
rg -n "pub fn int_w|fn int_w|Py_ssize_t|overflow_error" pyre/pyre-interpreter/src/base* pyre/pyre-interpreter/src/module -g '*.rs' | head -80Repository: youknowone/pyre
Length of output: 10614
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from urllib import request
import ssl
ctx = ssl.SSLContext()
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
urls = [
"https://raw.githubusercontent.com/python/cpython/v3.9.5/Modules/zlibmodule.c",
"https://raw.githubusercontent.com/python/cpython/3.11/Modules/zlibmodule.c",
]
for url in urls:
try:
data = request.urlopen(url, context=ctx, timeout=30).read().decode("utf-8", "replace")
except Exception as e:
print(f"Fetch failed: {url}: {e}")
continue
needles = ["_zlib_decomp_decompress", "z_decompress_impl"]
found = False
for needle in needles:
i = data.find(needle)
if i != -1:
found = True
slice = data[max(0, i-3200): min(len(data), i+7200)]
print(f"=== {url} around {needle} ===")
for line in slice.splitlines():
if "max_length" in line or "hard_limit" in line or "Py_ssize_t" in line or "ValueError" in line:
print(line)
break
if not found:
print(f"Needle not found in {url}")
PY
echo
echo "--- int_w implementation ---"
sed -n '8265,8295p' pyre/pyre-interpreter/src/baseobjspace.rsRepository: youknowone/pyre
Length of output: 1973
🌐 Web query:
github raw python/cpython Modules zlibmodule.c _zlib_decomp_decompress max_length
💡 Result:
In the Python CPython source code, _zlib_decomp_decompress (historically referenced as zlib_Decompress_decompress_impl) is the implementation function for the decompress() method of the zlib.Decompress object [1][2][3]. The max_length parameter, which is of type Py_ssize_t, allows a user to limit the size of the decompressed output data returned by a single call to the method [1][4][5]. Key technical details regarding max_length include: Functionality: If max_length is set to a non-zero value, the decompress() method will return no more than max_length bytes of decompressed data [4][5]. This mechanism is primarily used to prevent memory exhaustion when processing untrusted or potentially malicious compressed data (often referred to as zip bombs) [3]. Unconsumed Input: When max_length limits the output, any input data that could not be processed due to this limit is stored in the unconsumed_tail attribute of the Decompress object [4][5]. The user must pass this unconsumed_tail data to subsequent calls to decompress() to continue the decompression process [4][5]. If max_length is zero, the decompressor will attempt to decompress the entire input, and unconsumed_tail will remain empty [4][5]. Implementation: Within Modules/zlibmodule.c, the implementation manages memory for the output buffer [1][6]. When max_length is used, the code ensures the output buffer does not grow beyond this specified limit [6][3][7]. If the internal state machine (typically calling the underlying zlib library's inflate function) encounters a scenario where it would exceed this limit, it pauses, saves the remaining input, and returns the data processed up to that point [1][2][3]. Historical Context: The max_length parameter has been a standard feature for controlling decompression size in the zlib module for many years [8][7]. Since Python 3.6, max_length has been explicitly supported as a keyword argument [4][5].
Citations:
- 1: https://github.com/python/cpython/blob/main/Modules/zlibmodule.c
- 2: https://github.com/tamnd/gopy/blob/main/website/docs/annotations/modules/modules_zlib3_detail.mdx
- 3: https://gopy.tamnd.com/docs/annotations/modules/modules_zlib7_detail
- 4: https://docs.python.org/3/library/zlib.html
- 5: https://github.com/python/cpython/blob/main/Doc/library/zlib.rst
- 6: https://github.com/python/cpython/blob/3.12/Modules/zlibmodule.c
- 7: https://github.com/python/cpython/blob/v3.0/Modules/zlibmodule.c
- 8: https://docs.python.org/3.9/library/zlib.html
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from urllib import request
import ssl
urls = ["https://raw.githubusercontent.com/python/cpython/main/Modules/zlibmodule.c"]
ctx = ssl.SSLContext()
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
for url in urls:
data = request.urlopen(url, context=ctx, timeout=30).read().decode("utf-8", "replace")
for needle in ["_zlib_decomp_decompress", "z_decompress_impl"]:
idx = data.find(needle)
if idx != -1:
lines = data.splitlines()
for j in range(max(0, idx.count('\n') - 200), idx.count('\n') + 300):
line = lines[j]
if "max_length" in line or "hard_limit" in line or "Py_ssize_t" in line or "ValueError" in line:
print(f"{j+1}: {line}")
PY
echo
echo "--- relevant Rust conversions ---"
sed -n '4408,4426p' pyre/pyre-interpreter/src/baseobjspace.rs
sed -n '8302,8307p' pyre/pyre-interpreter/src/baseobjspace.rsRepository: youknowone/pyre
Length of output: 1364
Reject negative max_length values in _ZlibDecompressor.decompress.
max_length=-1 should remain the default only when omitted/None; an explicitly passed negative value must raise ValueError. max_length=0 should be treated as an unlimited decompression.
🤖 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/zlib/mod.rs` around lines 381 - 386, Update
the max_length handling in _ZlibDecompressor.decompress so an omitted or None
argument remains the default, zero maps to unlimited decompression, and
explicitly negative values raise ValueError instead of being discarded by
usize::try_from. Preserve the existing conversion and downstream behavior for
non-negative values.
| if !posonly_marked && pt.attrs.iter().any(|a| a.path().is_ident("posonly")) { | ||
| sig_stmts.push(quote! { __b.marker_posonly(); }); | ||
| posonly_marked = true; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
while IFS= read -r file; do
if rg -q '#\[(posonly|kwonly|kwargs)\]' "$file"; then
ast-grep outline "$file" --items all --type function
rg -n -C 12 '#\[(posonly|kwonly|kwargs)\]' "$file"
fi
done < <(fd --type f --extension rs . pyre)Repository: youknowone/pyre
Length of output: 205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked Rust files under pyre matching macro/keyword markers:\n'
git ls-files 'pyre/**/*.rs' | rg 'lib\.rs$|pyre-macros|gateway|signature|marker' || true
printf '\nMacro candidate locations:\n'
fd -e rs --hidden . pyre pyre-macros | sort
printf '\nFind marker attributes:\n'
rg -n -C 8 '#\[(posonly|kwonly|kwargs)\]' pyre -g '*.rs' || true
printf '\nFind SignatureBuilder / marker methods:\n'
rg -n -C 6 'struct SignatureBuilder|impl SignatureBuilder|marker_posonly|marker_kwonly|marker_posonly\(|marker_kwonly\(' pyre -g '*.rs' || trueRepository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Marker-only use sites, grouped by file, with line range and attribute order:\n'
rg -n -W 10 '#\[(posonly|kwonly|kwargs)\]' pyre -g '*.rs' | \
awk '
/^pyre\// { f=$0 }
/#\[(posonly|kwonly|kwargs)\]/ {
line=$0; sub(/^[0-9]+:/, "", line)
if (attr != "") print attr
attr=line
files[++n]=f; lines[n]=$0; attrs[n]=line
}
END {
if (n==0) { print "NO_MARKER_USAGE" }
for (i=1;i<=n;i++) {
if (i>1 && files[i] != files[i-1]) print "--- " files[i-1] "---"
print lines[i] "\n" attrs[i]
}
}
' || true
printf '\nSignatureBuilder definitions and marker implementation snippets:\n'
rg -n -C 8 'struct SignatureBuilder|impl SignatureBuilder|marker_posonly|stringify\(posonlyargcount\)|marker_kwonly' pyre pyre-macros -g '*.rs' || true
printf '\npyre_function macro relevant sections:\n'
sed -n '220,330p' pyre/pyre-macros/src/lib.rs
sed -n '1896,1970p' pyre/pyre-macros/src/lib.rsRepository: youknowone/pyre
Length of output: 29243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Markdown-safe marker-only use sites, grouped by file:\n'
python3 - <<'PY'
from pathlib import Path
import re
files = [Path(p) for p in __import__('subprocess').check_output(
['git','ls-files','pyre','**-/*.rs'], text=True).splitlines()
]
for path in files:
text = path.read_text(errors='replace')
if re.search(r'(?m)^#\[(posonly|kwonly|kwargs)\]', text):
print(f'--- {path} ---')
lines = text.splitlines()
in_block = False
for i, line in enumerate(lines, 1):
if re.search(r'(?m)^#\[(posonly|kwonly|kwargs)\]', line):
start=max(1,i-6); end=min(len(lines),i+8)
print(f'[{start}-{end}]')
for j in range(start,end+1):
print(f'{j}: {lines[j-1]}')
in_block=True
if not in_block:
print('(no matches?')
PY
printf '\nCurrent SignatureBuilder marker contract:\n'
sed -n '68,79p' pyre/pyre-interpreter/src/gateway.rs
printf '\nCurrent pyre_functions signature emission around posonly/kwonly:\n'
sed-
n '262,303p' pyre/pyre-macros/src/lib.rsRepository: youknowone/pyre
Length of output: 825
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Current pyre_functions signature emission around posonly/kwonly:\n'
sed -n '262,303p' pyre/pyre-macros/src/lib.rs
printf '\nPython behavioral probe of SignatureBuilder marker ordering and terminal posonly:\n'
python3 - <<'PY'
class SignatureBuilder:
def __init__(self):
self.name = "__test__"
self.argnames = []
self.varargname = None
self.kwargname = None
self.posonlyargcount = 0
self.kwonlystartindex = -1
def append(self, argname):
self.argnames.append(argname)
def marker_posonly(self):
assert self.posonlyargcount == 0 and self.kwonlystartindex == -1
self.posonlyargcount = len(self.argnames)
def marker_kwonly(self):
assert self.kwonlystartindex == -1
self.kwonlystartindex = len(self.argnames)
def signature(self):
kwonlyargcount = len(self.argnames) - self.kwonlystartindex if self.kwonlystartindex != -1 else 0
return self.argnames, self.varargname, self.kwargname, kwonlyargcount, self.posonlyargcount
for attrs, name in [
(['data', 'kwonly', 'default', 'strict_mode'], 'posonly before kwonly'),
(['kwonly', 'default', 'fix_imports', 'posonly', 'default', 'encoding'], 'posonly after kwonly'),
(['posonly', 'data'], 'posonly before end'),
(['data'], 'no marker'),
]:
print(f'--- {name} ---')
b=SignatureBuilder()
calls=[]
for item in attrs:
if item in ('kwonly','#[kwonly]'): calls.append('marker_kwonly'); b.marker_kwonly()
elif item in ('posonly','#[posonly]'): calls.append('marker_posonly'); b.marker_posonly()
else: b.append(item)
try:
print(calls, 'ok signature=', b.signature())
except AssertionError as e:
print(calls, 'panic asserted order violation:', e)
PYRepository: youknowone/pyre
Length of output: 2357
Validate #[posonly] order and end-of-signature placement.
pyre_function emits marker_posonly() at the first parameter that has #[posonly], then appends the remaining parameters. SignatureBuilder::marker_posonly() asserts that marker_kwonly() has not run, so a later #[kwonly] parameter panics. A trailing positional-only boundary with no following parameter is not represented. Add compile-time validation for #[posonly] to appear before any #[kwonly] and before **kwargs, and require an explicit boundary form for all-positional-only or positional-only-with-kwonly signatures.
🤖 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-macros/src/lib.rs` around lines 287 - 290, Add compile-time
validation in the pyre_function signature-generation flow around marker_posonly
so #[posonly] appears before any #[kwonly] parameter and before **kwargs; reject
invalid ordering instead of allowing marker_posonly() to panic. Also require an
explicit trailing positional-only boundary when the signature is entirely
positional-only or transitions from positional-only to kw-only parameters,
preserving the existing marker emission for valid non-trailing cases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 783710ec9b
ℹ️ 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".
| let buf = unsafe { readbuf(buffer)? }; | ||
| let offset = match offset { | ||
| Some(o) => unsafe { crate::builtins::space_index_w(o)? }, |
There was a problem hiding this comment.
When offset is an object with a Python __index__, space_index_w can execute arbitrary code after readbuf has returned a raw slice; that code can resize an underlying bytearray/array/mmap or otherwise invalidate the borrowed storage, after which do_unpack_from reads the stale slice. PyPy's @unwrap_spec(offset=int) performs this coercion before entering the body, and the previous resolve_buffer_offset did likewise. Convert offset before readbuf in both this method and the module-level unpack_from implementation.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| this.ensure_ready()?; | ||
| let format = majit_metainterp::jit::promote_string(this.format); | ||
| let fmt = unsafe { w_str_get_value(format) }; | ||
| let offset = unsafe { crate::builtins::space_index_w(args[2])? }; |
There was a problem hiding this comment.
When Struct.pack_into is called without both buffer and offset, the signature binder pads the missing slots with PY_NULL, but this method passes the null offset directly to space_index_w. Thus calls such as Struct('i').pack_into(bytearray(4)) now report that an object cannot be interpreted as an integer instead of the required-argument error that the previous implementation explicitly produced. Check args[1] and args[2] before converting either slot, as the new module-level pack_into already does.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
783710e to
8e86c18
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e86c18976
ℹ️ 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".
| // interp_zlib.py:66 `compress(data, __posonly__=None, level, wbits)` — | ||
| // `data` positional-only, `level`/`wbits` positional-or-keyword. | ||
| fn compress( | ||
| data: PyBufferStr, |
There was a problem hiding this comment.
When data is a bytearray and level or wbits has a custom __int__/__index__, the wrapper first converts PyBufferStr into a raw slice through charbuf_w, then int_or_default executes arbitrary Python code without holding a buffer export. If that coercion resizes the bytearray, the saved slice is invalidated before backend::compress reads it, causing stale-memory reads or corrupted output; decompress has the same ordering at line 517. The previous implementation copied with as_bytes before coercing these arguments, so retain an owned copy or an active export across coercion.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
bb9f920 to
d20e34d
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/snippets/stdlib_marshal.py`:
- Around line 114-116: Extend the positional-only API assertions near the
existing marshal.loads keyword test to also cover marshal.load with a BytesIO
file object passed by keyword. Import and reuse BytesIO as done in
test_file_api, and assert that marshal.load(file=...) raises TypeError.
In `@pyre/extra_tests/snippets/stdlib_struct.py`:
- Around line 105-112: Add a negative assertion to the unpack_from tests showing
that passing format by keyword raises the expected positional-only TypeError,
while retaining the existing valid positional and keyword cases for buffer and
offset in the struct.unpack_from block.
In `@pyre/pyre-interpreter/src/module/marshal/mod.rs`:
- Around line 586-598: Refactor load to call unmarshal_bytes instead of
duplicating wire deserialization and reject_code handling. Update
unmarshal_bytes to return both the deserialized value and consumed byte count,
then use that count with before and seek the file accordingly while preserving
the allow_code behavior in unmarshal_bytes.
- Around line 530-541: The marshal wrappers must root live PyObjectRef arguments
before option resolution can invoke Python. In
pyre/pyre-interpreter/src/module/marshal/mod.rs:530-541 (marshal.dumps),
544-553, 557-571, and 575-593, add pin_root and shadow_stack_get handling for
every argument used after resolve_version or resolve_allow_code, then pass the
rooted values into marshal_to_bytes or the corresponding load/dump operation
while preserving existing behavior.
In `@pyre/pyre-interpreter/src/module/struct/mod.rs`:
- Around line 1253-1275: Update the missing-argument errors in pack and
pack_into to use the gateway’s standard phrasing for omitted required positional
arguments, including the function name and explicit missing-argument count where
applicable. Keep the existing null checks and error type unchanged, and preserve
strict structural parity with the neighboring gateway diagnostics.
In `@pyre/pyre-interpreter/src/module/zlib/mod.rs`:
- Around line 510-511: Replace the unchecked i64-to-i32 cast for level with a
checked conversion that raises OverflowError when the value is outside the C int
range. Apply the same range validation at both level call sites in the compress
and compressobj paths, while preserving the existing default handling.
- Around line 84-104: Update int_or_default and zdict_or_none to treat only
PY_NULL/absent slots as omitted; remove the is_none checks so explicit None is
passed through to the existing integer and byte-conversion handling. Preserve
PY_NULL defaults for omitted optional arguments, allowing compress, decompress,
and compressobj to retain their upstream None behavior.
🪄 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: ca61ff47-f2e4-4e3b-9940-8dd43066cc74
📒 Files selected for processing (8)
pyre/extra_tests/snippets/stdlib_marshal.pypyre/extra_tests/snippets/stdlib_struct.pypyre/pyre-interpreter/src/module/_random/macro_smoke.rspyre/pyre-interpreter/src/module/binascii/mod.rspyre/pyre-interpreter/src/module/marshal/mod.rspyre/pyre-interpreter/src/module/struct/mod.rspyre/pyre-interpreter/src/module/zlib/mod.rspyre/pyre-macros/src/lib.rs
| # `bytes` / `file` are positional-only too. | ||
| with self.assertRaises(TypeError): | ||
| marshal.loads(bytes=data) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add the load(file=...) assertion that the comment claims.
The comment states that bytes and file are positional-only, but only marshal.loads(bytes=data) is tested. Add the load case so the file boundary is covered.
💚 Proposed test addition
# `bytes` / `file` are positional-only too.
with self.assertRaises(TypeError):
marshal.loads(bytes=data)
+ with self.assertRaises(TypeError):
+ marshal.load(file=BytesIO(data))Import BytesIO in this test, as test_file_api does.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 115-115: dill and marshal deserialize arbitrary objects/bytecode and execute code on untrusted input. Use a safe serialization format and never load untrusted data.
Context: marshal.loads(bytes=data)
Note: [CWE-502] Deserialization of Untrusted Data.
(dill-marshal-deserialization-python)
🪛 OpenGrep (1.26.0)
[WARNING] 116-116: marshal.loads() can execute arbitrary code during deserialization. Use a safe format like JSON instead.
(coderabbit.deserialization.python-marshal)
🪛 Ruff (0.16.1)
[warning] 115-115: Use pytest.raises instead of unittest-style assertRaises
Replace assertRaises with pytest.raises
(PT027)
[error] 116-116: Deserialization with the marshal module is possibly dangerous
(S302)
🤖 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/snippets/stdlib_marshal.py` around lines 114 - 116, Extend
the positional-only API assertions near the existing marshal.loads keyword test
to also cover marshal.load with a BytesIO file object passed by keyword. Import
and reuse BytesIO as done in test_file_api, and assert that
marshal.load(file=...) raises TypeError.
|
|
||
| # unpack_from accepts buffer / offset positionally or by keyword. | ||
| _buf = struct.pack("ii", 111, 222) | ||
| assert struct.unpack_from("ii", _buf, offset=0) == (111, 222) | ||
| assert struct.unpack_from("ii", buffer=_buf, offset=0) == (111, 222) | ||
| _s = struct.Struct("ii") | ||
| assert _s.unpack_from(_buf, offset=0) == (111, 222) | ||
| assert _s.unpack_from(buffer=_buf) == (111, 222) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add the positional-only negative case for format.
The block proves that buffer and offset accept keywords. It does not prove that format rejects one. That assertion pins the posonly boundary direction for struct.unpack_from.
💚 Proposed additional case
assert _s.unpack_from(buffer=_buf) == (111, 222)
+with assert_raises(TypeError):
+ struct.unpack_from(format="ii", buffer=_buf)📝 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.
| # unpack_from accepts buffer / offset positionally or by keyword. | |
| _buf = struct.pack("ii", 111, 222) | |
| assert struct.unpack_from("ii", _buf, offset=0) == (111, 222) | |
| assert struct.unpack_from("ii", buffer=_buf, offset=0) == (111, 222) | |
| _s = struct.Struct("ii") | |
| assert _s.unpack_from(_buf, offset=0) == (111, 222) | |
| assert _s.unpack_from(buffer=_buf) == (111, 222) | |
| # unpack_from accepts buffer / offset positionally or by keyword. | |
| _buf = struct.pack("ii", 111, 222) | |
| assert struct.unpack_from("ii", _buf, offset=0) == (111, 222) | |
| assert struct.unpack_from("ii", buffer=_buf, offset=0) == (111, 222) | |
| _s = struct.Struct("ii") | |
| assert _s.unpack_from(_buf, offset=0) == (111, 222) | |
| assert _s.unpack_from(buffer=_buf) == (111, 222) | |
| with assert_raises(TypeError): | |
| struct.unpack_from(format="ii", buffer=_buf) |
🤖 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/snippets/stdlib_struct.py` around lines 105 - 112, Add a
negative assertion to the unpack_from tests showing that passing format by
keyword raises the expected positional-only TypeError, while retaining the
existing valid positional and keyword cases for buffer and offset in the
struct.unpack_from block.
| fn dumps( | ||
| value: PyObjectRef, | ||
| version: Option<PyObjectRef>, | ||
| #[posonly] | ||
| #[kwonly] | ||
| allow_code: Option<PyObjectRef>, | ||
| ) -> Result<PyObjectRef, crate::PyError> { | ||
| let version = resolve_version(version)?; | ||
| let allow_code = resolve_allow_code(allow_code)?; | ||
| let out = marshal_to_bytes(value, version, allow_code)?; | ||
| Ok(bytesobject::w_bytes_from_bytes(&out)) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the inline-function gateway roots incoming arguments for the callee,
# and compare with the rooting pattern used by other Python-capable builtins.
fd -t f 'gateway.rs' pyre/pyre-interpreter/src | xargs rg -n -C 8 'pin_root|shadow_stack_get|inline_function'
rg -n -C 4 'pin_root' pyre/pyre-interpreter/src/module/zlib/mod.rs pyre/pyre-interpreter/src/module/struct/mod.rsRepository: youknowone/pyre
Length of output: 7028
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== marshal file sections =="
wc -l pyre/pyre-interpreter/src/module/marshal/mod.rs
sed -n '480,610p' pyre/pyre-interpreter/src/module/marshal/mod.rs
echo "== helper definitions =="
rg -n -C 6 'fn resolve_version|fn resolve_allow_code|fn bytes_like|marshal_to_bytes|unmarshal_bytes' pyre/pyre-interpreter/src/module/marshal/mod.rs
echo "== pyre_function call sites around marshal =="
rg -n -C 8 'pyre_function|make_module_builtin_function_with_arity_and_maybe_sig|inline_function' pyre/pyre-interpreter/src/module/marshal/mod.rs pyre/pyre-interpreter/src/gateway.rs
echo "== compare builtin rooting patterns =="
rg -n -C 8 'fn (loads|dump|dumps|load)' pyre/pyre-interpreter/src/module -g '*.rs' | sed -n '1,220p'Repository: youknowone/pyre
Length of output: 37720
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== gateway macro implementation =="
rg -n -C 12 'macro_rules! py_module|inline_functions|BuiltinCodeFn\(' pyre/pyre-interpreter/src/gateway.rs
echo "== gc_roots pin/push/get definitions =="
rg -n -C 6 'fn push_roots|fn pin_root|fn shadow_stack_get|fn shadow_stack_len|push_root' pyre/pyre-interpreter/src -g '*.rs'
echo "== marshal helper implementations =="
sed -n '36,70p' pyre/pyre-interpreter/src/module/marshal/mod.rs
sed -n '390,460p' pyre/pyre-interpreter/src/module/marshal/mod.rs
echo "== focused comparison builtin pickle dump/loads code =="
sed -n '730,785p' pyre/pyre-interpreter/src/module/_pickle/mod.rsRepository: youknowone/pyre
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== marshal macro exact section =="
rg -n -C 20 'inline_functions:' pyre/pyre-interpreter/src/gateway.rs pyre/pyre-interpreter/src -g '*.rs' | sed -n '1,260p'
echo "== gc_roots small implementation =="
fd -t f 'gc_roots.rs' pyre/pyre-interpreter/src -x sh -c 'echo "--- $1"; wc -l "$1"; rg -n -C 8 "pub fn (push_roots|pin_root|shadow_stack_get|shadow_stack_len)|struct .*RootScope|impl .*RootScope" "$1"; sed -n "1,220p"' sh {}
echo "== marshal public helper bodies =="
sed -n '1,50p' pyre/pyre-interpreter/src/module/marshal/mod.rs
sed -n '424,442p' pyre/pyre-interpreter/src/module/marshal/mod.rsRepository: youknowone/pyre
Length of output: 24832
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pyre_function definition and generated arity/sig symbols =="
rg -n -C 12 'pub fn create_builtins|pyre_function|struct BuiltinCode|BuiltinCodeFn|fn .*_pyre_arity|fn .*_pyre_sig|push_roots|pin.*root' pyre/pyre-interpreter/src/lib.rs pyre/pyre-interpreter/src/gateway.rs | sed -n '1,260p'
echo "== gc_roots implementation via precise symbol search =="
rg -n 'pub (struct|fn) (RootScope|push_roots|pin_root|shadow_stack_get|shadow_stack_len|pin_roots)|pin_roots|shadow_stack_len' pyre/pyre-interpreter/src -g '*.rs' | sed -n '1,220p'
echo "== pyre_function symbol occurrences =="
rg -n -C 8 '#\[pyre_function\]|macro_rules! pyre_function|fn pyre_function|create_builtins|pyre_function' pyre/pyre-interpreter/src/lib.rs pyre/pyre-interpreter/src/gateway.rs | sed -n '1,300p'Repository: youknowone/pyre
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pyre_function macro from pyre_macros =="
rg -n -C 10 'pub fn pyre|create.*builtins.*pyre|let name|shadow_stack_len|pin_root|push_roots|PyreFunction|BuiltinCodeFn|kwonly|posonly' pyre/pyre-macros/src pyre/pyre-object/src -g '*.rs' | sed -n '1,360p'
echo "== gc_roots exact implementation snippets =="
rg -n -A 18 'pub fn push_roots|pub fn pin_root|pub fn shadow_stack_get|pub fn shadow_stack_len|pub fn pin_roots|shadow_stack' pyre/pyre-object/src -g '*.rs' | sed -n '1,300p'
echo "== pyre_macros macro source files =="
find pyre/pyre-macros/src -maxdepth 2 -type f -print | sort
for f in $(find pyre/pyre-macros/src -maxdepth 2 -type f -print | sort); do echo "--- $f"; wc -l "$f"; done
rg -n -C 6 '#\[proc_macro_attribute\]|proc_macro|quote!|BuiltinCodeFn|args:|pin_root|shadow_stack_get' pyre/pyre-macros/src -g '*.rs' | sed -n '1,340p'Repository: youknowone/pyre
Length of output: 50371
Root marshal argument values before option resolution. #[pyre_function] generated wrappers pass PyObjectRef arguments into the inline wrappers, but those values are raw locals until the wrappers themselves pin them. resolve_version and resolve_allow_code can call into Python and move GC objects, so marshal.dumps, marshal.loads, marshal.dump, and marshal.load each need pin_root plus shadow_stack_get for live arguments used after resolving options.
📍 Affects 1 file
pyre/pyre-interpreter/src/module/marshal/mod.rs#L530-L541(this comment)pyre/pyre-interpreter/src/module/marshal/mod.rs#L544-L553pyre/pyre-interpreter/src/module/marshal/mod.rs#L557-L571pyre/pyre-interpreter/src/module/marshal/mod.rs#L575-L593
🤖 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/marshal/mod.rs` around lines 530 - 541, The
marshal wrappers must root live PyObjectRef arguments before option resolution
can invoke Python. In pyre/pyre-interpreter/src/module/marshal/mod.rs:530-541
(marshal.dumps), 544-553, 557-571, and 575-593, add pin_root and
shadow_stack_get handling for every argument used after resolve_version or
resolve_allow_code, then pass the rooted values into marshal_to_bytes or the
corresponding load/dump operation while preserving existing behavior.
| let mut reader = wire::Cursor { | ||
| data: data.as_slice(), | ||
| position: 0, | ||
| }; | ||
| let result = | ||
| wire::deserialize_value(&mut reader, PyreMarshalBag).map_err(marshal_error)?; | ||
| let new_position = w_int_new(before.saturating_add(reader.position as i64)); | ||
| call_method(file, "seek", &[new_position])?; | ||
| let result = result.get(); | ||
| if !allow_code { | ||
| reject_code(result)?; | ||
| } | ||
| Ok(result) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Reuse unmarshal_bytes in load.
The deserialize and reject_code body here repeats unmarshal_bytes. The only extra requirement is the consumed byte count. Return the consumed length from unmarshal_bytes and call it from load, so the allow_code rejection rule stays defined in one place.
🤖 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/marshal/mod.rs` around lines 586 - 598,
Refactor load to call unmarshal_bytes instead of duplicating wire
deserialization and reject_code handling. Update unmarshal_bytes to return both
the deserialized value and consumed byte count, then use that count with before
and seek the file accordingly while preserving the allow_code behavior in
unmarshal_bytes.
| fn pack(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | ||
| if args[0].is_null() { | ||
| return Err(crate::PyError::type_error("missing format argument")); | ||
| } | ||
| let fmt = format_to_string(args[0])?; | ||
| let values = unsafe { w_tuple_items_copy_as_vec(args[1]) }; | ||
| do_pack(&fmt, &values) | ||
| } | ||
|
|
||
| /// `interp_struct.py:76 pack_into(space, w_format, w_buffer, offset, args_w)`. | ||
| /// Bound slice `[format, buffer, offset, (*args tuple)]`. As with [`pack`], | ||
| /// the three required positional slots are `PY_NULL`-checked here. | ||
| fn pack_into(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> { | ||
| if args[0].is_null() || args[1].is_null() || args[2].is_null() { | ||
| return Err(crate::PyError::type_error( | ||
| "pack_into() missing format, buffer or offset argument", | ||
| )); | ||
| } | ||
| let fmt = format_to_string(args[0])?; | ||
| let offset = unsafe { crate::builtins::space_index_w(args[2])? }; | ||
| let values = unsafe { w_tuple_items_copy_as_vec(args[3]) }; | ||
| do_pack_into(&fmt, args[1], offset, &values) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Align the missing-argument messages with the gateway arity diagnostics.
Both bodies raise hand-written text. pack omits the function name that the neighboring messages include, and neither message matches the arity error PyPy's gateway produces for an omitted positional slot. Emit the same phrasing the binder uses for a missing required argument, or name the count explicitly.
♻️ Proposed message alignment
fn pack(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyError> {
if args[0].is_null() {
- return Err(crate::PyError::type_error("missing format argument"));
+ return Err(crate::PyError::type_error(
+ "pack() missing required argument 'format' (pos 1)",
+ ));
}As per coding guidelines: "Port RPython/PyPy code with 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/struct/mod.rs` around lines 1253 - 1275,
Update the missing-argument errors in pack and pack_into to use the gateway’s
standard phrasing for omitted required positional arguments, including the
function name and explicit missing-argument count where applicable. Keep the
existing null checks and error type unchanged, and preserve strict structural
parity with the neighboring gateway diagnostics.
Source: Coding guidelines
| /// Coerce an argument slot to an integer, `None`/absent (`PY_NULL`) → default. | ||
| /// | ||
| /// The Signature-bound call path fills every declared slot: a supplied | ||
| /// keyword / positional carries the value, an omitted optional carries | ||
| /// `PY_NULL`. | ||
| fn int_or_default(o: PyObjectRef, default: i64) -> Result<i64, crate::PyError> { | ||
| if o.is_null() || unsafe { is_none(o) } { | ||
| Ok(default) | ||
| } else { | ||
| crate::baseobjspace::int_w(o) | ||
| } | ||
| } | ||
|
|
||
| /// Fetch an optional `zdict` bytes argument by keyword or position. | ||
| fn arg_zdict( | ||
| pos: &[PyObjectRef], | ||
| kwargs: Option<PyObjectRef>, | ||
| index: usize, | ||
| ) -> Result<Option<Vec<u8>>, crate::PyError> { | ||
| match crate::builtins::kwarg_get(kwargs, "zdict").or_else(|| pos.get(index).copied()) { | ||
| Some(o) if !unsafe { is_none(o) } => Ok(Some(as_bytes(o)?)), | ||
| _ => Ok(None), | ||
| /// Coerce an optional `zdict` slot to bytes, `None`/absent (`PY_NULL`) → None. | ||
| fn zdict_or_none(o: PyObjectRef) -> Result<Option<Vec<u8>>, crate::PyError> { | ||
| if o.is_null() || unsafe { is_none(o) } { | ||
| Ok(None) | ||
| } else { | ||
| Ok(Some(as_bytes(o)?)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the macro's default-slot handling to confirm an omitted optional can stay PY_NULL.
rg -nP -C6 'posonly|default' pyre/pyre-macros/src/lib.rs | head -120
# Compare with the PyPy declarations these handlers port.
fd -a 'interp_zlib.py' | xargs -r rg -nP -C4 'unwrap_spec|def (compress|decompress|Compress___new__|Decompress___new__)'Repository: youknowone/pyre
Length of output: 9081
🏁 Script executed:
#!/bin/bash
# Inspect the Ruby/zlib implementation and CPython source/docs for zdict handling.
fd -a 'rzlib.*' -t f | xargs -r rg -nP -C8 'w_zdict|unpack|zdict|unpack'
python3 - <<'PY'
try:
import zlib
for name, args in [
("compress level=None", [b"x"], {"level": None}),
("decompress wbits=None", [b"x"], {"wbits": None}),
("compressobj zdict=None", [], {"zdict": None}),
("compressobj zdict=empty", [], {"zdict": b""}),
]:
try:
res = zlib.compress(*(args[:1]), **args[1:]) if name.startswith("compress") else zlib.decompress(*(args[:1]), **args[1:]) if name.startswith("decompress") else zlib.compressobj(**args[1:])
print(name, "OK", type(res).__name__ if callable(res) else "<bytes>")
except Exception as e:
print(name, type(e).__name__, e)
except Exception as e:
print("zlib unavailable:", e)
PYRepository: youknowone/pyre
Length of output: 13941
🏁 Script executed:
#!/bin/bash
# Inspect PyPy's w_zdict type/implementation and the exact Python zlib C signature for context.
python3 - <<'PY'
try:
import zlib, inspect
for name, func in [
("compress", zlib.compress),
("decompress", zlib.decompress),
("compressobj", zlib.compressobj),
("Decompress.__init__", zlib.Decompress.__init__),
]:
try:
print(name, inspect.signature(func))
except Exception as e:
print(name, "SIG_ERROR", e)
except Exception as e:
print("zlib unavailable:", type(e).__name__, e)
PY
# Search for PyPy w_zdict implementation.
rg -nP -C8 'w_zdict|zdict' pypy/ pypy-interpreter/ --glob '*.py' --glob '*.rs' || trueRepository: youknowone/pyre
Length of output: 14442
🏁 Script executed:
#!/bin/bash
# Inspect the target zlib Rust code and the macro's generated defaults/unwrap behavior.
wc -l pyre/pyre-interpreter/src/module/zlib/mod.rs
sed -n '1,130p' pyre/pyre-interpreter/src/module/zlib/mod.rs
sed -n '530,555p' pyre/pyre-interpreter/src/module/zlib/mod.rs
rg -n -C4 'int_or_default|zdict_or_none|compressobj.*python_func|deflateInit' pyre/pyre-interpreter/src/module/zlib/mod.rs
sed -n '360,395p' pyre/pyre-macros/src/lib.rsRepository: youknowone/pyre
Length of output: 11093
Distinguish None from omitted optional arguments in this path.
unpack_kwargs_from passes supplied signature slots unchanged, so level=None, wbits=None, and zdict=None are not PY_NULL. Current helpers turn explicit None into the function default, while PyPy ports w_zdict=None and the documented integer default slots through unwrap_spec(...) converters that accept None. Drop the is_none branch in int_or_default and zdict_or_none, and keep declared omitted optionals as PY_NULL; otherwise compress(level=None), decompress(wbits=None), and compressobj(zdict=None) bypass upstream None handling.
🤖 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/zlib/mod.rs` around lines 84 - 104, Update
int_or_default and zdict_or_none to treat only PY_NULL/absent slots as omitted;
remove the is_none checks so explicit None is passed through to the existing
integer and byte-conversion handling. Preserve PY_NULL defaults for omitted
optional arguments, allowing compress, decompress, and compressobj to retain
their upstream None behavior.
Source: Coding guidelines
| let level = int_or_default(level, -1)? as i32; | ||
| let wbits = to_wbits(int_or_default(wbits, backend::MAX_WBITS as i64)?); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
as i32 truncates an out-of-range level instead of raising.
int_or_default returns i64. The as i32 cast wraps silently, so zlib.compress(b"x", level=1 << 32) yields level 0 and succeeds, and zlib.compressobj(level=1 << 32) behaves the same way. CPython and PyPy raise OverflowError for a value that does not fit a C int. Range-check the value before the cast.
🐛 Proposed fix for the level range check
+/// Coerce an argument slot to a C `int` level, rejecting out-of-range values.
+fn level_or_default(o: PyObjectRef, default: i64) -> Result<i32, crate::PyError> {
+ let v = int_or_default(o, default)?;
+ i32::try_from(v).map_err(|_| {
+ crate::PyError::overflow_error("Python int too large to convert to C int")
+ })
+}Then use it at both call sites:
- let level = int_or_default(level, -1)? as i32;
+ let level = level_or_default(level, -1)?;Also applies to: 546-547
🤖 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/zlib/mod.rs` around lines 510 - 511, Replace
the unchecked i64-to-i32 cast for level with a checked conversion that raises
OverflowError when the value is outside the C int range. Apply the same range
validation at both level call sites in the compress and compressobj paths, while
preserving the existing default handling.
expand_pyre_function recognized #[kwonly] and #[kwargs] but had no way to mark a positional-only boundary, so a signature-driven module function could not express positional-only leading parameters. A #[posonly] attribute on the first non-positional-only parameter now emits SignatureBuilder::marker_posonly before that parameter's append, symmetric to the #[kwonly] handling and emitted before it (the builder asserts posonly precedes kwonly). The marker attribute is stripped from the generated inner fn. marker_posonly mirrors gateway.py visit_posonly (the __posonly__=None sentinel). macro_smoke.rs gains _posonly_bound_probe and a test asserting the derived Signature carries posonlyargcount == 1, that a keyword still binds the non-positional-only tail, and that passing the positional-only name as a keyword raises TypeError. Assisted-by: Claude
…compressor off the marker ABI The four module functions moved from the functions: arm (registered as varargs `/ *` closures with a null Signature) to inline_functions:, so the #[pyre_function] expansion derives a Signature and the call path binds keywords into positional PY_NULL-padded slots before the wrapper runs. data / string are positional-only via #[posonly] (interp_zlib.py:66,92). compressobj now declares all six parameters level/method/wbits/memLevel/strategy/zdict (interp_zlib.py:228); method/memLevel/strategy stay accepted-and-ignored (Compressor::new threads only level/wbits/zdict) but are now keyword-bindable, replacing the hardcoded positional indices that skipped them. _ZlibDecompressor.__new__ and .decompress become named functions registered through make_new_descr_maybe_sig / make_builtin_function_maybe_sig with a hand-built Signature (cls / self positional-only via marker_posonly). arg_int / arg_zdict, which peeled the marker dict and resolved by keyword-or-position, are replaced by int_or_default / zdict_or_none that read a filled Signature slot (value, or PY_NULL for an omitted optional). crc32 / adler32 keep their varargs closures (no keyword API). This removes all six split_builtin_kwargs peels in the module. Pre-existing divergences left untouched: int_or_default accepts level=None as the default, and method/memLevel/strategy remain ignored. Assisted-by: Claude
Register b2a_hex/hexlify, a2b_qp/b2a_qp, and a2b_base64/b2a_base64/b2a_uu through the inline_functions: arm so #[pyre_function] derives their Signature and the caller binds keywords into positional slots. Register the all-positional-only crc32/crc_hqx through a hand-built SignatureBuilder (marker_posonly after every append) in extra_init:, since #[posonly] on a parameter cannot mark a run with no trailing parameter. Both take HOPELESS arity so the positional path routes through the binder rather than the fixed-arity fast entry. The bodies read PY_NULL-padded positional slots instead of peeling split_builtin_kwargs / bind_builtin_kwargs; the two helpers and the marker producer stay for the remaining null-sig callers. crc32/crc_hqx report keyword and arity errors under the bare function name (crc32() ...) the way funcrun_obj names an interp2app function, dropping the manual binascii. prefix. a2b_base64's strict_mode stays keyword-only, unchanged; interp_base64.py marks no __kwonly__ there, so upstream keeps it positional-or-keyword. Assisted-by: Claude
…he marker ABI
Convert all five `__pyre_kw__` marker peels in `_struct` to the Signature
gateway. `unpack_from` moves to the `inline_functions:` arm (`format`
positional-only via `#[posonly]` on `buffer`, `offset` an `Option` so an
omitted slot is 0 while an explicit `offset=None` raises). `pack`, `pack_into`,
and the `Struct.pack_into` method are `*args` builtins: their `&[PyObjectRef]`
bodies are registered by hand with a `varargname`-bearing `Signature`
(`sig_posonly_then_varargs`), so the binder packs the excess positionals into a
tuple the body reads via `w_tuple_items_copy_as_vec`. `Struct.unpack_from`
converts through the `#[pyre_methods]` arm with typed `buffer`/`offset` params;
`Struct.pack_into` is registered into the type dict from `extra_init:` because
the method arm emits no Signature for a varargs method. `resolve_buffer_offset`
is deleted (its two callers now bind by Signature).
`pack_into` / `Struct.pack_into` previously discarded stray keywords silently;
the all-positional-only Signature now rejects them ("takes no keyword
arguments"), matching the reference. The keyword-rejection error name is the
bare function name from the Signature path rather than the module-qualified
form the hand-written peel spelled.
The stdlib_struct snippet gains positive keyword `unpack_from` calls and
keyword-rejection assertions for pack/pack_into.
Assisted-by: Claude
Move the four keyword-accepting marshal functions to the `inline_functions:` arm, so the macro derives each `Signature` and the gateway binds keywords into PY_NULL-padded slots before the body runs, replacing the four `split_builtin_kwargs` peels. `value`/`version` are positional-only and `allow_code` keyword-only (`dumps(value, version, /, *, allow_code=True)`); the `#[posonly] #[kwonly]` markers stack on `allow_code` so `marker_posonly()` fires after every positional param is appended. `version`/`allow_code` are `Option` so an omitted slot takes the default while an explicit `None` still reaches `int_w`/`is_true`. `dump`/`dumps` now reject the `version=` keyword the marker path accepted through `parse_version`. `marshal_to_bytes`/`unmarshal_bytes` hold the shared serialize/deserialize bodies; `dumps_bytes`/`loads_bytes` delegate to them. Assisted-by: Claude
struct `unpack_from` (module and method): coerce `offset` through `space_index_w` before `readbuf` borrows the buffer's backing store as a raw slice, so an `__index__` that resizes or frees that store cannot leave the body reading freed memory. `Struct.pack_into` guards a PY_NULL `buffer` / `offset` before conversion, reporting the missing argument instead of a "cannot be interpreted as an integer" error. binascii `slot_bool` returns `Result` and propagates `is_true` errors rather than swallowing a raising `__bool__` / `__len__` with the default; a2b_qp, b2a_qp, a2b_base64, b2a_base64, and b2a_uu take the result with `?`. `slot_u32` and `crc_hqx` read their CRC seed through `truncatedint_w` (matching `oldcrc='truncatedint_w'`), truncating a wide integer to 32 bits instead of raising OverflowError; an omitted slot still defaults, but an explicit `None` now raises. zlib decompress (`Decompress` and `_ZlibDecompressor`): an explicit `None` `max_length` goes through `int_w` and raises instead of being treated as the omitted-slot default. Assisted-by: Claude
builtin_code_call assumes an already-bound flat slice; its direct callers pass the output of bind_kwargs_to_signature. The JIT residual-call path (bh_call_fn_impl) instead handed it raw positionals, so a varargname-bearing Signature builtin — _struct pack/pack_into and Struct.pack_into, whose bodies read a packed *args tuple slot via w_tuple_items_copy_as_vec — received the unbound arguments and read a non-tuple as a tuple, overrunning w_tuple_getitem_known (SIGSEGV under JIT; test_pickle/test_pickletools depend on struct.pack). Rename finish_builtin_code_positional to builtin_code_call_positional, make it pub, and route pyre-jit's residual builtin dispatch through it: it binds a HOPELESS-arity Signature through _match_signature before invoking the body, the same as the frame dispatch already did. builtin_code_call still never binds, so the already-bound direct call sites are unchanged. Assisted-by: Claude
d20e34d to
fe5d0d8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe5d0d8df8
ℹ️ 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 match pyre_interpreter::call::builtin_code_call_positional( | ||
| code as pyre_object::PyObjectRef, | ||
| &call_args, |
There was a problem hiding this comment.
When the blackhole residual path calls a signature-bearing variadic builtin such as the newly registered _struct.pack, builtin_code_call_positional invokes bind_kwargs_to_signature, whose w_tuple_new can trigger the moving collector. The code passed here was copied from the rooted callable before that allocation but is not itself pinned or reloaded, so the subsequent builtin_code_call(current_code, ...) may dereference its evacuated address. The interpreter path explicitly pins code before the same helper; preserve that rooting/reload behavior here (or inside the helper) as well.
AGENTS.md reference: AGENTS.md:L14-L20
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-jit/src/call_jit.rs`:
- Around line 5096-5103: Update the dispatch branch around
builtin_code_call_positional to root or pin code before signature binding can
allocate GC objects, then reload the rooted code immediately before invoking the
positional builtin. Pass this reloaded pointer to the dispatch call instead of
the original pre-binding code pointer.
🪄 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: ccde6fee-11c8-45ca-b0e4-9f8fe7427633
📒 Files selected for processing (2)
pyre/pyre-interpreter/src/call.rspyre/pyre-jit/src/call_jit.rs
| // `call_args` are raw positionals; a HOPELESS-arity Signature | ||
| // (`*args`, optional positional) needs `_match_signature` binding | ||
| // before the body reads its slots. `builtin_code_call` never binds, | ||
| // so route through the positional entry that does. | ||
| return match pyre_interpreter::call::builtin_code_call_positional( | ||
| code as pyre_object::PyObjectRef, | ||
| &call_args, | ||
| ) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 \
'fn bind_kwargs_to_signature|bind_kwargs_to_signature\(|builtin_code_call_positional\(|pin_root\(code|shadow_stack_get\(code_slot' \
pyreRepository: youknowone/pyre
Length of output: 46749
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- JIT caller context ---'
sed -n '5000,5135p' pyre/pyre-jit/src/call_jit.rs
printf '%s\n' '--- positional gateway and binder ---'
sed -n '900,980p' pyre/pyre-interpreter/src/call.rs
sed -n '2227,2520p' pyre/pyre-interpreter/src/call.rs
printf '%s\n' '--- code/function representation and accessors ---'
rg -n -C 10 \
'fn getcode|pub .*getcode|struct BuiltinCode|BuiltinCode.*malloc|builtin_code_get_signature|builtin_code_call_positional' \
pyre/pyre-interpreter pyre/pyre-object pyre/pyre-jitRepository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- BuiltinCode allocation and function code field ---'
sed -n '270,330p' pyre/pyre-interpreter/src/gateway.rs
sed -n '590,635p' pyre/pyre-interpreter/src/gateway.rs
sed -n '1098,1145p' pyre/pyre-interpreter/src/function.rs
printf '%s\n' '--- root setup and reload_args definition ---'
rg -n -C 18 \
'fn reload_args|let root_base|push_roots\(\)|pin_root\(.*callable|pin_root\(.*arg' \
pyre/pyre-jit/src/call_jit.rs
printf '%s\n' '--- binder allocation calls ---'
python3 - <<'PY'
from pathlib import Path
p = Path("pyre/pyre-interpreter/src/call.rs")
text = p.read_text()
start = text.index("pub(crate) fn bind_kwargs_to_signature")
end = text.index("\n/// [`call_with_kwargs_in_ctx`]", start)
body = text[start:end]
for i, line in enumerate(body.splitlines(), 1):
if any(token in line for token in (
"vec![", "Vec::", "w_tuple_new", "w_dict_new_kwargs",
"w_str_from_wtf8_managed", "w_dict_store", "push_roots",
"pin_root", "builtin_code_call",
)):
print(f"{start + body[:sum(len(x)+1 for x in body.splitlines()[:i-1])].count(chr(10)) + 1}: {line}")
PYRepository: youknowone/pyre
Length of output: 16474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 16 \
'pub fn builtin_code_new|fn builtin_code_new|builtin_code_new_with_signature|malloc_typed|BUILTIN_CODE_TYPE|BuiltinCode.*GC' \
pyre/pyre-interpreter/src/gateway.rs pyre/pyre-interpreter/src/function.rs pyre
printf '%s\n' '--- direct JIT builtin dispatch and root slot arithmetic ---'
sed -n '4946,4975p' pyre/pyre-jit/src/call_jit.rs
sed -n '5052,5112p' pyre/pyre-jit/src/call_jit.rsRepository: youknowone/pyre
Length of output: 50372
Verify the code root before signature binding.
bind_kwargs_to_signature can allocate GC objects when it packs *args or **kwargs. Pin code before calling builtin_code_call_positional, then reload it from the root immediately before dispatch. The current branch passes the pre-binding pointer to builtin_code_call.
🤖 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-jit/src/call_jit.rs` around lines 5096 - 5103, Update the dispatch
branch around builtin_code_call_positional to root or pin code before signature
binding can allocate GC objects, then reload the rooted code immediately before
invoking the positional builtin. Pass this reloaded pointer to the dispatch call
instead of the original pre-binding code pointer.
1 participant
Ports three modules' keyword-accepting builtins from the
__pyre_kw__marker-dict ABI to the PyPySignaturegateway. The caller binds keywords into PY_NULL-padded positional slots before the wrapper runs, and each body reads slots positionally instead of peelingsplit_builtin_kwargs/bind_builtin_kwargs. The marker producer and both helpers stay for the remaining null-sig callers.Commits
#[posonly]marker to#[pyre_function]— a#[posonly]attribute on the first non-positional-only parameter ends the positional-only run before it (the twin of#[kwonly]), emittingmarker_posonly(). Mirrors PyPyvisit_posonly/__posonly__=None._ZlibDecompressoroff the marker ABI — module functions moved to theinline_functions:arm (macro derives theSignature);_ZlibDecompressor.__new__/.decompressregistered with a hand-builtSignatureBuilder.compressobjnow declares all six parameters (was readingwbits/zdictat hardcoded positions, skipping method/memLevel/strategy).b2a_hex/hexlify,a2b_qp/b2a_qp, anda2b_base64/b2a_base64/b2a_uuviainline_functions:; the all-positional-onlycrc32/crc_hqxvia a hand-builtSignatureBuilderinextra_init:(bothHOPELESSarity so the positional path routes through the binder).crc32/crc_hqxnow report keyword/arity errors under the bare function name, matching howfuncrun_objnames an interp2app function.unpack_from(module + method) moved to signature-driven params (#[posonly]onbuffer,offsetoptional); the three varargs functionspack/pack_into/Struct.pack_intoregistered with a hand-built posonly-then-varargsSignatureBuilder, reading the packed*argstuple tail from the last bound slot.pack_into/Struct.pack_intonow reject stray keywords (takes no keyword arguments) instead of silently discarding them.Verification
cargo test --all --no-default-features --features dynasm— green (7531/0)python3 pyre/check.py— bit-exact 3-backend, 0 mismatch except one base-owned red onsynth/pickle_terminal_raise_resume(a pre-existing JIT resume layout-lottery, reproduces on the parent base withPYRE_JIT=1and is clean underPYRE_JIT=0; not introduced by this set)cargo fmt --all -- --check— clean🤖 Generated with Claude Code
Summary by CodeRabbit
Enhancements
binascii,marshal,struct, andzlib.Tests