Align more error messages with CPython 3.14.5#7928
Conversation
|
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:
📝 WalkthroughWalkthroughRefactors VM syntax-error construction: adds ruff_python_parser imports, updates macro-generated exception helper return types to fully-qualified builtins refs, introduces SyntaxErrorInfo to derive messages and caret behavior from parser errors, integrates it into new_syntax_error_maybe_incomplete; also updates a module-level ChangesSyntax Error Handling Refactor
SymbolTable Message Update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_positional_only_arg.py dependencies: dependent tests: (no tests depend on positional_only_arg) [ ] test: cpython/Lib/test/test_metaclass.py (TODO: 4) dependencies: dependent tests: (no tests depend on metaclass) [ ] test: cpython/Lib/test/test_syntax.py (TODO: 305) dependencies: dependent tests: (no tests depend on syntax) [x] test: cpython/Lib/test/test_named_expressions.py (TODO: 8) dependencies: dependent tests: (no tests depend on named_expressions) Legend:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/vm/src/vm/vm_new.rs (1)
661-661: 💤 Low valueUse
and_thento avoid nestedOption<Option<String>>.
source.map(...)withget_statementreturningOption<String>createsOption<Option<String>>. While this works due to howToPyObjectis implemented for nestedOptions, usingand_thenis more idiomatic and flattens toOption<String>.♻️ Suggested refactor
- let statement = source.map(|src| get_statement(src, error.location())); + let statement = source.and_then(|src| get_statement(src, error.location()));🤖 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/vm/src/vm/vm_new.rs` at line 661, The current assignment let statement = source.map(|src| get_statement(src, error.location())); produces an Option<Option<String>> because get_statement returns Option<String>; change the call to use and_then (i.e. source.and_then(|src| get_statement(src, error.location()))) so the nested Option is flattened to Option<String>; update the expression where statement is defined in vm_new.rs and keep the same parameters (src and error.location()) when switching to and_then.
🤖 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.
Nitpick comments:
In `@crates/vm/src/vm/vm_new.rs`:
- Line 661: The current assignment let statement = source.map(|src|
get_statement(src, error.location())); produces an Option<Option<String>>
because get_statement returns Option<String>; change the call to use and_then
(i.e. source.and_then(|src| get_statement(src, error.location()))) so the nested
Option is flattened to Option<String>; update the expression where statement is
defined in vm_new.rs and keep the same parameters (src and error.location())
when switching to and_then.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 86bf8420-246f-487c-830a-d0e3819ae0c7
⛔ Files ignored due to path filters (1)
Lib/test/test_syntax.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/vm/src/vm/vm_new.rs
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/vm/src/vm/vm_new.rs`:
- Around line 668-679: The cfg_select! block creates syntax_error_info immutably
in the default arm but the code later calls syntax_error_info.with_msg(&mut
self), causing a mutability mismatch; fix it by allocating a single mutable
binding: create let mut syntax_error_info = SyntaxErrorInfo::new(msg, false)
outside or before the cfg branch, then inside only run the parser-only behavior
under #[cfg(feature = "parser")] (call
syntax_error_info.analyze_compile_error(error) there) so the binding is always
mutable for later calls to syntax_error_info.with_msg; update the cfg_select! /
cfg attributes accordingly around analyze_compile_error to avoid duplicating the
allocation.
- Around line 100-105: Update the error strings for the lexical parse errors to
match CPython wording: in the ParseErrorType::Lexical arm handling
LexicalErrorType::LineContinuationError change the message to "unexpected
character after line continuation character" and in the arm for
LexicalErrorType::UnclosedStringError change the message to "unterminated string
literal" so the variants ParseErrorType::Lexical and
LexicalErrorType::UnclosedStringError produce exact CPython messages.
- Around line 115-132: Three parser error messages use the word "argument" but
CPython 3.14 uses "parameter"; update those message strings to use "parameter"
instead. Locate the ParseErrorType variants emitting these messages—specifically
ParseErrorType::DuplicateKeywordArgumentError (the "keyword argument repeated"
message) and ParseErrorType::VarParameterWithDefault (the "var-positional
argument cannot have default value" message)—and change their formatted/static
strings to use "parameter" (e.g., "keyword parameter repeated", "var-positional
parameter cannot have default value"); also update any other ParseErrorType
message in the same area that contains "argument" to use "parameter" for
consistency. Ensure only the wording changes, preserving formatting/argument
interpolation (e.g., {arg_name}) and tests.
🪄 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: 0e9e28c2-1a3a-41d1-b684-53e2342a46da
📒 Files selected for processing (1)
crates/vm/src/vm/vm_new.rs
| ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(), | ||
|
|
||
| ParseErrorType::EmptyImportNames => "Expected one or more names after 'import'".into(), | ||
|
|
||
| ParseErrorType::DuplicateKeywordArgumentError(arg_name) => { | ||
| format!("keyword argument repeated: {arg_name}") | ||
| } | ||
|
|
||
| ParseErrorType::UnparenthesizedGeneratorExpression => { | ||
| "Generator expression must be parenthesized".into() | ||
| } | ||
|
|
||
| ParseErrorType::NonDefaultParamAfterDefaultParam => { | ||
| "parameter without a default follows parameter with a default".into() | ||
| } | ||
|
|
||
| ParseErrorType::VarParameterWithDefault => { | ||
| "var-positional argument cannot have default value".into() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd crates/vm/src/vm && wc -l vm_new.rs && head -150 vm_new.rs | tail -50Repository: RustPython/RustPython
Length of output: 2004
🏁 Script executed:
cd crates/vm/src/vm && sed -n '115,132p' vm_new.rsRepository: RustPython/RustPython
Length of output: 867
🏁 Script executed:
cd crates/vm/src/vm && sed -n '175,180p' vm_new.rsRepository: RustPython/RustPython
Length of output: 335
🌐 Web query:
CPython 3.14 "named parameters must follow bare *" syntax error message
💡 Result:
In CPython, the SyntaxError message “named parameters must follow bare ” is raised when you put a bare asterisk () in a function parameter list in a way that does not allow any following keyword-only parameters. In other words, the grammar requires that “” be followed by keyword-only parameter definitions (e.g., “def f(, kwonly):...”); if “” is the last token (no keyword-only parameters after it), CPython reports this syntax error. Example that triggers it: >>> def f():... pass... SyntaxError: named arguments must follow bare * [1][2] Why this happens: a bare “” marks the transition to keyword-only parameters, so “” must be immediately followed by at least one keyword-only parameter (or otherwise the signature is incomplete/invalid). [1][2] Notes: - The error text you cited can appear in tooling with slightly different wording (“named arguments” vs “named parameters”), but the underlying CPython syntax rule is the same: after a bare “*”, you must have keyword-only parameters. [3][1] If you want, paste the exact CPython 3.14 source code snippet (and how you’re running it—REPL/file) and I can identify the specific invalid signature form that produces the error in that case.
Citations:
- 1: https://stackoverflow.com/questions/14301967/what-does-a-bare-asterisk-do-in-a-parameter-list-what-are-keyword-only-parame
- 2: https://realpython.com/python-asterisk-and-slash-special-parameters/
- 3: Setting only
*to an empty parameter list gets a wrong error message saying "arguments" instead of "parameters" python/mypy#21003
🌐 Web query:
CPython "var-positional parameter cannot have default value" error message
💡 Result:
In CPython/Python, you cannot give a default value to a var-positional parameter (*args). What you likely ran into is the generic “non-default/required parameter follows default” class of syntax checks: required parameters (those without defaults) must not appear after parameters that have defaults, and the *args position determines which parameters are “required”. In particular, a parameter list like def f(a=1, *args, b): ... is rejected because b is required but appears after a defaulted parameter, and the parser surfaces it as the “parameter without a default follows parameter with a default” SyntaxError [1]. A related CPython issue also discusses how this error applies to keyword-only parameters and how signatures involving *args can appear confusing in terms of whether a later parameter is treated as positional or keyword-only [2]. If your concrete code snippet is something like def func(*args=...): then the correct mental model is simply: the *args parameter is always the “collect remaining positional arguments” slot, so it’s not legal syntax to attach a default initializer to it (unlike normal named positional-or-keyword parameters).
Citations:
- 1:
SyntaxError: parameter without a default follows parameter with a defaultis inaccurate python/cpython#129545 - 2: Missing error: "parameter without a default follows parameter with a default" with a function that uses
*argspython/cpython#129810
🌐 Web query:
CPython "at least one parameter must precede /" syntax error
💡 Result:
The SyntaxError text “at least one parameter must precede /” is raised when you use the positional-only separator “/” in a function parameter list without any parameters before it (i.e., an invalid signature like def f(/,...):...). Python’s syntax rule for positional-only parameters uses “/” to separate positional-only parameters (to the left) from the rest (to the right) [1][2]. PEP 570 defines that all parameters before “/” are positional-only, and that “/” is the delimiter between the left-hand parameter group and following parameters [1]. Therefore, the delimiter cannot appear as the very first element of the parameter list; at least one parameter must appear on the left side of “/”. Evidence in the official docs: - The tutorial section explains that positional-only parameters are placed before a “/”, and that “/” logically separates them from the rest of the parameters [2]. - PEP 570 similarly states that parameters left of “/” are treated as positional-only, implying “/” can only function as a separator after that left-hand parameter group [1]. If you share the exact snippet you’re running (including the full def statement), I can pinpoint precisely which part triggers the error.
Citations:
🌐 Web query:
CPython 3.14 test_syntax.py "named parameters" "var-positional parameter"
💡 Result:
In CPython’s syntax tests, a “var-positional” parameter is the *name part of a function signature. CPython rejects giving it a default value: SyntaxError: var-positional parameter cannot have default value is asserted in Lib/test/test_syntax.py (e.g. def foo(a,*b=3,c): ...). [1] This is consistent with how the grammar defines *args-style parameters: a var-positional parameter collects an arbitrary number of positional arguments, so it cannot simultaneously represent a single optional value via a default. The syntax test explicitly treats *b=3 (and also lambda a,*b=3,c: ...) as a syntax error. [1] Notes on “named parameters” (keyword arguments): what you can name in calls is separate from whether you may put a default on the *args parameter in the function definition. The test coverage for keyword argument behavior is different from the parser rule that disallows defaults on var-positional parameters. [1] Authoritative source inspected: CPython’s own test file Lib/test/test_syntax.py, which includes the exact SyntaxError cases for *b=3 / var-positional parameter cannot have default value. [1]
Citations:
🌐 Web query:
site:github.com cpython "named parameters must follow bare" 3.14
💡 Result:
In CPython 3.14, “named parameters must follow bare *” refers to a syntax rule for function parameter lists: once you include a bare * (meaning “start of keyword-only parameters”), any following parameters that are allowed after that * must be keyword-only parameters, and those keyword-only parameters must appear in the form “name[=default]” (i.e., named parameters). If you put something other than a keyword-only named parameter after the bare *, the parser raises a SyntaxError with that message. This message is part of CPython’s general validation of parameter lists around the * / keyword-only boundary (PEP 3102 describes the semantics of keyword-only parameters introduced via the bare * marker). [1] Relevant CPython issues discuss adjacent wording and related parser constraints around this exact error message: - CPython issue reports an error message specifically about “Named parameters must follow bare *” when defining a signature like def func(*): ... and notes the message should be corrected from “arguments” to “parameters” (mypy reproduces the CPython-derived message text). [2] - CPython issues discuss the accuracy and phrasing of parameter-related SyntaxError messages in this area (e.g., “non-default argument follows default argument” vs “parameter”). These indicate that CPython has been actively refining the precise text of such syntax errors, including those near the * / keyword-only syntax. [3][4] If you want, paste the exact function definition you’re compiling/running and I can tell you which specific token after * triggers the “must follow bare *” SyntaxError in CPython 3.14.
Citations:
- 1: https://github.com/python/peps/blob/main/peps/pep-3102.rst
- 2: Setting only
*to an empty parameter list gets a wrong error message saying "arguments" instead of "parameters" python/mypy#21003 - 3:
SyntaxError: parameter without a default follows parameter with a defaultis inaccurate python/cpython#129545 - 4: "SyntaxError: non-default argument follows default argument" confuses python/cpython#91210
🌐 Web query:
site:raw.githubusercontent.com/python/cpython "named parameters must follow bare" test_syntax
💡 Result:
I couldn’t find any authoritative technical source that actually contains the specific phrase “named parameters must follow bare test_syntax”. The web results I retrieved look unrelated (for example, Python docs about doctest and a sqlite3 note about named placeholders), and none describe a rule with that wording. If you meant a common Python syntax error rule, the relevant one is usually phrased as: positional/non-default arguments must come before default arguments (e.g., you can get “SyntaxError: non-default argument follows default argument”). However, that’s not the same as “named parameters must follow bare … test_syntax”, and I did not find a matching source for your exact query. Please paste the exact code snippet and the exact error message (and the language/tool name), and I’ll re-research specifically for that scenario.
Citations:
- 1: https://raw.githubusercontent.com/python/cpython/master/Doc/library/doctest.rst
- 2: https://raw.githubusercontent.com/python/cpython/main/Doc/whatsnew/3.12.rst
- 3: https://raw.githubusercontent.com/python/cpython/3.12/Lib/argparse.py
- 4: https://raw.githubusercontent.com/python/cpython/main/Doc/library/typing.rst
- 5: https://raw.githubusercontent.com/python/cpython/master/Doc/library/functions.rst
🌐 Web query:
site:raw.githubusercontent.com/python/cpython "at least one parameter must precede" test_syntax
💡 Result:
The phrase "at least one parameter must precede" is not a standard error message in Python's core libraries or common testing frameworks like pytest. Based on a search of Python's source code and documentation, this specific string does not appear as a defined error message. It is possible that the phrase is: 1. A misremembered or slightly altered version of a different error message (e.g., related to argument parsing or function signatures). 2. A specific error message from a third-party library or a custom application not indexed in general technical documentation. 3. A misunderstanding of a technical requirement, such as the rule in Python's typing module where a Union must contain at least one type [1], or requirements for positional arguments in functions [2]. If you are encountering this error in a specific context, please check the stack trace to identify the library or module raising the exception, as it is likely specific to that codebase.
Citations:
- 1: https://raw.githubusercontent.com/python/cpython/main/Doc/library/typing.rst
- 2: https://raw.githubusercontent.com/python/cpython/master/Doc/library/doctest.rst
🌐 Web query:
CPython github.com/python/cpython "named parameters must follow bare *" OR "var-positional parameter cannot have default value" OR "at least one parameter must precede"
💡 Result:
In Python, function parameter syntax is strictly governed by rules designed to ensure unambiguous argument binding. The error messages you referenced correspond to specific violations of these rules: 1. Named parameters must follow bare *: This error occurs when you attempt to define a parameter without a default value after a bare * (which denotes the start of keyword-only parameters) without providing a default value, or more generally, when the structure of keyword-only arguments is violated [1][2]. A bare * indicates that all subsequent parameters must be passed as keyword arguments [3]. 2. Var-positional parameter cannot have default value: A var-positional parameter (defined as *args) collects any number of positional arguments into a tuple [4]. Because it consumes all remaining positional arguments, it cannot have a default value; assigning one would be syntactically meaningless as there is no scenario where the parameter would be "missing" to trigger the default [4]. 3. At least one parameter must precede: This error typically arises when using the positional-only parameter separator (/). The / must be preceded by at least one positional-only parameter [5]. Similarly, if you were to use other markers incorrectly, the parser requires them to be placed in a valid context relative to other parameters [6]. In summary, Python requires that: - Positional-only parameters (before /) must be defined first [3][7]. - Positional-or-keyword parameters follow [3][7]. - Var-positional parameters (*args) collect remaining positional arguments [4]. - Keyword-only parameters (after * or *args) must be passed by name [1][3]. - Var-keyword parameters (kwargs) collect remaining keyword arguments [8]. Violating the order of these categories or attempting to assign defaults to variadic parameters results in a SyntaxError during the parsing of the function definition [4][9][5].
Citations:
- 1: https://github.com/python/peps/blob/main/peps/pep-3102.rst
- 2: https://stackoverflow.com/questions/56939639/default-values-for-positional-function-arguments-in-python
- 3: https://peps.python.org/pep-570
- 4: https://dev.to/hyperkai/args-in-python-532n
- 5: https://www.scribd.com/document/995887114/152-Positional-Only-Arguments
- 6: https://blog.csdn.net/tycool/article/details/78251
- 7: https://peps.python.org/pep-0457/
- 8: https://stackoverflow.com/questions/68878875/parameters-with-default-values-and-variadic-parameters-together
- 9: https://stackoverflow.com/questions/51915103/python-ordering-rules-for-arguments-passing-in-a-function-call
🌐 Web query:
CPython compile.c "named parameters must follow" OR "named arguments must follow"
💡 Result:
In CPython the message is "named arguments must follow bare *" (not “named parameters must follow …”), and it’s not emitted from compile.c.
Where it lives / how to find it:
- Modern CPython (PEG parser / pegen): the error text is attached to an invalid-grammar production in
Grammar/python.gram, e.g.invalid_star_etc/invalid_lambda_star_etcwhich raisesRAISE_SYNTAX_ERROR("named arguments must follow bare *"). (git.joshthomas.dev) - Older CPython (pre-PEG era): the same string appears in
Python/ast.c, where the AST builder raisesast_error(..., "named arguments must follow bare *")when*appears in a parameter list without any following keyword-only parameters. (chromium.googlesource.com)
Background/history: this exact SyntaxError wording dates back to the introduction of keyword-only parameters (PEP 3102 era); see the historical tracker discussion (migrated to GitHub) showing the same message. (bugs.python.org)
Citations:
- 1: https://git.joshthomas.dev/mirrors/cpython/src/commit/145bf269df3530176f6ebeab1324890ef7070bf8/Grammar/python.gram
- 2: https://chromium.googlesource.com/external/github.com/python/cpython/%2B/refs/tags/v3.7.17/Python/ast.c
- 3: https://bugs.python.org/issue2613?utm_source=openai
🌐 Web query:
CPython grammar "var-positional parameter" "default value" error message
💡 Result:
In CPython grammar, the syntax for a var-positional parameter is “” followed by a parameter name, optionally with a type annotation, and there is no provision for a default value for that parameter. The grammar rule for star_etc only allows “'' param_no_default …” or “'' param_no_default_star_annotation …” (and other forms like ','), i.e., no “'=' expression” is allowed on that branch. [1] When you write code like def f(*args='Hello'): ..., CPython raises a SyntaxError with the message “var-positional argument cannot have default value”. This is documented in a CPython issue that includes the exact error text. [2] So the key points are: 1) Grammar: *args (var-positional) is defined without default (no “= expression” for that form). [1] 2) Error: trying to attach a default to *args yields “SyntaxError: var-positional argument cannot have default value”. [2] If you need the exact grammar section name in the official docs: the var-positional part appears under star_etc in the full grammar specification. [1]
Citations:
- 1: https://docs.python.org/3.11/reference/grammar.html
- 2: The error messages of
def func(*args='Hello'): passanddef func(**kwargs='Hello'): passshould explain more clearly python/cpython#137786
🌐 Web query:
CPython 3.14 syntax error messages "parameter" changes from "argument"
💡 Result:
In CPython 3.14, there is (or was) a parser error-message wording inconsistency around function definition “parameter lists”: some SyntaxError messages say “argument” when the formal construct is a “parameter”. A CPython issue documents the proposed/desired change to use “parameter” / “parameters” wording (e.g., “duplicate parameter …” and “named parameters …”) and notes that this kind of change is only made in the latest version to reduce breakage and may not be backported.[1] Specifically, the issue reports these current messages and proposed replacements: - It reports def func(param, param): ... raising SyntaxError: duplicate argument 'param' in function definition, and suggests it should be SyntaxError: duplicate parameter 'param' in function definition.[1] - It reports def func(param1, param2, *): ... raising SyntaxError: named arguments must follow bare *, and suggests it should be SyntaxError: named parameters must follow bare *.[1] So the answer to your query is: yes—CPython’s 3.14 SyntaxError strings for certain function-parameter-list problems have been observed/targeted for replacement of “argument” with “parameter”, but the CPython issue indicates the wording change is treated as a last-mile polish and may be limited to newer versions (not necessarily backported broadly).[1] (Your query mentions “today is 2026-05-19”; the issue is dated 2025 and discusses behavior around current development/later versions, not necessarily guaranteeing that every 3.14.x build already uses the new wording.)
Citations:
🌐 Web query:
CPython "at least one" "parameter" "precede" "/" positional-only
💡 Result:
In CPython/Python syntax, the forward-slash marker “/” for positional-only parameters must be preceded by at least one parameter (i.e., you cannot start a function signature with “/”). This is expressed by the rule that “All parameters left of the / are treated as positional-only”, and by the grammar/usage rule that “at least one argument must precede /” (and also that “/ may appear only once”). [1][2] Practical consequence: a signature like def f(/, x): is invalid, while def f(x, /): is valid. [1][2]
Citations:
- 1: https://dev.to/hyperkai/positional-only-parameters-in-python-function-1lk8
- 2: https://peps.python.org/pep-570
Align error messages with CPython 3.14 by using parameter instead of argument.
CPython 3.14 has updated these error messages to use "parameter" wording for consistency. Update the three occurrences across lines 115–132 and 175–180.
Suggested changes
- ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(),
+ ParseErrorType::ExpectedKeywordParam => "named parameters must follow bare *".into(),- "var-positional argument cannot have default value".into()
+ "var-positional parameter cannot have default value".into()- "at least one argument must precede /".into()
+ "at least one parameter must precede /".into()🤖 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/vm/src/vm/vm_new.rs` around lines 115 - 132, Three parser error
messages use the word "argument" but CPython 3.14 uses "parameter"; update those
message strings to use "parameter" instead. Locate the ParseErrorType variants
emitting these messages—specifically
ParseErrorType::DuplicateKeywordArgumentError (the "keyword argument repeated"
message) and ParseErrorType::VarParameterWithDefault (the "var-positional
argument cannot have default value" message)—and change their formatted/static
strings to use "parameter" (e.g., "keyword parameter repeated", "var-positional
parameter cannot have default value"); also update any other ParseErrorType
message in the same area that contains "argument" to use "parameter" for
consistency. Ensure only the wording changes, preserving formatting/argument
interpolation (e.g., {arg_name}) and tests.
| cfg_select! { | ||
| feature = "parser" => { | ||
| let mut syntax_error_info = SyntaxErrorInfo::new(msg, false); | ||
| syntax_error_info.analyze_compile_error(error); | ||
| } | ||
| #[cfg(feature = "parser")] | ||
| crate::compiler::CompileError::Parse(rustpython_compiler::ParseError { | ||
| error: ruff_python_parser::ParseErrorType::OtherError(s), | ||
| .. | ||
| }) => { | ||
| if s.starts_with("Expected an identifier, but found a keyword") { | ||
| msg = "invalid syntax".to_owned(); | ||
| } | ||
| else if s.eq_ignore_ascii_case("positional patterns cannot follow keyword patterns") { | ||
| msg = "positional patterns follow keyword patterns".to_owned(); | ||
| } | ||
| _ => { | ||
| let syntax_error_info = SyntaxErrorInfo::new(msg, false); | ||
| } | ||
| _ => {} | ||
| } | ||
| }; | ||
|
|
||
| if syntax_error_type.is(self.ctx.exceptions.tab_error) { | ||
| msg = "inconsistent use of tabs and spaces in indentation".to_owned(); | ||
| syntax_error_info.with_msg("inconsistent use of tabs and spaces in indentation"); |
There was a problem hiding this comment.
Fix the mutability mismatch in syntax_error_info binding.
The _ cfg arm binds syntax_error_info immutably (line 675), but line 682 calls with_msg(&mut self) unconditionally. Extract the common SyntaxErrorInfo::new(msg, false) allocation and use #[cfg(feature = "parser")] for the conditional analyze_compile_error call:
Suggested fix
- cfg_select! {
- feature = "parser" => {
- let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
- syntax_error_info.analyze_compile_error(error);
- }
- _ => {
- let syntax_error_info = SyntaxErrorInfo::new(msg, false);
- }
- };
+ let mut syntax_error_info = SyntaxErrorInfo::new(msg, false);
+ #[cfg(feature = "parser")]
+ syntax_error_info.analyze_compile_error(error);📝 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.
| cfg_select! { | |
| feature = "parser" => { | |
| let mut syntax_error_info = SyntaxErrorInfo::new(msg, false); | |
| syntax_error_info.analyze_compile_error(error); | |
| } | |
| #[cfg(feature = "parser")] | |
| crate::compiler::CompileError::Parse(rustpython_compiler::ParseError { | |
| error: ruff_python_parser::ParseErrorType::OtherError(s), | |
| .. | |
| }) => { | |
| if s.starts_with("Expected an identifier, but found a keyword") { | |
| msg = "invalid syntax".to_owned(); | |
| } | |
| else if s.eq_ignore_ascii_case("positional patterns cannot follow keyword patterns") { | |
| msg = "positional patterns follow keyword patterns".to_owned(); | |
| } | |
| _ => { | |
| let syntax_error_info = SyntaxErrorInfo::new(msg, false); | |
| } | |
| _ => {} | |
| } | |
| }; | |
| if syntax_error_type.is(self.ctx.exceptions.tab_error) { | |
| msg = "inconsistent use of tabs and spaces in indentation".to_owned(); | |
| syntax_error_info.with_msg("inconsistent use of tabs and spaces in indentation"); | |
| let mut syntax_error_info = SyntaxErrorInfo::new(msg, false); | |
| #[cfg(feature = "parser")] | |
| syntax_error_info.analyze_compile_error(error); | |
| if syntax_error_type.is(self.ctx.exceptions.tab_error) { | |
| syntax_error_info.with_msg("inconsistent use of tabs and spaces in indentation"); |
🤖 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/vm/src/vm/vm_new.rs` around lines 668 - 679, The cfg_select! block
creates syntax_error_info immutably in the default arm but the code later calls
syntax_error_info.with_msg(&mut self), causing a mutability mismatch; fix it by
allocating a single mutable binding: create let mut syntax_error_info =
SyntaxErrorInfo::new(msg, false) outside or before the cfg branch, then inside
only run the parser-only behavior under #[cfg(feature = "parser")] (call
syntax_error_info.analyze_compile_error(error) there) so the binding is always
mutable for later calls to syntax_error_info.with_msg; update the cfg_select! /
cfg attributes accordingly around analyze_compile_error to avoid duplicating the
allocation.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/vm/src/vm/vm_new.rs (1)
686-700:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMutability mismatch causes compile error when
parserfeature is disabled.The
cfg_select!macro createssyntax_error_infoas mutable in theparserbranch (line 688) but immutable in the_branch (line 692). Line 697 unconditionally callssyntax_error_info.with_msg(...), which requires&mut self. This produces a compile error when theparserfeature is disabled.Fix
- cfg_select! { - feature = "parser" => { - let mut syntax_error_info = SyntaxErrorInfo::new(msg, false); - syntax_error_info.analyze_compile_error(error); - } - _ => { - let syntax_error_info = SyntaxErrorInfo::new(msg, false); - } - }; + let mut syntax_error_info = SyntaxErrorInfo::new(msg, false); + #[cfg(feature = "parser")] + syntax_error_info.analyze_compile_error(error);🤖 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/vm/src/vm/vm_new.rs` around lines 686 - 700, The variable syntax_error_info is mutable only in the "parser" cfg_select! branch but used mutably afterward by syntax_error_info.with_msg; make its mutability consistent by declaring a mutable SyntaxErrorInfo before or within both branches. Locate the cfg_select! block that constructs SyntaxErrorInfo (and the SyntaxErrorInfo::new calls) and ensure the binding is created as let mut syntax_error_info = ... in every branch (or move a single let mut syntax_error_info = ... above the cfg_select!), so syntax_error_type.is(self.ctx.exceptions.tab_error) and the later syntax_error_info.with_msg(...) compile without error.
🤖 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/vm/src/vm/vm_new.rs`:
- Line 115: Update the error message text in the ParseErrorType match arms to
use "parameter" instead of "argument" to match CPython 3.14 wording: change the
string for ParseErrorType::ExpectedKeywordParam from "named arguments must
follow bare *" to "named parameters must follow bare *", and likewise replace
"var-positional argument cannot have default value" and "at least one argument
must precede /" with "var-positional parameter cannot have default value" and
"at least one parameter must precede /" in their corresponding ParseErrorType
arms in vm_new.rs.
- Line 679: The variable statement is being set with source.map(|src|
get_statement(src, error.location())) which yields Option<Option<String>>
because get_statement returns Option<String>; change this to use and_then (or
otherwise flatten) so statement becomes Option<String> (e.g.,
source.and_then(|src| get_statement(src, error.location()))) before assigning it
to the text attribute used later; update the expression that constructs
statement (and any downstream use expecting Option<String>) to remove the nested
Option.
---
Duplicate comments:
In `@crates/vm/src/vm/vm_new.rs`:
- Around line 686-700: The variable syntax_error_info is mutable only in the
"parser" cfg_select! branch but used mutably afterward by
syntax_error_info.with_msg; make its mutability consistent by declaring a
mutable SyntaxErrorInfo before or within both branches. Locate the cfg_select!
block that constructs SyntaxErrorInfo (and the SyntaxErrorInfo::new calls) and
ensure the binding is created as let mut syntax_error_info = ... in every branch
(or move a single let mut syntax_error_info = ... above the cfg_select!), so
syntax_error_type.is(self.ctx.exceptions.tab_error) and the later
syntax_error_info.with_msg(...) compile without error.
🪄 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: ee49b272-06ab-4817-b575-6a7172652d84
⛔ Files ignored due to path filters (1)
Lib/test/test_syntax.pyis excluded by!Lib/**
📒 Files selected for processing (2)
crates/codegen/src/symboltable.rscrates/vm/src/vm/vm_new.rs
✅ Files skipped from review due to trivial changes (1)
- crates/codegen/src/symboltable.rs
| "cannot use starred expression here".into() | ||
| } | ||
|
|
||
| ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(), |
There was a problem hiding this comment.
Align error messages with CPython 3.14 by using "parameter" instead of "argument".
The PR title indicates alignment with CPython 3.14.5, but these messages still use "argument" where CPython 3.14 uses "parameter":
- Line 115:
"named arguments must follow bare *"→ should be"named parameters must follow bare *" - Line 132:
"var-positional argument cannot have default value"→ should be"var-positional parameter cannot have default value" - Line 192:
"at least one argument must precede /"→ should be"at least one parameter must precede /"
Suggested fix
- ParseErrorType::ExpectedKeywordParam => "named arguments must follow bare *".into(),
+ ParseErrorType::ExpectedKeywordParam => "named parameters must follow bare *".into(), ParseErrorType::VarParameterWithDefault => {
- "var-positional argument cannot have default value".into()
+ "var-positional parameter cannot have default value".into()
} ParseErrorType::OtherError(s)
if s.eq_ignore_ascii_case(
"position-only parameter separator not allowed as first parameter",
) =>
{
- "at least one argument must precede /".into()
+ "at least one parameter must precede /".into()
}Also applies to: 131-132, 187-193
🤖 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/vm/src/vm/vm_new.rs` at line 115, Update the error message text in the
ParseErrorType match arms to use "parameter" instead of "argument" to match
CPython 3.14 wording: change the string for ParseErrorType::ExpectedKeywordParam
from "named arguments must follow bare *" to "named parameters must follow bare
*", and likewise replace "var-positional argument cannot have default value" and
"at least one argument must precede /" with "var-positional parameter cannot
have default value" and "at least one parameter must precede /" in their
corresponding ParseErrorType arms in vm_new.rs.
|
great! |
Summary by CodeRabbit
Bug Fixes
Refactor