Align all remaining error messages with CPython - #7993
Conversation
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_builtin.py (TODO: 14) dependencies: dependent tests: (no tests depend on builtin) [x] test: cpython/Lib/test/test_tstring.py (TODO: 2) dependencies: dependent tests: (no tests depend on tstring) [ ] test: cpython/Lib/test/test_bytes.py (TODO: 16) dependencies: dependent tests: (no tests depend on bytes) [ ] lib: cpython/Lib/json dependencies:
dependent tests: (13 tests)
[ ] test: cpython/Lib/test/test_syntax.py (TODO: 2) dependencies: dependent tests: (no tests depend on syntax) [ ] test: cpython/Lib/test/test_str.py (TODO: 5) dependencies: dependent tests: (no tests depend on str) [ ] test: cpython/Lib/test/test_memoryview.py (TODO: 6) dependencies: dependent tests: (no tests depend on memoryview) [x] lib: cpython/Lib/hashlib.py dependencies:
dependent tests: (145 tests)
[x] test: cpython/Lib/test/test_format.py dependencies: dependent tests: (no tests depend on format) [ ] test: cpython/Lib/test/test_structseq.py dependencies: dependent tests: (no tests depend on structseq) [x] lib: cpython/Lib/lzma.py dependencies:
dependent tests: (101 tests)
[ ] test: cpython/Lib/test/test_descr.py (TODO: 31) dependencies: dependent tests: (no tests depend on descr) [x] test: cpython/Lib/test/test_coroutines.py (TODO: 14) dependencies: dependent tests: (7 tests) [x] lib: cpython/Lib/plistlib.py dependencies:
dependent tests: (25 tests)
[x] lib: cpython/Lib/ast.py dependencies:
dependent tests: (149 tests)
[x] lib: cpython/Lib/datetime.py dependencies:
dependent tests: (67 tests)
[ ] test: cpython/Lib/test/test_posix.py (TODO: 3) dependencies: dependent tests: (101 tests)
[x] lib: cpython/Lib/pdb.py dependencies:
dependent tests: (1 tests)
[x] test: cpython/Lib/test/test_asyncgen.py (TODO: 3) dependencies: dependent tests: (no tests depend on asyncgen) [x] test: cpython/Lib/test/test_marshal.py (TODO: 4) dependencies: dependent tests: (25 tests)
[ ] lib: cpython/Lib/socket.py dependencies:
dependent tests: (101 tests)
[ ] lib: cpython/Lib/sqlite3 dependencies:
dependent tests: (2 tests)
[x] lib: cpython/Lib/threading.py dependencies:
dependent tests: (163 tests)
[ ] test: cpython/Lib/test/test_extcall.py (TODO: 7) dependencies: dependent tests: (no tests depend on extcall) [x] test: cpython/Lib/test/test_range.py (TODO: 2) dependencies: dependent tests: (no tests depend on range) [ ] test: cpython/Lib/test/test_exceptions.py (TODO: 21) dependencies: dependent tests: (no tests depend on exception) [x] lib: cpython/Lib/bz2.py dependencies:
dependent tests: (101 tests)
[x] lib: cpython/Lib/enum.py dependencies:
dependent tests: (16 tests)
[ ] test: cpython/Lib/test/test_mmap.py (TODO: 24) dependencies: dependent tests: (2 tests)
[ ] test: cpython/Lib/test/test_generators.py (TODO: 7) dependencies: dependent tests: (no tests depend on generator) [ ] test: cpython/Lib/test/test_class.py (TODO: 12) dependencies: dependent tests: (no tests depend on class) [ ] test: cpython/Lib/test/test_dict.py (TODO: 4) dependencies: dependent tests: (no tests depend on dict) Legend:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTightens symbol validation (rejecting ChangesSymbol validation and parse error alignment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/codegen/src/compile.rs (1)
4718-4729: ⚡ Quick winExtract duplicated type parameter validation into a helper method.
The type parameter
__debug__validation logic is duplicated identically in both function and class compilation paths. Extract this into a shared helper to improve maintainability and prevent future divergence.♻️ Proposed refactor to eliminate duplication
Add a helper method to the compiler struct:
fn validate_type_params_no_debug(&self, type_params: Option<&ast::TypeParams>) -> CompileResult<()> { if let Some(params) = type_params { for tp in ¶ms.type_params { let tp_name = match tp { ast::TypeParam::TypeVar(t) => &t.name, ast::TypeParam::TypeVarTuple(t) => &t.name, ast::TypeParam::ParamSpec(t) => &t.name, }; if tp_name.as_str() == "__debug__" { return Err(self.error(CodegenErrorType::Assign("__debug__"))); } } } Ok(()) }Then replace the duplicated blocks with a single call in each location:
- // Reject `def f[__debug__](): ...` type parameter (mirrors class defs). - if let Some(params) = type_params { - for tp in ¶ms.type_params { - let tp_name = match tp { - ast::TypeParam::TypeVar(t) => &t.name, - ast::TypeParam::TypeVarTuple(t) => &t.name, - ast::TypeParam::ParamSpec(t) => &t.name, - }; - if tp_name.as_str() == "__debug__" { - return Err(self.error(CodegenErrorType::Assign("__debug__"))); - } - } - } + // Reject `def f[__debug__](): ...` type parameter (mirrors class defs). + self.validate_type_params_no_debug(type_params)?;Apply the same simplification in the class compilation path (lines 5307-5318).
Also applies to: 5307-5318
🤖 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 `@crates/codegen/src/compile.rs` around lines 4718 - 4729, Extract the duplicated "__debug__" type-parameter check into a new helper on the compiler impl (e.g. fn validate_type_params_no_debug(&self, type_params: Option<&ast::TypeParams>) -> CompileResult<()>) that iterates params.type_params, extracts the name from ast::TypeParam variants, and returns Err(self.error(CodegenErrorType::Assign("__debug__"))) if a name equals "__debug__", otherwise Ok(()). Replace the duplicated validation blocks in the function compilation path and the class compilation path with a single call to this helper, keeping existing types (ast::TypeParams, CompileResult) and error construction unchanged.
🤖 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 `@crates/compiler/src/lib.rs`:
- Around line 986-1025: The helper parenthesized_param_message currently only
inspects the current physical line, missing multiline cases; update it to scan
backwards from line_start through previous lines to find a preceding "def " or
"lambda" token and then evaluate parenthesis depth across that full span.
Specifically, for "def " (use def_idx search but on the concatenated slice from
the found def position to the error start) walk characters from that def
position counting '(' and ')' to determine if the error position is inside the
function parameter list (return the CPython message if inside), and for "lambda"
search backwards similarly to find a lambda token without a ':' between it and
the error (consider spanning newlines). Keep the existing return messages and
reuse parenthesized_param_message, but replace single-line checks with these
backward-scanning, depth-aware checks so multiline parameter lists are detected.
- Around line 1397-1419: chunk_has_bare_assignment currently treats any
alphanumeric LHS as an identifier; update it to reject literals/keywords by
first checking the trimmed lhs start and exact content: ensure lhs is non-empty
and its first byte is ASCII alphabetic or b'_' (reject if it starts with a
digit, quote, '(' '[' '{', etc.), then ensure the whole lhs matches the
identifier shape (bytes().all(...) as you already do) and is not one of the
Python literal/keyword tokens like "True", "False", or "None" (use a small
static set and compare lhs.eq_ignore_ascii_case or exact match as appropriate).
Return false for those cases so chunk_has_bare_assignment only returns true for
real identifiers.
- Around line 1682-1698: The current is_in_case_pattern only checks the current
line for a "case " header so it misses multi-line case headers; change the logic
to scan backwards from range.start() to find the nearest preceding non-empty
line whose trimmed start begins with "case " (use the existing start and
line_start variables as anchors) and treat the location as being in a case
pattern if such a "case " header exists with indentation (case_indent) less than
the current line and there is no '=' token between the header start and the
range.start(); update the code around is_in_case_pattern, keeping references to
start, line_start, rest, and case_indent, to iterate previous lines (instead of
only checking source[line_start..]) and ensure multiline headers are recognized
before falling back to the match-search logic.
---
Nitpick comments:
In `@crates/codegen/src/compile.rs`:
- Around line 4718-4729: Extract the duplicated "__debug__" type-parameter check
into a new helper on the compiler impl (e.g. fn
validate_type_params_no_debug(&self, type_params: Option<&ast::TypeParams>) ->
CompileResult<()>) that iterates params.type_params, extracts the name from
ast::TypeParam variants, and returns
Err(self.error(CodegenErrorType::Assign("__debug__"))) if a name equals
"__debug__", otherwise Ok(()). Replace the duplicated validation blocks in the
function compilation path and the class compilation path with a single call to
this helper, keeping existing types (ast::TypeParams, CompileResult) and error
construction unchanged.
🪄 Autofix (Beta)
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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c64ba8a6-f09d-45ff-8156-2a8997556f5b
⛔ Files ignored due to path filters (4)
Lib/test/test_genexps.pyis excluded by!Lib/**Lib/test/test_named_expressions.pyis excluded by!Lib/**Lib/test/test_patma.pyis excluded by!Lib/**Lib/test/test_syntax.pyis excluded by!Lib/**
📒 Files selected for processing (5)
crates/codegen/src/compile.rscrates/codegen/src/symboltable.rscrates/compiler/src/lib.rscrates/vm/src/stdlib/_ast.rscrates/vm/src/vm/vm_new.rs
4383c21 to
a84b1a4
Compare
|
@coderabbitai review The duplicated |
|
✅ Actions performedReview triggered.
|
ShaharNaveh
left a comment
There was a problem hiding this comment.
tysm for working on this!
On one hand I can't argue with the value this brings, and on the other it feels a bit ridiculous because we are just reimplementing the lexer & parser atp.
I'll wait for @youknowone inputs
05843ed to
19195ed
Compare
|
@ShaharNaveh @youknowone is this okay to merge? |
19195ed to
669fc00
Compare
|
I am sorry about late review of this. To be honest, I felt hard to review this changes and couldn't justify reimplementing the lexer & parser for this. |
now its not reimplementing the parser, just doing the error messages |
|
Post-merge self-review of the remaining diff ( 1. Error priority. The def f():
global x
print(x)
nonlocal x
# CPython: name 'x' is used prior to nonlocal declaration (line 4)
# before fix: name 'x' is nonlocal and global (line 2)
def f():
x = 1
def g():
nonlocal x
x = 2
global x
# CPython: name 'x' is assigned to before global declaration
# before fix: name 'x' is nonlocal and globalFix: moved the 2. Directive location overwrite. def f():
global x
global x
nonlocal x
# CPython: error at line 2; before fix: line 3Fix: only store the location when Validation: 785/785 codegen unit tests pass (added tests for both behaviors), clippy/rustfmt clean, |
|
The two fixes above are now pushed in d944e0a ( |
Why this touches
|
Runtime error-message alignment (d1aad65, 5605a25, 94c901e)Following the parser-side alignment earlier in this PR, these commits complete the runtime error messages: VM, builtins, protocol layers, native stdlib modules, and the format-string machinery — all verified message-by-message against the CPython 3.14.7 sources, with no parser changes. How it was verified
What changedArity errors — new helpers in
A CPython quirk reproduced faithfully: keyword rejections use class-qualified names ( Constructor argument errors for Semantic messages — concat errors with Unraisable reports — Format strings — PEP 649 — attached async for — The stdlib commit (d1aad65) continues the module-by-module alignment from 0e12017 across the native bindings (sqlite3, array, binascii, csv, fcntl, json, locale, lzma, math, mmap, openssl, pystruct, resource, select, socket, ssl, termios, zlib) plus the shared cformat/marshal strings. Test impact
|
|
@JamesClarke7283 can you please fix the merge/rebase |
On it |
b05af61 to
fca3d8b
Compare
Continues RustPython#7928/RustPython#7933/RustPython#7988. Translates many more ruff ParseErrorType variants to CPython's exact wording in CompileError::from_ruff_parse_error, and routes ast.parse() / compile(PyCF_ONLY_AST) through the same path so those messages match too (previously they leaked raw ruff strings). Adds a few codegen/symtable checks. Covered: aug-assign/delete/set/dict/f-string/t-string targets; "cannot use {kind} as import target"; string-prefix incompatibility and "invalid character 'X' (U+XXXX)"; parenthesized def/lambda params; missing default/argument value; dict ':' / value syntax; "'elif' block follows an 'else' block"; raise-from; comprehension 'if'; ternary statement keywords; match "case ... as <target>" -> "cannot use {kind} as pattern target" and "case ... as _"; __debug__ as def/class/type-param/except name; "name 'x' is nonlocal and global"; generic type-parameter wording. Lib/test: drop the now-passing "# TODO: RUSTPYTHON; Wrong error message" doctest markers and @expectedfailure decorators. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`eval` calls the host `print` function, so rust-lld reported it as an undefined symbol and the wasm32-unknown-unknown build failed to link (`kv_get`/`kv_put` are unused, so they were GC'd and did not error). Annotate the `extern "C"` block with `#[link(wasm_import_module = "env")]` so the linker emits the host functions as wasm imports from the `env` module, matching the wasmer host runtime in wasm-runtime/src/main.rs. Verified: `cargo build` (the CI "check wasm32-unknown without js" step) now links and produces the .wasm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_dictcomps.test_illegal_assignment, test_fstring.test_invalid_string_prefixes and test_unicode_identifiers.test_invalid now pass thanks to this PR's error-message alignment, so their @unittest.expectedFailure markers caused "unexpected success" failures in CI. Remove the obsolete markers (same cleanup already applied to test_syntax/test_genexps/test_named_expressions/test_patma). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Report "Invalid star expression" for bare leading `*` in set/dict displays and non-call parenthesised groups (`{*}`, `(*,)`)
- Collapse double-comma in dict/set/list displays (`{1:2,, 3}`, `[1,, 2]`) to "invalid syntax"
- Add is_bare_star_first_in_group helper
- Drop stale "Is this intended to be part of the string?" uppercase-message entry
pdb's `_exec_in_closure` wraps the debugger input in a generated `nonlocal <var>` scope, so a user's `global g` conflicts with it. CPython rejects that with "name 'g' is nonlocal and global", which pdb catches to fall back to a plain exec. Now that the symbol table raises the same error, test_pdb_closure produces CPython's output and the `+EXPECTED_FAILURE` marker inverts it into a failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All of this happens in the post-parse diagnostic layer; the parser itself is an external pinned crate and is untouched. - Reject incompatible string prefixes (`ub''`, `turf"..."`, ...) with CPython's message, using the same check order as `_PyLexer_check_string_prefixes` so a prefix with several conflicts names the same pair. - Report the "here. Maybe you meant '==' instead of '='?" hint for set, dict, f-string and t-string assignment targets, and narrow the scanned span to the enclosing statement so an indented `x() = 1` is diagnosed like a top-level one. Narrowing is gated on the parser's own error offset so an earlier malformed header still wins. - Consult the import- and match-target scanners before the generic "forgot a comma?" heuristic, and let them see `as` targets nested in parentheses. - Skip statement-only diagnostics when compiling in `eval` mode, where CPython reports a plain "invalid syntax". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follows the SyntaxError work with the non-syntax `wrong error message`
markers. Each message was compared against CPython 3.14.7 directly.
- structseq: accept CPython's second `dict` argument, reporting "got
duplicate or unexpected field name(s)" when a key duplicates a
positional field or names none, and raise "readonly attribute" from
the field descriptors as member descriptors do.
- posix_spawn: validate `scheduler` in the body so a wrong type says
"scheduler must be a tuple or None".
- socket.sendto: bind by hand to report "sendto() takes 2 or 3 arguments
(N given)" and "socket.sendto() takes no keyword arguments".
- bz2: report libbzip2's "Invalid data stream", and make a decompressor
unusable after a failure instead of resuming from inconsistent state.
- import: resolve `__import__` against the running frame's builtins, so
`exec(code, {"__builtins__": {}})` raises ImportError, and pass None
rather than () as the from-list of a plain import.
- symboltable: name the variable as written, not mangled, in
"assignment expression cannot rebind comprehension iteration variable".
- _pydatetime: raise the message CPython's C _datetime uses when
subtracting a naive and an aware datetime; the pure-Python module is
the only implementation here.
test_hashlib and test_ast stay marked: both need the callee's name, or
non-string keyword keys, to reach argument binding, which is a change to
the calling convention rather than to a message.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The authorizer denied every statement. SQLite passes NULL for the arguments an action does not use — all four are NULL for SQLITE_SELECT — and `ptr_to_str` raised MemoryError on NULL, which the trampoline swallowed as SQLITE_DENY before the callback ever ran. Those arguments now reach the callback as None, matching CPython's callback trace. That was also the reason the denial message differed: RustPython stopped at the non-column SQLITE_SELECT check, which SQLite reports as the generic "not authorized", where CPython reached the column check and got "access to t2.c1 is prohibited". Also: - Bound the argument count before handing it to SQLite, so create_function and create_window_function report "'narg' must be between -1 and 1000, not -100" instead of a generic creation failure. - Raise ValueError for every invalid `autocommit`, without the ", not X" suffix CPython does not use; a non-integer raised TypeError before. A working authorizer makes the "concurrent mutation" tests reachable, and they hang: they call back into the connection from inside a callback, which deadlocks on the connection mutex. Skipped with that reason until the locking is re-entrant; CI builds with `sqlite`, so leaving them running would hang the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CPython's clinic-generated signatures name the callee and the position of
`data`, and check a duplicated argument before an unknown keyword and an
unknown keyword before the data/string conflict. The generic binder knows
none of that, so the constructors bind by hand:
hashlib.md5(b'', data=b'') argument for openssl_md5() given by name
('data') and position (1)
hashlib.md5(_=None) openssl_md5() got an unexpected keyword
argument '_'
`hashlib.blake2b` resolves to `_blake2.blake2b` rather than the openssl
constructor, so the two share an implementation that takes the name to
report.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Continues the runtime error-message alignment from 0e12017 across the native stdlib modules: argument validation for the sqlite3, array, binascii, csv, fcntl, json, locale, lzma, math, mmap, openssl, pystruct, resource, select, socket, ssl, termios and zlib bindings, plus the shared %-formatting (cformat) and marshal format strings in compiler-core. Each message was matched against the CPython 3.14.7 sources, including argument-by-name wording and the order CPython validates in. Assisted-by: Claude:Claude Opus 5
Completes the runtime error-message alignment for the VM, builtins and
protocol layers, all verified against CPython 3.14.7:
- Arity errors: new helpers in function::argument mirror CPython's
three message styles (_PyArg_CheckPositional, METH_O/noargs wrappers
and the clinic forms). Builtin functions (abs/chr/len/map/pow/round/
format/filter/...) and methods across dict, list, set, tuple, str,
bytes, bytearray, int, float, slice, range and property now report
CPython's exact wording, including class-qualified names for keyword
rejections but bare names for positional counts. Native-method arity
errors no longer count the receiver, and the generic binder renders
exact/singular forms when min == max.
- Constructor argument errors for str/bytes/bytearray/int/float/set/
range/slice/complex/enumerate/type, including duplicate name-and-
position and missing-required-argument wording.
- Semantic messages: concat errors use tp_name semantics (double quotes,
module-qualified names), sequence-repeat reports "can't multiply
sequence by non-int", str.join reports "can only join an iterable",
__index__ conversions, attribute set/delete errors (read-only and
no-__dict__ suffixes), NoneType immutability, raise vs gen.throw
wording, unbound-method and wrapper-descriptor messages, and the
str.translate table "must be" wording.
- Unraisable reports: __del__ failures report "Exception ignored while
calling deallocator <repr>" and generator-close failures report
"Exception ignored while closing generator <repr>" with the synthetic
GeneratorExit error carrying a traceback.
- Format strings: "Single '{'/'}' encountered", "unmatched '{' in
format spec", "Unknown conversion specifier" (validated at format
time so _string.formatter_parser stays lenient), and
"Invalid format specifier '<spec>' for object of type '<T>'".
- PEP 649: attached __annotate__ functions get the
"<outer>.__annotate__" qualname (gh-137814).
- async for: GET_AITER validates __aiter__/__anext__ presence and
GET_ANEXT awaits via _PyCoro_GetAwaitableIter with from-cause errors.
capi callers updated for the new method signatures.
Assisted-by: ZCode:GLM-5.3
Assisted-by: Claude:Claude Opus 5
Removes 37 stale TODO: RUSTPYTHON markers across 26 test files now that the corresponding error messages match CPython 3.14: async-for errors (test_coroutines), str()/Template/concat errors (test_str, test_tstring, string_tests), unraisable reports (test_exceptions, test_generators), __annotate__ qualnames (test_type_annotations), constructor arity (test_range, test_posix, test_sqlite3, test_struct), find-family messages (test_bytes), attribute errors (test_class, test_descr, test_descrtut), exec/eval arguments (test_pdb, test_extcall), map (test_itertools), marshal readers, lzma filter specs, enum, json scanstring (C variant only; the pure-Python scanner still lacks the OverflowError, so that variant keeps a scoped marker), mmap resize and pdb's exec/eval doctests. Assisted-by: ZCode:GLM-5.3
fca3d8b to
684929d
Compare
Rebased onto current mainHistory is now linear on top of upstream/main (no merge commits), and the conflicts against the recent landings were resolved as follows:
The only remaining test_socket failures are UDPLITE ones — this kernel has no |
- posix: qualify the PyTuple path in the posix_spawn scheduler check so
macOS (where the bare import is not in scope) compiles.
- coroutine: gate the Radium import on not(threading); without the
feature lasti is a Cell and its load() comes from the trait. Fixes the
wasm build and miri.
- marshal (compiler-core): drop the duplicate NullObject Display arm
left by the conflict resolution and the now-unnecessary usize cast;
both were -Dwarnings clippy errors.
- socket: getaddrinfo's IDNA path no longer clones the host str
(redundant_clone), passing it by value.
- str: port upstream's char-aware count() body behind this branch's
FuncArgs front - an empty needle is counted in characters (chars + 1),
not encoded byte positions, so "가나다".count("") is 4 again. Also
drops a duplicate #[inline] and a redundant #[must_use] that tripped
-Dwarnings, and py_split_str takes an #[expect(too_many_arguments)]
like its anystr counterpart.
Verified with the CI commands: workspace clippy -Dwarnings with the CI
feature set and excludes, the sandbox-mode checks, the snippets suite
(builtin_str green), and test_str/test_marshal/test_socket (only the
UDPLITE tests fail here - this kernel has no IPPROTO_UDPLITE).
Assisted-by: ZCode:GLM-5.3
- socket: getaddrinfo's port accepts str, bytes and bytearray service names like CPython's setipaddr; anything else (floats, lists) raises OSError "Int or String expected". The int shortcut (decimal string) stays. This un-breaks asyncio's create_connection, whose resolution path fed the service name through getaddrinfo on all three CI operating systems. - socket: the PyInt import moved into the linux-only sendmsg_afalg, so macOS/Windows builds no longer see an unused import under -Dwarnings. - mmap: the flags field is now cfg(linux/netbsd), matching its only reader (the mremap expansion check), so macOS clippy no longer flags it as never read. - test_threading: test_join_daemon_thread_in_finalization stays an expected failure - it passed once on CI runners but fails deterministically here because daemon-thread shutdown ordering differs; the marker documents the dependency. Assisted-by: ZCode:GLM-5.3
- mmap: the re-bound `flags` in py_new is only stored on linux/netbsd (the mremap expansion check is its only reader), so allow the unused variable on other unixes. - os: dir_fd_and_fd_invalid is only called from the unix chown; allow dead_code off unix instead of gating the definition. With these, clippy -Dwarnings passes with the CI feature set on every platform: the previous run had only these two jobs failing. Assisted-by: ZCode:GLM-5.3
- array: the intermediate int/unsigned-int range conversions are no-ops on Windows (c_long is i32, c_ulong is u32 there); allow the useless-conversion lint there while keeping CPython's two-step error order on LP64 platforms (verified against CPython for H/I overflow, negative, and 2**40 inputs). - socket: PyIter moved from the module import list into the unix-only sendmsg, removing the unused import on Windows. Assisted-by: ZCode:GLM-5.3
c_ulong is u32 there, which is the Self of the impl, so clippy's use_self fires; on LP64 platforms it stays raw::c_ulong to keep CPython's "Python int too large to convert to C unsigned long" step. Assisted-by: ZCode:GLM-5.3
|
@JamesClarke7283 can you please run: assuming that |
Reviewing the whole PR against CPython 3.14 surfaced a batch of
methods whose argument errors still went through the generic binder
("Expected type 'int' but 'float' found."), plus three re-entrancy
safety regressions that the new conversion order exposed:
- bytes/bytearray expandtabs, hex(bytes_per_sep), zfill (bytes) and
the padding family (center/ljust/rjust width) now convert with
PyNumber_AsSsize_t semantics ("'float' object cannot be
interpreted as an integer", OverflowError at the ssize_t bounds).
- find-family start/end and startswith/endswith bounds convert as
slice indices ("slice indices must be integers or None or have an
__index__ method"), clamp like _PyEval_SliceIndex instead of
erroring at the bounds, and - for bytes/bytearray - are validated
before the needle so a bad index reports first, as CPython's
parsing order does. The needle itself is also converted after the
slice indices.
- bytes/bytearray decode() reports "decode() argument 'encoding'/
'errors' must be str, not X" and join() reports "can only join an
iterable".
- hex(sep) measures the separator with PyObject_Length semantics
("object of type 'float' has no len()", validated before the
empty-input and bytes_per_sep==0 early returns), and the fillchar
length error names type and length the way CPython's stringlib
does ("center(): argument 2 must be a byte string of length 1,
not a bytes object of length 2").
- gh-143195 / gh-142560 re-entrancy: bytearray's find/index/rfind/
rindex/count/__contains__/split/rsplit/hex and memoryview's hex
now run their argument conversions under an export guard, so a
re-entrant __len__/__index__/__buffer__ that resizes the buffer
raises BufferError ("Existing exports of data: object cannot be
re-sized" / "memoryview has 1 exported buffer") as in CPython.
memoryview.release() refuses while exports are live.
Un-marks the tests those fixes make pass (test_hex_use_after_free,
test_search_methods_reentrancy_raises_buffererror).
Assisted-by: ZCode:GLM-5.3
Summary
Continues the parser error-message alignment from #7928 / #7933 / #7988. Translates many more ruff
ParseErrorTypevariants to CPython 3.14.5's exact wording inCompileError::from_ruff_parse_error(using the source slice / parsed AST kind), plus a few codegen/symtable checks.crates/vm/src/stdlib/_ast.rsnow routes its parse errors throughfrom_ruff_parse_error, soast.parse()/compile(..., PyCF_ONLY_AST)produce the same CPython-aligned messages as the exec path (they previously leaked raw ruff strings — and this fixes a regression where the var-param message diverged on the AST path).What's aligned
cannot use {attribute,subscript,tuple,list,literal,function call} as import target;import X from Y→ "Did you mean to use 'from ... import ...' instead?"'u' and 'b' prefixes are incompatible);invalid character 'X' (U+XXXX):and value syntax'elif' block follows an 'else' block;raise from; comprehensionif/ unparenthesized target; ternary statement-keyword hintscase … as <target>→cannot use {kind} as pattern target, andcase … as _→cannot use '_' as a target__debug__as adef/class/type-param/except-handler name →cannot assign to __debug__name 'x' is nonlocal and global; generic type-parameter wording (… cannot be used within the definition of a generic)This drops the now-passing
# TODO: RUSTPYTHON; Wrong error messagedoctest markers and@expectedFailuredecorators acrosstest_syntax.py,test_genexps.py,test_named_expressions.py, andtest_patma.py.Not covered (intentionally left marked)
A few
test_syntax.pycases need ruff-parser or deeper codegen changes and remain marked rather than emitting a silently-wrong message: the type-commentbare *case,class C(x for x in L)andf((x)=2)(ruff accepts these without a parse error), anddict(...); x $ y(duplicate-keyword vs lexer ordering). Runtime-error messages in unrelated subsystems (struct-sequence, format-spec, datetime,__import__, ast__replace__) are out of scope for this parser-focused change.Verification
cargo clippy -p rustpython-compiler -p rustpython-codegen -- -D warningsclean;cargo fmt.ast.parse().test_syntax test_genexps test_named_expressions test_patma test_type_params test_scope test_grammar test_compile test_exceptionsall pass (961 tests).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
_as a capture/store targetChores