RustPython version to 3.14#6718
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Python 3.14 bumps and implements broad PEP 649 deferred annotation support across compiler, symbol table, bytecode, VM runtime, and stdlib, plus related API, instruction, flag, exception initializer, bytes/fromhex, JIT, itertools, tests, and CI updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Compiler
participant SymbolTable
participant CodeObject
participant Runtime as VM
Note over Compiler,VM: Compile-time PEP 649 annotation handling
Compiler->>SymbolTable: enter_annotation_scope()
SymbolTable->>SymbolTable: create/push annotation_block
Compiler->>Compiler: compile annotations into closure
Compiler->>CodeObject: emit annotation closure (co)
Compiler->>SymbolTable: exit_annotation_scope()
SymbolTable->>SymbolTable: pop annotation_block
Compiler->>CodeObject: attach annotations metadata (conditional map / __annotate__)
sequenceDiagram
participant Frame
participant Mapping
participant Globals
Note over Frame,Globals: LoadFromDictOrGlobals lookup (runtime)
Frame->>Frame: execute LoadFromDictOrGlobals(idx)
Frame->>Mapping: pop mapping from stack
Frame->>Mapping: try mapping lookup (dict.get / mapping protocol)
alt found
Mapping-->>Frame: push value
else not found
Frame->>Globals: load_global_or_builtin(name)
Globals-->>Frame: push value or raise NameError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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. 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 |
|
Code has been automatically formatted The code in this PR has been formatted using:
git pull origin py-3.14 |
bacd767 to
fb3b617
Compare
whats_left.py
Outdated
| @@ -39,7 +39,7 @@ | |||
| sys.exit(f"whats_left.py must be run under CPython, got {implementation} instead") | |||
| if sys.version_info[:2] < (3, 13): | |||
There was a problem hiding this comment.
| if sys.version_info[:2] < (3, 13): | |
| if sys.version_info[:2] < (3, 14): |
50c4028 to
b036a08
Compare
|
failing tests: |
674a474 to
2089aff
Compare
|
okay! now all test passes |
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
crates/venvlauncher/src/main.rs (1)
133-133: Consider updating to Python314 for consistency.While the test logic doesn't depend on the specific version, this test still references Python313 while
test_read_homewas updated to Python314. For consistency across the test suite in this version upgrade PR, consider updating this reference as well.📝 Suggested update for consistency
- writeln!(file, "home=C:\\Python313").unwrap(); + writeln!(file, "home=C:\\Python314").unwrap();And update the corresponding assertion:
- assert_eq!(home, "C:\\Python313"); + assert_eq!(home, "C:\\Python314");scripts/fix_test.py (1)
61-67: Mutable class attribute bug:tests = []is shared across all instances.The
tests = []on line 63 is a class-level mutable default. AllTestResultinstances share the same list, so appending totest_results.testsinparse_results()mutates this shared list. If the function is called multiple times (e.g., in future refactoring), results from previous calls will persist.🐛 Proposed fix: Initialize in `__init__`
class TestResult: - tests_result: str = "" - tests = [] - stdout = "" + def __init__(self): + self.tests_result: str = "" + self.tests: list[Test] = [] + self.stdout: str = "" def __str__(self): return f"TestResult(tests_result={self.tests_result},tests={len(self.tests)})"crates/vm/src/version.rs (1)
131-132: Update PYC_MAGIC_NUMBER to 3627 for Python 3.14 compatibility.The
PYC_MAGIC_NUMBERvalue of2997is incorrect. CPython 3.14's PYC magic number is3627. This mismatch breaks.pycfile compatibility. Update the value to:pub const PYC_MAGIC_NUMBER: u16 = 3627;crates/vm/src/bytes_inner.rs (1)
424-469: Skip whitespace before reading the low nibble infromhex()to match CPython behavior.The current implementation skips ASCII whitespace before the top nibble (line 436) but not before the bottom nibble. This causes valid inputs like
'1 a'or'1\na'(space/newline between nibbles of a single byte) to incorrectly raise "non-hexadecimal number found" instead of being parsed successfully. CPython'sbytes.fromhex()skips all ASCII whitespace throughout the input, including between nibbles within a pair.Test case
' 1A\n2B\t30\v'demonstrates the expected behavior—whitespace should be transparent anywhere in the hex string. The suggested fix adds a whitespace-skipping loop before the bottom nibble read, correctly mirroring the top-nibble logic.🩹 Suggested fix
- let (i, b) = match iter.next() { - Some(val) => val, - None => break None, // odd number of hex digits - }; + let (i, b) = loop { + match iter.next() { + Some((i, b)) if is_py_ascii_whitespace(b) => continue, + Some(val) => break val, + None => break None, // odd number of hex digits + } + };crates/codegen/src/compile.rs (2)
2132-2203: Wirein_conditional_blockduring conditional statement compilation.
in_conditional_blockis consulted at line 5951 to determine whether annotations are conditional, but it's never toggled during conditional statement compilation in compile.rs. Annotations insideif/elif,for,while,with, andtryblocks need the flag set while compiling their bodies, and it should be restored afterward.Apply this pattern to all conditional statement paths: save the flag state, set it to
true, compile the body, then restore it:- self.compile_statements(body)?; + let prev = self.in_conditional_block; + self.in_conditional_block = true; + self.compile_statements(body)?; + self.in_conditional_block = prev;Affected locations in the
Stmt::Ifhandler (lines 2132-2203):
- Line 2147: first
ifbody- Line 2168:
elifbodies (inside the loop)- Line 2183: final
elif/elsebodyAlso apply to
compile_while(body),compile_for(body),compile_with(body), andcompile_try_statement(body).
4181-4294: Reset conditional-annotation index per class scope.
next_conditional_annotation_indexis compiler-global, butcompile_module_annotateenumerates class annotations from index 0 in its local scope. In nested classes, the global counter continues from where the outer class left off, causing a mismatch: the__annotate__function will look for indices [0, 1, ...] in__conditional_annotations__, but the outer class's compiler will have added indices [N, N+1, ...], making membership checks fail.Suggested fix
fn compile_class_body( &mut self, name: &str, body: &[Stmt], type_params: Option<&TypeParams>, firstlineno: u32, ) -> CompileResult<CodeObject> { + let prev_conditional_idx = self.next_conditional_annotation_index; + self.next_conditional_annotation_index = 0; // 1. Enter class scope let key = self.symbol_table_stack.len(); self.push_symbol_table()?; @@ -4290,7 +4292,9 @@ self.emit_return_value(); // Exit scope and return the code object - Ok(self.exit_scope()) + let code = self.exit_scope(); + self.next_conditional_annotation_index = prev_conditional_idx; + Ok(code) }
🤖 Fix all issues with AI agents
In `@crates/codegen/src/compile.rs`:
- Around line 5928-5966: The conditional that only records annotation indices
when is_module || self.in_conditional_block is causing unconditional class-scope
annotations to be skipped; in the block guarded by
self.current_symbol_table().has_conditional_annotations, remove the inner if
is_conditional check and always perform the logic that takes
self.next_conditional_annotation_index (increment it), loads
__conditional_annotations__, pushes the index constant, calls
Instruction::SetAdd and Instruction::PopTop so every annotation gets its index
recorded; update references: current_symbol_table(),
has_conditional_annotations, next_conditional_annotation_index,
"__conditional_annotations__", Instruction::SetAdd and Instruction::PopTop in
compile_module_annotate (the surrounding code shown).
In `@crates/compiler-core/src/marshal.rs`:
- Line 205: FORMAT_VERSION must be incremented to reflect that CodeFlags' wire
format widened from u16 to u32; update the crate's FORMAT_VERSION constant (the
one used for serialized code objects) so previously cached/serialized data is
invalidated, ensuring deserialization in marshal.rs (where CodeFlags is read
with read_u32 in the CodeFlags::from_bits_truncate call) won't be
misinterpreted; change the numeric FORMAT_VERSION value accordingly and run
tests to confirm no other serialization paths need version bumps.
In `@crates/jit/tests/common.rs`:
- Around line 66-130: The BUILD_MAP handling in
extract_annotations_from_annotate_code only treats the map value as a name
(code.names[val_idx]) and drops const-based forward-ref annotations; update the
loop that processes each key/value pair so after extracting (_val_is_const,
val_idx) you check both possibilities: if code.names.get(val_idx) exists use
that string as now, otherwise check code.constants[val_idx] for
ConstantData::Str and use its value; insert the resulting string into
annotations as StackValue::String so forward-ref annotations like "int" are
preserved.
In `@crates/vm/src/builtins/function.rs`:
- Around line 596-679: The __annotations__ implementation currently holds the
annotations and annotate mutexes while calling the annotate callable, risking
deadlock; change it to clone the annotate callable out of self.annotate.lock(),
drop the locks, then invoke annotate_fn.call(...) without holding either lock,
then reacquire self.annotations.lock() to validate/cache the returned dict (and
handle the downcast error) before returning it; ensure set___annotate__ and
set___annotations__ behavior remains the same (they can still clear the other
lock’s cached value) so concurrent setters update state correctly.
In `@crates/vm/src/builtins/object.rs`:
- Around line 618-648: Update the instance-aware resolution: in
is_getstate_overridden and object_getstate use obj.get_attr(identifier!(vm,
__getstate__)) (not obj.class().get_attr) so instance attributes are considered;
compare the resolved attribute to object_type.get_attr(identifier!(vm,
__getstate__)) and only treat it as “not overridden” when the resolved attribute
is the exact object.__getstate__; in object_getstate, if the resolved attribute
is None return a TypeError stating __getstate__ must be callable (or handle per
CPython semantics) instead of calling it, otherwise call the resolved callable
when it differs from object.__getstate__, and fallback to
object_getstate_default only when the resolved attribute equals
object.__getstate__.
In `@crates/vm/src/builtins/template.rs`:
- Around line 177-179: The __add__ method in template.rs is missing the
#[pymethod] attribute so it won't be exported to Python; add the #[pymethod]
annotation above fn __add__(&self, other: PyObjectRef, vm: &VirtualMachine) ->
PyResult<PyRef<Self>> so the dunder addition is exposed (leaving the body
calling self.concat(&other, vm) unchanged); verify other Python dunder methods
in the same impl also have #[pymethod] if they should be exported.
- Around line 308-341: The iterator stops early because when an empty string is
skipped you set from_strings to false but do not advance index; change the logic
in PyTemplateIter::next so that in the from_strings=true branch you first check
index >= template.strings.len() and return StopIteration, then get the item and
if it is an empty PyStr increment the atomic index (zelf.index.fetch_add(1,
Ordering::SeqCst)) and continue the loop without toggling from_strings; only
flip from_strings (zelf.from_strings.store(false, ...)) when you are actually
yielding a non-empty string. This ensures skipped empty strings advance the
index and remaining strings are correctly considered even when no interpolation
exists at that position.
In `@crates/vm/src/builtins/type.rs`:
- Around line 879-905: The code in set___annotate__ stores the new
__annotate_func__ but only clears __annotations_cache__ when the new value is
not Python None, leaving stale cached annotations when __annotate__ is set to
None; change the logic in set___annotate__ (function name) to always
remove/clear the __annotations_cache__ entry (identifier!(vm,
__annotations_cache__)) after inserting the new __annotate_func__
(identifier!(vm, __annotate_func__)), i.e. unconditionally call
attrs.swap_remove(identifier!(vm, __annotations_cache__)) so the cache is
cleared whether the new value is a callable or None.
In `@crates/vm/src/frame.rs`:
- Around line 1171-1179: The current Instruction::LoadFromDictOrGlobals code
swallows all errors by calling dict.get_item(name, vm).ok(), causing
non-KeyError exceptions from custom mappings to be ignored; change the logic to
match on dict.get_item(name, vm) and: if Ok(v) push that value; if Err(e)
determine whether e is a KeyError (use the VM's KeyError sentinel, e.g.
vm.ctx.exceptions.key_error or vm.is_instance check) and only in that case fall
back to calling load_global_or_builtin(name, vm)?; for any other Err(e)
propagate the error by returning Err(e) so non-KeyError exceptions are not
swallowed (affecting the Instruction::LoadFromDictOrGlobals path that uses
pop_value, push_value, name, dict, get_item and load_global_or_builtin).
In `@crates/vm/src/stdlib/io.rs`:
- Around line 4311-4347: The __setstate__ implementation is replacing the
internal buffer even when there are active exports, violating the "no resize
while exported" rule; before mutating/replacing the buffer (where you currently
do *zelf.buffer.write() = BufferedIO::new(Cursor::new(...))), call the buffer's
try_resizable(vm)? (or explicitly check zelf.buffer.exports > 0) and return an
appropriate error if resizing is disallowed, then only perform the
BufferedIO::new(Cursor::new(...)) assignment and subsequent seek when
try_resizable succeeds; keep references to __setstate__, try_resizable, buffer,
exports, and BufferedIO::new to locate the change.
- Around line 4105-4139: The __setstate__ implementation currently replaces the
internal buffer before checking whether the StringIO is closed, allowing
mutation of a closed object; modify it to call zelf.buffer(vm)? first and check
for closed() (or otherwise validate state) and return an error if closed before
performing any mutations (i.e., creating BufferedIO::new(Cursor::new(raw_bytes))
and writing into *zelf.buffer.write()); move the buffer replacement and seek
operations to after the closed check and use the buffer(vm)? accessor for
mutations to ensure a closed StringIO cannot be mutated.
In `@extra_tests/snippets/builtin_bool.py`:
- Around line 21-23: Replace the use of set([1, 2]) with the equivalent set
literal to satisfy ruff C405; locate the set([1, 2]) occurrence in
builtin_bool.py and change it to the set literal {1, 2} so the linter warning is
resolved.
In `@extra_tests/snippets/stdlib_typing.py`:
- Line 20: Update the regression test comment string "# Regression test for:
https://github.com/RustPython/RustPython/issues/6718" to reference the correct
issue(s) this test covers (e.g., replace 6718 with 6702 or 6543 as appropriate)
or clarify that it refers to this PR if that was intended; locate the comment in
extra_tests/snippets/stdlib_typing.py (the line containing the regression test
URL) and edit the URL/text accordingly so it points to the actual issue
number(s) being closed.
In `@extra_tests/snippets/syntax_assignment.py`:
- Around line 62-73: The linter flags __annotate__ as undefined; to silence F821
without changing behavior, add a local binding that reads it from globals before
use (e.g. assign __annotate__ = globals().get("__annotate__")) and then keep the
existing checks (assert callable(__annotate__), annotations = __annotate__(1),
etc.); this resolves the static undefined-name error while preserving runtime
semantics for the symbols __annotate__ and annotations.
🧹 Nitpick comments (8)
extra_tests/snippets/code_co_consts.py (1)
93-96: Consider addingselfparameter or@staticmethoddecorator to class methods.The methods are defined without
self, which works for this test since you're accessingcls_with_doc.methoddirectly from the class to inspect__code__. However, this is unconventional and could confuse readers. Consider either:
- Adding
selfto make them proper instance methods, or- Adding
@staticmethoddecorator to make the intent explicit♻️ Suggested clarification
class cls_with_doc: + `@staticmethod` def method(): """Method docstring""" return 1class cls_no_doc: + `@staticmethod` def method(): return 1Also applies to: 104-106
crates/vm/src/stdlib/thread.rs (2)
901-938: Consider retaining entries whentry_lockfails on inner state.When
inner.try_lock()fails at line 912-914, the entry is removed from the registry (return false). However, this thread handle still exists and may be used later. If someone callsjoin()on it, the thread won't be in the registry but also won't be properly marked asDone, potentially causing unexpected behavior.Consider retaining entries when the lock cannot be acquired:
♻️ Suggested change
// Try to lock the inner state - skip if we can't let Some(mut inner_guard) = inner.try_lock() else { - return false; + return true; // Keep entry - we couldn't safely process it };Alternatively, if removal is intentional, add a comment explaining that orphaned handles after fork are acceptable because the child process will create new handles for any threads it spawns.
309-321: Acknowledged: UTF-8 truncation concern.The existing TODO comment at lines 311-312 correctly identifies that truncating at byte 15 may split a multi-byte UTF-8 character. This is a pre-existing concern and not introduced by this PR, but worth tracking.
Would you like me to open an issue to track this UTF-8 boundary handling improvement, or generate a fix that properly handles multi-byte character boundaries?
extra_tests/snippets/stdlib_typing.py (1)
21-21: Consider moving import to top of file.Per PEP 8, imports should be at the top of the file. While the placement here groups the import with its related tests for clarity, moving it alongside the other
typingimport on line 2 would be more conventional.Suggested refactor
At the top of the file, consolidate imports:
from collections.abc import Awaitable, Callable -from typing import TypeVar +from typing import Optional, TypeVar, UnionThen remove line 21.
scripts/fix_test.py (1)
192-201: Redundant condition check.The condition on line 195 (
if test.result == "fail" or test.result == "error") is always true sinceparse_results()already filters to only include tests with these results (line 91-92). The check is harmless but could be simplified.♻️ Optional cleanup
# Collect failing tests (with deduplication for subtests) seen_tests = set() # Track (class_name, method_name) to avoid duplicates for test in tests.tests: - if test.result == "fail" or test.result == "error": - test_parts = path_to_test(test.path) - if len(test_parts) == 2: - test_key = tuple(test_parts) - if test_key not in seen_tests: - seen_tests.add(test_key) - print(f"Marking test: {test_parts[0]}.{test_parts[1]}") + test_parts = path_to_test(test.path) + if len(test_parts) == 2: + test_key = tuple(test_parts) + if test_key not in seen_tests: + seen_tests.add(test_key) + print(f"Marking test: {test_parts[0]}.{test_parts[1]}")crates/vm/src/builtins/int.rs (1)
290-294: Consider enabling or tracking this Python 3.14 feature.This commented-out code for negative-to-unsigned conversion errors is part of the 3.14 upgrade. If this behavior is required for Python 3.14 compliance, consider enabling it. Otherwise, convert to a proper
TODOwith an issue reference to track implementation.♻️ If intended for 3.14, enable the feature
- // Python 3.14+: ValueError for negative int to unsigned type - // if I::min_value() == I::zero() && self.as_bigint().sign() == Sign::Minus { - // return Err(vm.new_value_error("Cannot convert negative int".to_owned())); - // } + // Python 3.14+: ValueError for negative int to unsigned type + if I::min_value() == I::zero() && self.as_bigint().sign() == Sign::Minus { + return Err(vm.new_value_error("cannot convert negative int to unsigned".to_owned())); + }crates/vm/src/exceptions.rs (1)
1556-1583: Minor inconsistency with other initializers.The
PyImportErrorinitializer passes the originalargs(with kwargs intact) toPyBaseException::slot_init, whilePyAttributeErrorandPyNameErrorexplicitly create aFuncArgswith empty kwargs. Both work correctly sincePyBaseException::initonly usesargs.args, but this is inconsistent.Optional: Align with other initializers for consistency
dict.set_item("name_from", vm.unwrap_or_none(name_from), vm)?; - PyBaseException::slot_init(zelf, args, vm) + let base_args = FuncArgs::new(args.args.clone(), KwArgs::default()); + PyBaseException::slot_init(zelf, base_args, vm)crates/vm/src/builtins/template.rs (1)
297-303: Iterator__reduce__does not preserve iteration state.The reducer only stores the template reference, not the current
indexandfrom_stringsstate. Unpickling will restart iteration from the beginning. This may be intentional (many Python iterators behave this way), but it differs from iterators that preserve position.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (39)
Lib/_colorize.pyis excluded by!Lib/**Lib/_opcode_metadata.pyis excluded by!Lib/**Lib/annotationlib.pyis excluded by!Lib/**Lib/argparse.pyis excluded by!Lib/**Lib/calendar.pyis excluded by!Lib/**Lib/ensurepip/__init__.pyis excluded by!Lib/**Lib/ensurepip/_bundled/pip-25.3-py3-none-any.whlis excluded by!Lib/**Lib/ensurepip/_uninstall.pyis excluded by!Lib/**Lib/inspect.pyis excluded by!Lib/**Lib/site.pyis excluded by!Lib/**Lib/string/__init__.pyis excluded by!Lib/**Lib/string/templatelib.pyis excluded by!Lib/**Lib/test/test__colorize.pyis excluded by!Lib/**Lib/test/test_argparse.pyis excluded by!Lib/**Lib/test/test_builtin.pyis excluded by!Lib/**Lib/test/test_calendar.pyis excluded by!Lib/**Lib/test/test_copy.pyis excluded by!Lib/**Lib/test/test_csv.pyis excluded by!Lib/**Lib/test/test_descr.pyis excluded by!Lib/**Lib/test/test_ensurepip.pyis excluded by!Lib/**Lib/test/test_enum.pyis excluded by!Lib/**Lib/test/test_grammar.pyis excluded by!Lib/**Lib/test/test_itertools.pyis excluded by!Lib/**Lib/test/test_lzma.pyis excluded by!Lib/**Lib/test/test_memoryio.pyis excluded by!Lib/**Lib/test/test_module/__init__.pyis excluded by!Lib/**Lib/test/test_opcodes.pyis excluded by!Lib/**Lib/test/test_pickle.pyis excluded by!Lib/**Lib/test/test_pickletools.pyis excluded by!Lib/**Lib/test/test_site.pyis excluded by!Lib/**Lib/test/test_typing.pyis excluded by!Lib/**Lib/test/test_unittest/test_runner.pyis excluded by!Lib/**Lib/test/test_zlib.pyis excluded by!Lib/**Lib/test/typinganndata/ann_module.pyis excluded by!Lib/**Lib/typing.pyis excluded by!Lib/**Lib/venv/scripts/nt/venvlauncher.exeis excluded by!**/*.exe,!Lib/**Lib/venv/scripts/nt/venvlaunchert.exeis excluded by!**/*.exe,!Lib/**Lib/venv/scripts/nt/venvwlauncher.exeis excluded by!**/*.exe,!Lib/**Lib/venv/scripts/nt/venvwlaunchert.exeis excluded by!**/*.exe,!Lib/**
📒 Files selected for processing (62)
.cspell.dict/python-more.txt.github/copilot-instructions.md.github/workflows/ci.yaml.github/workflows/cron-ci.yaml.github/workflows/update-doc-db.ymlDEVELOPMENT.mdREADME.mdcrates/codegen/src/compile.rscrates/codegen/src/symboltable.rscrates/compiler-core/src/bytecode.rscrates/compiler-core/src/bytecode/instruction.rscrates/compiler-core/src/bytecode/oparg.rscrates/compiler-core/src/marshal.rscrates/derive-impl/src/pyclass.rscrates/jit/tests/common.rscrates/stdlib/src/hashlib.rscrates/stdlib/src/ssl/error.rscrates/venvlauncher/src/main.rscrates/vm/Lib/python_builtins/__reducelib.pycrates/vm/src/builtins/bytearray.rscrates/vm/src/builtins/bytes.rscrates/vm/src/builtins/code.rscrates/vm/src/builtins/function.rscrates/vm/src/builtins/int.rscrates/vm/src/builtins/interpolation.rscrates/vm/src/builtins/mod.rscrates/vm/src/builtins/module.rscrates/vm/src/builtins/object.rscrates/vm/src/builtins/singletons.rscrates/vm/src/builtins/template.rscrates/vm/src/builtins/type.rscrates/vm/src/bytes_inner.rscrates/vm/src/exception_group.rscrates/vm/src/exceptions.rscrates/vm/src/frame.rscrates/vm/src/stdlib/ast/expression.rscrates/vm/src/stdlib/ast/pyast.rscrates/vm/src/stdlib/ast/string.rscrates/vm/src/stdlib/io.rscrates/vm/src/stdlib/itertools.rscrates/vm/src/stdlib/thread.rscrates/vm/src/types/zoo.rscrates/vm/src/version.rscrates/vm/src/vm/context.rscrates/vm/src/vm/mod.rscrates/vm/src/vm/vm_new.rscrates/vm/src/vm/vm_ops.rsextra_tests/snippets/builtin_bool.pyextra_tests/snippets/builtin_bytearray.pyextra_tests/snippets/builtin_bytes.pyextra_tests/snippets/builtin_exceptions.pyextra_tests/snippets/builtin_int.pyextra_tests/snippets/builtins_module.pyextra_tests/snippets/code_co_consts.pyextra_tests/snippets/example_interactive.pyextra_tests/snippets/stdlib_itertools.pyextra_tests/snippets/stdlib_socket.pyextra_tests/snippets/stdlib_typing.pyextra_tests/snippets/syntax_assignment.pyextra_tests/snippets/syntax_function2.pyscripts/fix_test.pywhats_left.py
💤 Files with no reviewable changes (2)
- extra_tests/snippets/stdlib_itertools.py
- crates/vm/Lib/python_builtins/__reducelib.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style usingcargo fmtto format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/stdlib/src/ssl/error.rscrates/compiler-core/src/bytecode.rscrates/vm/src/builtins/bytearray.rscrates/compiler-core/src/bytecode/oparg.rscrates/stdlib/src/hashlib.rscrates/vm/src/builtins/singletons.rscrates/vm/src/builtins/mod.rscrates/vm/src/builtins/bytes.rscrates/vm/src/stdlib/ast/expression.rscrates/vm/src/builtins/code.rscrates/vm/src/exception_group.rscrates/vm/src/stdlib/ast/pyast.rscrates/vm/src/vm/context.rscrates/jit/tests/common.rscrates/venvlauncher/src/main.rscrates/vm/src/types/zoo.rscrates/vm/src/version.rscrates/vm/src/builtins/object.rscrates/vm/src/stdlib/thread.rscrates/vm/src/bytes_inner.rscrates/vm/src/vm/vm_ops.rscrates/compiler-core/src/marshal.rscrates/vm/src/vm/mod.rscrates/vm/src/builtins/interpolation.rscrates/vm/src/vm/vm_new.rscrates/compiler-core/src/bytecode/instruction.rscrates/vm/src/builtins/type.rscrates/vm/src/frame.rscrates/vm/src/builtins/template.rscrates/derive-impl/src/pyclass.rscrates/vm/src/builtins/module.rscrates/vm/src/builtins/int.rscrates/vm/src/stdlib/ast/string.rscrates/codegen/src/symboltable.rscrates/vm/src/stdlib/itertools.rscrates/vm/src/stdlib/io.rscrates/codegen/src/compile.rscrates/vm/src/builtins/function.rscrates/vm/src/exceptions.rs
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Follow PEP 8 style for custom Python code
Use ruff for linting Python code
Files:
extra_tests/snippets/example_interactive.pyextra_tests/snippets/stdlib_typing.pyextra_tests/snippets/builtin_bytearray.pyextra_tests/snippets/stdlib_socket.pyextra_tests/snippets/builtin_int.pyextra_tests/snippets/syntax_function2.pyextra_tests/snippets/builtins_module.pyextra_tests/snippets/syntax_assignment.pywhats_left.pyextra_tests/snippets/builtin_bytes.pyextra_tests/snippets/builtin_bool.pyextra_tests/snippets/builtin_exceptions.pyscripts/fix_test.pyextra_tests/snippets/code_co_consts.py
🧠 Learnings (13)
📚 Learning: 2026-01-14T14:52:10.778Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Applied to files:
.github/copilot-instructions.mdcrates/vm/src/builtins/mod.rscrates/vm/src/vm/mod.rscrates/vm/src/frame.rscrates/vm/src/builtins/module.rscrates/vm/src/builtins/function.rscrates/vm/src/exceptions.rs
📚 Learning: 2026-01-14T14:52:10.779Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.779Z
Learning: When comparing behavior with CPython, use `python` command to explicitly run CPython and `cargo run -- script.py` to run RustPython
Applied to files:
.github/copilot-instructions.mdREADME.mdcrates/vm/src/vm/mod.rs
📚 Learning: 2026-01-14T14:52:10.779Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.779Z
Learning: In most cases, Python code should not be edited; bug fixes should be made through Rust code modifications only
Applied to files:
.github/copilot-instructions.mdcrates/vm/src/vm/mod.rsextra_tests/snippets/builtin_exceptions.py
📚 Learning: 2025-12-09T08:46:58.660Z
Learnt from: youknowone
Repo: RustPython/RustPython PR: 6358
File: crates/vm/src/exception_group.rs:173-185
Timestamp: 2025-12-09T08:46:58.660Z
Learning: In crates/vm/src/exception_group.rs, the derive() method intentionally always creates a BaseExceptionGroup instance rather than preserving the original exception class type. This is a deliberate design decision that differs from CPython's behavior.
Applied to files:
crates/vm/src/exception_group.rscrates/derive-impl/src/pyclass.rscrates/vm/src/exceptions.rs
📚 Learning: 2026-01-14T14:52:10.778Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to Lib/**/*.py : Minimize modifications to CPython standard library files in the Lib/ directory; these files should be edited very conservatively and modifications should be minimal and only to work around RustPython limitations
Applied to files:
crates/vm/src/vm/mod.rsextra_tests/snippets/builtin_exceptions.py
📚 Learning: 2026-01-14T14:52:10.778Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to Lib/**/*.py : Add a `# TODO: RUSTPYTHON` comment when modifications are made to Lib/ directory files
Applied to files:
crates/vm/src/vm/mod.rsextra_tests/snippets/builtin_exceptions.py
📚 Learning: 2026-01-14T14:52:10.779Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.779Z
Learning: Do not edit the Lib/ directory directly except for copying files from CPython to work around RustPython limitations
Applied to files:
crates/vm/src/vm/mod.rs
📚 Learning: 2026-01-14T14:52:10.779Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.779Z
Learning: Use `cargo run -- script.py` instead of `python script.py` when testing Python code with RustPython
Applied to files:
crates/vm/src/vm/mod.rs
📚 Learning: 2026-01-14T14:52:10.779Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.779Z
Learning: Run a full clean build when modifying bytecode instructions using: `rm -r target/debug/build/rustpython-* && find . | grep -E "\.pyc$" | xargs rm -r`
Applied to files:
crates/vm/src/vm/mod.rs
📚 Learning: 2026-01-14T14:52:10.779Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.779Z
Learning: Run `./whats_left.py` to get a list of unimplemented methods when looking for contribution opportunities
Applied to files:
whats_left.py
📚 Learning: 2026-01-14T14:52:10.778Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to Lib/test/**/*.py : Use `unittest.skip("TODO: RustPython <reason>")` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment when marking tests in Lib/ that cannot run
Applied to files:
extra_tests/snippets/builtin_exceptions.py
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using `#ifdef` checks rather than providing fallback values for other platforms.
Applied to files:
extra_tests/snippets/builtin_exceptions.py
📚 Learning: 2026-01-14T14:52:10.778Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-14T14:52:10.778Z
Learning: Applies to **/test*.py : Only remove `unittest.expectedFailure` decorators and upper TODO comments from tests when tests actually pass, or add these decorators when tests cannot be fixed
Applied to files:
scripts/fix_test.py
🧬 Code graph analysis (24)
crates/vm/src/builtins/bytearray.rs (1)
crates/vm/src/bytes_inner.rs (3)
fromhex(424-470)string(474-474)fromhex_object(473-487)
crates/vm/src/builtins/mod.rs (1)
crates/vm/src/stdlib/sre.rs (1)
template(140-165)
extra_tests/snippets/example_interactive.py (1)
crates/vm/src/builtins/code.rs (1)
co_consts(604-607)
crates/vm/src/builtins/bytes.rs (1)
crates/vm/src/bytes_inner.rs (3)
fromhex(424-470)string(474-474)fromhex_object(473-487)
crates/vm/src/stdlib/ast/expression.rs (1)
crates/vm/src/stdlib/ast/string.rs (1)
tstring_to_object(602-618)
crates/vm/src/vm/context.rs (3)
crates/vm/src/builtins/function.rs (2)
__annotate__(653-658)__annotations__(596-626)crates/vm/src/builtins/module.rs (2)
__annotate__(187-198)__annotations__(224-259)crates/vm/src/builtins/type.rs (2)
__annotate__(855-877)__annotations__(909-949)
crates/vm/src/types/zoo.rs (2)
crates/vm/src/builtins/template.rs (1)
iter(234-236)crates/vm/src/builtins/interpolation.rs (1)
init(205-207)
crates/vm/src/builtins/object.rs (3)
crates/vm/src/builtins/tuple.rs (4)
class(41-43)class(527-529)len(194-196)__getnewargs__(352-362)crates/vm/src/builtins/int.rs (3)
__getnewargs__(566-568)a(629-629)a(665-665)crates/vm/src/protocol/mapping.rs (1)
items(182-188)
crates/vm/src/bytes_inner.rs (1)
crates/vm/src/builtins/bytes.rs (2)
bytes(129-129)fromhex(319-323)
crates/vm/src/vm/vm_ops.rs (2)
crates/vm/src/protocol/mapping.rs (1)
slots(108-110)crates/vm/src/protocol/sequence.rs (1)
slots(134-136)
extra_tests/snippets/builtin_bytearray.py (4)
crates/vm/src/builtins/bytearray.rs (1)
fromhex(325-330)crates/vm/src/builtins/bytes.rs (1)
fromhex(319-323)crates/vm/src/bytes_inner.rs (1)
fromhex(424-470)extra_tests/snippets/testutils.py (1)
assert_raises(5-12)
crates/vm/src/builtins/interpolation.rs (1)
crates/vm/src/builtins/template.rs (10)
class(29-31)class(280-282)new(35-40)new(286-292)py_new(46-92)s(149-149)s(154-154)cmp(213-230)other(123-123)init(343-346)
crates/compiler-core/src/bytecode/instruction.rs (1)
crates/compiler-core/src/bytecode/oparg.rs (2)
from(25-27)from(68-70)
crates/vm/src/frame.rs (1)
crates/vm/src/builtins/template.rs (2)
interpolations(103-105)strings(98-100)
extra_tests/snippets/builtins_module.py (1)
crates/vm/src/stdlib/builtins.rs (1)
isinstance(527-529)
extra_tests/snippets/syntax_assignment.py (1)
extra_tests/snippets/testutils.py (1)
assert_raises(5-12)
whats_left.py (2)
crates/vm/src/stdlib/sys.rs (2)
version_info(1159-1161)implementation(495-507)crates/vm/src/stdlib/builtins.rs (1)
hasattr(426-434)
extra_tests/snippets/builtin_bytes.py (3)
crates/vm/src/builtins/bytes.rs (2)
bytes(129-129)fromhex(319-323)crates/vm/src/builtins/bytearray.rs (1)
fromhex(325-330)crates/vm/src/bytes_inner.rs (1)
fromhex(424-470)
crates/vm/src/stdlib/ast/string.rs (1)
crates/vm/src/stdlib/ast/expression.rs (16)
ast_to_object(11-53)ast_to_object(148-165)ast_to_object(191-208)ast_to_object(234-254)ast_to_object(285-302)ast_to_object(327-344)ast_to_object(370-390)ast_to_object(421-445)ast_to_object(477-491)ast_to_object(511-528)ast_to_object(554-571)ast_to_object(597-617)ast_to_object(648-666)ast_to_object(694-708)ast_to_object(728-742)ast_to_object(761-775)
extra_tests/snippets/builtin_bool.py (1)
extra_tests/snippets/testutils.py (1)
assert_raises(5-12)
crates/vm/src/stdlib/io.rs (1)
crates/vm/src/builtins/object.rs (2)
__getstate__(287-290)value(435-435)
crates/codegen/src/compile.rs (2)
crates/codegen/src/symboltable.rs (4)
new(80-98)new(204-211)new(811-827)enter_annotation_scope(887-919)crates/vm/src/builtins/function.rs (3)
new(59-110)new(903-905)new(1025-1029)
crates/vm/src/builtins/function.rs (2)
crates/vm/src/builtins/type.rs (8)
module(1866-1866)__annotations__(909-949)obj(208-208)obj(652-652)obj(1192-1192)class(137-139)name(592-597)__annotate__(855-877)crates/vm/src/builtins/module.rs (6)
new(72-77)__annotations__(224-259)class(58-60)name(120-126)name(125-125)__annotate__(187-198)
extra_tests/snippets/code_co_consts.py (1)
crates/vm/src/builtins/code.rs (2)
co_consts(604-607)co_flags(631-633)
🪛 Flake8 (7.3.0)
extra_tests/snippets/syntax_assignment.py
[error] 70-70: undefined name 'annotate'
(F821)
[error] 71-71: undefined name 'annotate'
(F821)
extra_tests/snippets/code_co_consts.py
[error] 87-87: do not assign a lambda expression, use a def
(E731)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| .collect::<Result<Box<[(SourceLocation, SourceLocation)]>>>()?; | ||
|
|
||
| let flags = CodeFlags::from_bits_truncate(rdr.read_u16()?); | ||
| let flags = CodeFlags::from_bits_truncate(rdr.read_u32()?); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -e rs marshal | head -20Repository: RustPython/RustPython
Length of output: 132
🏁 Script executed:
cat -n crates/compiler-core/src/marshal.rs | head -50Repository: RustPython/RustPython
Length of output: 1727
🏁 Script executed:
cat -n crates/compiler-core/src/marshal.rs | sed -n '190,220p'Repository: RustPython/RustPython
Length of output: 1461
🏁 Script executed:
cat -n crates/compiler-core/src/marshal.rs | sed -n '650,675p'Repository: RustPython/RustPython
Length of output: 1226
🏁 Script executed:
rg "FORMAT_VERSION" crates/compiler-core/src/marshal.rs -B 2 -A 2Repository: RustPython/RustPython
Length of output: 145
🏁 Script executed:
rg "CodeFlags" crates/compiler-core/src/marshal.rs -B 2 -A 2 | head -60Repository: RustPython/RustPython
Length of output: 246
🏁 Script executed:
rg "struct CodeFlags|enum CodeFlags|type CodeFlags" crates/compiler-core/src/ -B 2 -A 5Repository: RustPython/RustPython
Length of output: 619
🏁 Script executed:
rg "bitflags!" crates/compiler-core/src/marshal.rs -B 2 -A 10Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
rg -l "CodeFlags" crates/compiler-core/src/ | head -10Repository: RustPython/RustPython
Length of output: 137
🏁 Script executed:
git log --oneline crates/compiler-core/src/bytecode.rs | head -20Repository: RustPython/RustPython
Length of output: 115
🏁 Script executed:
git log --oneline crates/compiler-core/src/marshal.rs | head -20Repository: RustPython/RustPython
Length of output: 115
🏁 Script executed:
git log -p --all -- crates/compiler-core/src/bytecode.rs | grep -A 10 -B 10 "CodeFlags:" | head -50Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
git log -p --all -- crates/compiler-core/src/marshal.rs | grep -A 5 -B 5 "FORMAT_VERSION" | head -60Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
cat -n crates/compiler-core/src/bytecode.rs | grep -A 20 "bitflags!"Repository: RustPython/RustPython
Length of output: 811
🏁 Script executed:
fd -i changelog -o -i "CHANGELOG" -o -i "release" | head -5Repository: RustPython/RustPython
Length of output: 181
🏁 Script executed:
git log --all --oneline | wc -lRepository: RustPython/RustPython
Length of output: 66
🏁 Script executed:
git show f0edec00e --stat | head -20Repository: RustPython/RustPython
Length of output: 1141
🏁 Script executed:
rg "cache|Cache" crates/compiler-core/src/marshal.rs -iRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
rg "compat|Compat|backwards|backward" crates/compiler-core/src/marshal.rs -iRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
git log --all --oneline -- "*/marshal.rs" | head -30Repository: RustPython/RustPython
Length of output: 167
🏁 Script executed:
git show HEAD:crates/compiler-core/src/bytecode.rs | grep -A 15 "bitflags!"Repository: RustPython/RustPython
Length of output: 580
🏁 Script executed:
git show 380fa39eb --stat | grep -E "(marshal|bytecode)"Repository: RustPython/RustPython
Length of output: 654
🏁 Script executed:
git show 380fa39eb -- crates/compiler-core/src/bytecode.rs | grep -A 20 -B 5 "CodeFlags"Repository: RustPython/RustPython
Length of output: 2802
🏁 Script executed:
git show 380fa39eb -- crates/compiler-core/src/marshal.rs | head -100Repository: RustPython/RustPython
Length of output: 2675
🏁 Script executed:
git log --all --oneline --follow -- "crates/compiler-core/src/bytecode.rs" | head -20Repository: RustPython/RustPython
Length of output: 167
🏁 Script executed:
git show 380fa39eb:crates/compiler-core/src/bytecode.rs | grep -A 10 "pub struct CodeFlags"Repository: RustPython/RustPython
Length of output: 332
🏁 Script executed:
git diff 380fa39eb HEAD -- crates/compiler-core/src/bytecode.rs | grep -A 10 -B 10 "CodeFlags"Repository: RustPython/RustPython
Length of output: 885
🏁 Script executed:
git log --all --oneline --follow -- "crates/compiler-core/src/bytecode.rs" | grep -v "Auto-format"Repository: RustPython/RustPython
Length of output: 116
🏁 Script executed:
git log --all --reverse --oneline | head -20Repository: RustPython/RustPython
Length of output: 167
🏁 Script executed:
git show HEAD:crates/compiler-core/src/bytecode.rs | grep "pub const FORMAT_VERSION"Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
git show 380fa39eb:crates/compiler-core/src/bytecode.rs | grep "pub const FORMAT_VERSION"Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
git log --all --oneline -p -- crates/compiler-core/src/bytecode.rs crates/compiler-core/src/marshal.rs | grep -A 2 -B 2 "FORMAT_VERSION" | head -30Repository: RustPython/RustPython
Length of output: 239
🏁 Script executed:
git show 380fa39eb:crates/compiler-core/src/marshal.rs | sed -n '200,210p'Repository: RustPython/RustPython
Length of output: 382
🏁 Script executed:
git show 380fa39eb:crates/compiler-core/src/bytecode.rs | grep -A 8 "pub struct CodeFlags"Repository: RustPython/RustPython
Length of output: 308
🏁 Script executed:
rg "FORMAT_VERSION" crates/ -B 2 -A 2Repository: RustPython/RustPython
Length of output: 581
🏁 Script executed:
git show 380fa39eb:crates/compiler-core/src/marshal.rs | sed -n '660,670p'Repository: RustPython/RustPython
Length of output: 356
🏁 Script executed:
rg "read_u16|write_u16" crates/compiler-core/src/marshal.rsRepository: RustPython/RustPython
Length of output: 146
🏁 Script executed:
rg "test_marshal|test.*code.*object" --type rust | head -20Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
rg "pub fn dump|pub fn load" crates/compiler-core/src/marshal.rsRepository: RustPython/RustPython
Length of output: 47
Bump FORMAT_VERSION: CodeFlags wire format changed from u16 to u32.
CodeFlags was widened from u16 to u32 between commits, but FORMAT_VERSION remains at 4. This creates a backward compatibility issue: any pre-existing serialized code objects will misalign on deserialization. Increment FORMAT_VERSION to invalidate stale cached data.
🤖 Prompt for AI Agents
In `@crates/compiler-core/src/marshal.rs` at line 205, FORMAT_VERSION must be
incremented to reflect that CodeFlags' wire format widened from u16 to u32;
update the crate's FORMAT_VERSION constant (the one used for serialized code
objects) so previously cached/serialized data is invalidated, ensuring
deserialization in marshal.rs (where CodeFlags is read with read_u32 in the
CodeFlags::from_bits_truncate call) won't be misinterpreted; change the numeric
FORMAT_VERSION value accordingly and run tests to confirm no other serialization
paths need version bumps.
| /// Extract annotations from an annotate function's bytecode. | ||
| /// The annotate function uses BUILD_MAP with key-value pairs loaded before it. | ||
| /// Keys are parameter names (from LOAD_CONST), values are type names (from LOAD_NAME/LOAD_GLOBAL). | ||
| fn extract_annotations_from_annotate_code(code: &CodeObject) -> HashMap<String, StackValue> { | ||
| let mut annotations = HashMap::new(); | ||
| let mut stack: Vec<(bool, usize)> = Vec::new(); // (is_const, index) | ||
| let mut op_arg_state = OpArgState::default(); | ||
|
|
||
| for &word in code.instructions.iter() { | ||
| let (instruction, arg) = op_arg_state.get(word); | ||
|
|
||
| match instruction { | ||
| Instruction::LoadConst { idx } => { | ||
| stack.push((true, idx.get(arg) as usize)); | ||
| } | ||
| Instruction::LoadName(idx) | Instruction::LoadGlobal(idx) => { | ||
| stack.push((false, idx.get(arg) as usize)); | ||
| } | ||
| Instruction::BuildMap { size, .. } => { | ||
| let count = size.get(arg) as usize; | ||
| // Stack has key-value pairs in order: k1, v1, k2, v2, ... | ||
| // So we need count * 2 items from the stack | ||
| let start = stack.len().saturating_sub(count * 2); | ||
| let pairs: Vec<_> = stack.drain(start..).collect(); | ||
|
|
||
| for chunk in pairs.chunks(2) { | ||
| if chunk.len() == 2 { | ||
| let (key_is_const, key_idx) = chunk[0]; | ||
| let (_val_is_const, val_idx) = chunk[1]; | ||
|
|
||
| // Key should be a const string (parameter name) | ||
| if key_is_const | ||
| && let ConstantData::Str { value } = &code.constants[key_idx] | ||
| { | ||
| let param_name = value.to_string_lossy().into_owned(); | ||
| // Value should be a name (type name) | ||
| if let Some(type_name) = code.names.get(val_idx) { | ||
| annotations | ||
| .insert(param_name, StackValue::String(type_name.clone())); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| // Return after processing BUILD_MAP - we got our annotations | ||
| return annotations; | ||
| } | ||
| Instruction::Resume { .. } | ||
| | Instruction::LoadFast(_) | ||
| | Instruction::CompareOp { .. } | ||
| | Instruction::ExtendedArg => { | ||
| // Ignore these instructions for annotation extraction | ||
| } | ||
| Instruction::ReturnValue | Instruction::ReturnConst { .. } => { | ||
| // End of function - return what we have | ||
| return annotations; | ||
| } | ||
| _ => { | ||
| // For other instructions, clear the stack tracking as we don't understand the effect | ||
| stack.clear(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| annotations | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd crates/jit && find . -name "common.rs" -type fRepository: RustPython/RustPython
Length of output: 82
🏁 Script executed:
cd crates/jit && cat -n tests/common.rs | head -150 | tail -100Repository: RustPython/RustPython
Length of output: 4784
🏁 Script executed:
cd crates/jit && grep -n "StackValue" tests/common.rs | head -20Repository: RustPython/RustPython
Length of output: 1448
🏁 Script executed:
cd crates/jit && grep -n "annotations" tests/common.rs | head -30Repository: RustPython/RustPython
Length of output: 1302
🏁 Script executed:
cd crates/jit && grep -B5 -A10 "extract_annotations_from_annotate_code" tests/common.rs | head -40Repository: RustPython/RustPython
Length of output: 1727
🏁 Script executed:
cd crates/jit && grep -r "annotations" tests/ --include="*.py" | head -20Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
cd crates/jit && grep -B10 -A10 "def.*->.*\"" tests/ --include="*.py" 2>/dev/null | head -40Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
cd crates/jit && find tests -name "*.py" -type f | head -10Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
cd crates/jit && grep -A20 "def .*annotate" tests/common.rsRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
cd crates/jit && python3 << 'EOF'
# Verify that forward refs in Python annotations are LOAD_CONST
import dis
import io
code_str = '''
def annotate():
return {
'x': "int",
'y': str
}
'''
exec(code_str)
print("Bytecode for forward ref ('int' string) vs reference (str type):")
print("=" * 60)
dis.dis(annotate)
EOFRepository: RustPython/RustPython
Length of output: 464
🏁 Script executed:
cd crates/jit && grep -r "def.*:.*\"" tests/common.rsRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
cd crates/jit && sed -n '200,250p' tests/common.rsRepository: RustPython/RustPython
Length of output: 2762
🏁 Script executed:
cd crates/jit && grep -B5 -A15 "SetFunctionAttribute" tests/common.rsRepository: RustPython/RustPython
Length of output: 1218
🏁 Script executed:
cd crates/jit && python3 << 'EOF'
# Verify StackValue can hold strings from both const and names
import json
# Test case 1: Forward ref annotation (LOAD_CONST string)
test_forward_ref = """
def annotate():
return {
'param': "ForwardRef",
}
"""
# Test case 2: Type reference annotation (LOAD_NAME/LOAD_GLOBAL)
test_type_ref = """
def annotate():
return {
'param': str,
}
"""
print("Test 1 - Forward ref (string literal):")
print(test_forward_ref)
print("\nTest 2 - Type reference (name):")
print(test_type_ref)
print("\nBoth should work with proper annotation extraction.")
EOFRepository: RustPython/RustPython
Length of output: 316
Handle string-literal annotation values too.
Forward-ref style annotations like "int" are LOAD_CONST values; the current code only checks code.names for values, silently dropping any const-based annotations. This causes forward refs to be ignored, resulting in missing annotations.
🛠️ Suggested fix
- let (_val_is_const, val_idx) = chunk[1];
+ let (val_is_const, val_idx) = chunk[1];
// Key should be a const string (parameter name)
if key_is_const
&& let ConstantData::Str { value } = &code.constants[key_idx]
{
let param_name = value.to_string_lossy().into_owned();
- // Value should be a name (type name)
- if let Some(type_name) = code.names.get(val_idx) {
- annotations
- .insert(param_name, StackValue::String(type_name.clone()));
- }
+ // Value can be a name or a const string (forward ref)
+ let type_name = if val_is_const {
+ match &code.constants[val_idx] {
+ ConstantData::Str { value } => {
+ Some(value.to_string_lossy().into_owned())
+ }
+ _ => None,
+ }
+ } else {
+ code.names.get(val_idx).cloned()
+ };
+ if let Some(type_name) = type_name {
+ annotations.insert(param_name, StackValue::String(type_name));
+ }🤖 Prompt for AI Agents
In `@crates/jit/tests/common.rs` around lines 66 - 130, The BUILD_MAP handling in
extract_annotations_from_annotate_code only treats the map value as a name
(code.names[val_idx]) and drops const-based forward-ref annotations; update the
loop that processes each key/value pair so after extracting (_val_is_const,
val_idx) you check both possibilities: if code.names.get(val_idx) exists use
that string as now, otherwise check code.constants[val_idx] for
ConstantData::Str and use its value; insert the resulting string into
annotations as StackValue::String so forward-ref annotations like "int" are
preserved.
| #[pymethod] | ||
| fn __setstate__(zelf: PyRef<Self>, state: PyTupleRef, vm: &VirtualMachine) -> PyResult<()> { | ||
| if state.len() != 4 { | ||
| return Err(vm.new_type_error(format!( | ||
| "__setstate__ argument should be 4-tuple, got {}", | ||
| state.len() | ||
| ))); | ||
| } | ||
|
|
||
| let content: PyStrRef = state[0].clone().try_into_value(vm)?; | ||
| // state[1] is newline - TODO: use when newline handling is implemented | ||
| let pos: u64 = state[2].clone().try_into_value(vm)?; | ||
| let dict = &state[3]; | ||
|
|
||
| // Set content | ||
| let raw_bytes = content.as_bytes().to_vec(); | ||
| *zelf.buffer.write() = BufferedIO::new(Cursor::new(raw_bytes)); | ||
|
|
||
| // Set position | ||
| 4D1C | zelf.buffer(vm)? | |
| .seek(SeekFrom::Start(pos)) | ||
| .map_err(|err| os_err(vm, err))?; | ||
|
|
||
| // Set __dict__ if provided | ||
| if !vm.is_none(dict) { | ||
| let dict_ref: PyRef<PyDict> = dict.clone().try_into_value(vm)?; | ||
| if let Some(obj_dict) = zelf.as_object().dict() { | ||
| obj_dict.clear(); | ||
| for (key, value) in dict_ref.into_iter() { | ||
| obj_dict.set_item(&*key, value, vm)?; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(()) |
There was a problem hiding this comment.
Avoid mutating closed StringIO during setstate.
__setstate__ replaces the buffer before any closed check, so a closed object can be mutated even though an error is later raised. Fail fast (or use buffer(vm)? for the mutation) to keep state consistent.
🔧 Suggested fix
@@
#[pymethod]
fn __setstate__(zelf: PyRef<Self>, state: PyTupleRef, vm: &VirtualMachine) -> PyResult<()> {
+ if zelf.closed.load() {
+ return Err(io_closed_error(vm));
+ }
if state.len() != 4 {
return Err(vm.new_type_error(format!(
"__setstate__ argument should be 4-tuple, got {}",
state.len()
)));
}
@@
let dict = &state[3];
// Set content
let raw_bytes = content.as_bytes().to_vec();
- *zelf.buffer.write() = BufferedIO::new(Cursor::new(raw_bytes));
+ let mut buffer = zelf.buffer(vm)?;
+ *buffer = BufferedIO::new(Cursor::new(raw_bytes));
// Set position
- zelf.buffer(vm)?
- .seek(SeekFrom::Start(pos))
+ buffer
+ .seek(SeekFrom::Start(pos))
.map_err(|err| os_err(vm, err))?;
+ drop(buffer);
@@
Ok(())
}📝 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.
| #[pymethod] | |
| fn __setstate__(zelf: PyRef<Self>, state: PyTupleRef, vm: &VirtualMachine) -> PyResult<()> { | |
| if state.len() != 4 { | |
| return Err(vm.new_type_error(format!( | |
| "__setstate__ argument should be 4-tuple, got {}", | < BEA4 /tr>|
| state.len() | |
| ))); | |
| } | |
| let content: PyStrRef = state[0].clone().try_into_value(vm)?; | |
| // state[1] is newline - TODO: use when newline handling is implemented | |
| let pos: u64 = state[2].clone().try_into_value(vm)?; | |
| let dict = &state[3]; | |
| // Set content | |
| let raw_bytes = content.as_bytes().to_vec(); | |
| *zelf.buffer.write() = BufferedIO::new(Cursor::new(raw_bytes)); | |
| // Set position | |
| zelf.buffer(vm)? | |
| .seek(SeekFrom::Start(pos)) | |
| .map_err(|err| os_err(vm, err))?; | |
| // Set __dict__ if provided | |
| if !vm.is_none(dict) { | |
| let dict_ref: PyRef<PyDict> = dict.clone().try_into_value(vm)?; | |
| if let Some(obj_dict) = zelf.as_object().dict() { | |
| obj_dict.clear(); | |
| for (key, value) in dict_ref.into_iter() { | |
| obj_dict.set_item(&*key, value, vm)?; | |
| } | |
| } | |
| } | |
| Ok(()) | |
| #[pymethod] | |
| fn __setstate__(zelf: PyRef<Self>, state: PyTupleRef, vm: &VirtualMachine) -> PyResult<()> { | |
| if zelf.closed.load() { | |
| return Err(io_closed_error(vm)); | |
| } | |
| if state.len() != 4 { | |
| return Err(vm.new_type_error(format!( | |
| "__setstate__ argument should be 4-tuple, got {}", | |
| state.len() | |
| ))); | |
| } | |
| let content: PyStrRef = state[0].clone().try_into_value(vm)?; | |
| // state[1] is newline - TODO: use when newline handling is implemented | |
| let pos: u64 = state[2].clone().try_into_value(vm)?; | |
| let dict = &state[3]; | |
| // Set content | |
| let raw_bytes = content.as_bytes().to_vec(); | |
| let mut buffer = zelf.buffer(vm)?; | |
| *buffer = BufferedIO::new(Cursor::new(raw_bytes)); | |
| // Set position | |
| buffer | |
| .seek(SeekFrom::Start(pos)) | |
| .map_err(|err| os_err(vm, err))?; | |
| drop(buffer); | |
| // Set __dict__ if provided | |
| if !vm.is_none(dict) { | |
| let dict_ref: PyRef<PyDict> = dict.clone().try_into_value(vm)?; | |
| if let Some(obj_dict) = zelf.as_object().dict() { | |
| obj_dict.clear(); | |
| for (key, value) in dict_ref.into_iter() { | |
| obj_dict.set_item(&*key, value, vm)?; | |
| } | |
| } | |
| } | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In `@crates/vm/src/stdlib/io.rs` around lines 4105 - 4139, The __setstate__
implementation currently replaces the internal buffer before checking whether
the StringIO is closed, allowing mutation of a closed object; modify it to call
zelf.buffer(vm)? first and check for closed() (or otherwise validate state) and
return an error if closed before performing any mutations (i.e., creating
BufferedIO::new(Cursor::new(raw_bytes)) and writing into *zelf.buffer.write());
move the buffer replacement and seek operations to after the closed check and
use the buffer(vm)? accessor for mutations to ensure a closed StringIO cannot be
mutated.
| #[pymethod] | ||
| fn __setstate__(zelf: PyRef<Self>, state: PyTupleRef, vm: &VirtualMachine) -> PyResult<()> { | ||
| if zelf.closed.load() { | ||
| return Err(vm.new_value_error("__setstate__ on closed file")); | ||
| } | ||
| if state.len() != 3 { | ||
| return Err(vm.new_type_error(format!( | ||
| "__setstate__ argument should be 3-tuple, got {}", | ||
| state.len() | ||
| ))); | ||
| } | ||
|
|
||
| let content: PyBytesRef = state[0].clone().try_into_value(vm)?; | ||
| let pos: u64 = state[1].clone().try_into_value(vm)?; | ||
| let dict = &state[2]; | ||
|
|
||
| // Set content | ||
| *zelf.buffer.write() = BufferedIO::new(Cursor::new(content.as_bytes().to_vec())); | ||
|
|
||
| // Set position | ||
| zelf.buffer(vm)? | ||
| .seek(SeekFrom::Start(pos)) | ||
| .map_err(|err| os_err(vm, err))?; | ||
|
|
||
| // Set __dict__ if provided | ||
| if !vm.is_none(dict) { | ||
| let dict_ref: PyRef<PyDict> = dict.clone().try_into_value(vm)?; | ||
| if let Some(obj_dict) = zelf.as_object().dict() { | ||
| obj_dict.clear(); | ||
| for (key, value) in dict_ref.into_iter() { | ||
| obj_dict.set_item(&*key, value, vm)?; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Respect active exports in BytesIO.__setstate__ to avoid invalidating memoryviews.
__setstate__ replaces the buffer even when exports > 0, which violates the existing “no resize while exported” rule. Use try_resizable(vm)? (or check exports) before mutating the buffer.
🔧 Suggested fix
@@
#[pymethod]
fn __setstate__(zelf: PyRef<Self>, state: PyTupleRef, vm: &VirtualMachine) -> PyResult<()> {
if zelf.closed.load() {
return Err(vm.new_value_error("__setstate__ on closed file"));
}
+ if zelf.exports.load() > 0 {
+ return Err(vm.new_buffer_error(
+ "Existing exports of data: object cannot be re-sized".to_owned(),
+ ));
+ }
if state.len() != 3 {
return Err(vm.new_type_error(format!(
"__setstate__ argument should be 3-tuple, got {}",
state.len()
)));
}
@@
let dict = &state[2];
// Set content
- *zelf.buffer.write() = BufferedIO::new(Cursor::new(content.as_bytes().to_vec()));
+ let mut buffer = zelf.try_resizable(vm)?;
+ *buffer = BufferedIO::new(Cursor::new(content.as_bytes().to_vec()));
// Set position
- zelf.buffer(vm)?
- .seek(SeekFrom::Start(pos))
+ buffer
+ .seek(SeekFrom::Start(pos))
.map_err(|err| os_err(vm, err))?;
+ drop(buffer);
@@
Ok(())
}🤖 Prompt for AI Agents
In `@crates/vm/src/stdlib/io.rs` around lines 4311 - 4347, The __setstate__
implementation is replacing the internal buffer even when there are active
exports,
7082
violating the "no resize while exported" rule; before
mutating/replacing the buffer (where you currently do *zelf.buffer.write() =
BufferedIO::new(Cursor::new(...))), call the buffer's try_resizable(vm)? (or
explicitly check zelf.buffer.exports > 0) and return an appropriate error if
resizing is disallowed, then only perform the BufferedIO::new(Cursor::new(...))
assignment and subsequent seek when try_resizable succeeds; keep references to
__setstate__, try_resizable, buffer, exports, and BufferedIO::new to locate the
change.
| # Test that Union/Optional in function parameter annotations work correctly. | ||
| # This tests that annotation scopes can access global implicit symbols (like Union) | ||
| # that are imported at module level but not explicitly bound in the function scope. | ||
| # Regression test for: https://github.com/RustPython/RustPython/issues/6718 |
There was a problem hiding this comment.
Verify the issue reference.
The comment references #6718, but according to the PR objectives, that is this PR's number, not an issue. The issues being closed are #6702 and #6543. Consider updating the reference to point to the actual issue this regression test covers.
🤖 Prompt for AI Agents
In `@extra_tests/snippets/stdlib_typing.py` at line 20, Update the regression test
comment string "# Regression test for:
https://github.com/RustPython/RustPython/issues/6718" to reference the correct
issue(s) this test covers (e.g., replace 6718 with 6702 or 6543 as appropriate)
or clarify that it refers to this PR if that was intended; locate the comment in
extra_tests/snippets/stdlib_typing.py (the line containing the regression test
URL) and edit the URL/text accordingly so it points to the actual issue
number(s) being closed.
scripts/auto_mark_test.py
Outdated
| @@ -73,7 +73,7 @@ def parse_results(result): | |||
| test_results.stdout = result.stdout | |||
| in_test_results = False | |||
| for line in lines: | |||
| if line == "Run tests sequentially": | |||
| if "Run" in line and "sequentially" in line: | |||
There was a problem hiding this comment.
Would it make more sense for this to be a regular expression match?
There was a problem hiding this comment.
that makes sense. This is merged in #6748 . I will create a separated PR for it
Oh! confused
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/vm/src/bytes_inner.rs (1)
424-469: Fix whitespace handling to match CPython 3.14 behavior.The implementation only skips whitespace before the high nibble, but CPython's
bytes.fromhex()ignores ASCII whitespace anywhere in the input. Input like"0 1"would currently raise"non-hexadecimal number found in fromhex() arg at position X"instead of successfully parsing it asb"\x01".Add whitespace skipping before parsing the low nibble (after fetching the second byte in the pair) to match CPython's behavior.
crates/codegen/src/compile.rs (1)
4240-4293: Resetnext_conditional_annotation_indexfor each class scope.Without resetting per class, the annotation index drifts from previous module or class compilation, causing the
__annotate__function to reference incorrect indices. Save and restore the counter around class body compilation.Proposed fix
@@ - // 2. Set up class namespace + // 2. Set up class namespace + // Reset conditional-annotation index for class scope (restore after) + let saved_conditional_index = self.next_conditional_annotation_index; + self.next_conditional_annotation_index = 0; @@ - // Exit scope and return the code object - Ok(self.exit_scope()) + // Exit scope and return the code object + let code = self.exit_scope(); + self.next_conditional_annotation_index = saved_conditional_index; + Ok(code)
🤖 Fix all issues with AI agents
In `@crates/stdlib/src/ssl/error.rs`:
- Around line 7-11: Remove the unused import types::Constructor from the use
statement in this file; the #[pyexception(...)] macros here do not use
Constructor (or Initializer), so delete Constructor from the import list to
silence the clippy unused-import warning and keep only the actually used symbols
(e.g., Py, PyPayload, PyRef, PyResult, VirtualMachine,
builtins::{PyBaseException, PyOSError, PyStrRef}).
In `@extra_tests/snippets/stdlib_socket.py`:
- Around line 134-136: Remove the commented-out assertion for socket.htonl(-1)
and replace it with a proper test: either (A) fix the Rust implementation so
socket.htonl raises OverflowError for out-of-range values and then
restore/uncomment the test asserting the correct exception on socket.htonl(-1),
or (B) keep the test but mark it with unittest.skip or unittest.expectedFailure
(and a short TODO referencing the tracking issue) so the test suite does not
contain commented code; target the test function that contains the
socket.htonl(-1) assertion and update its decorator accordingly.
♻️ Duplicate comments (5)
crates/compiler-core/src/marshal.rs (1)
205-205: Bump marshalFORMAT_VERSIONfor 32‑bitCodeFlags.
CodeFlagsis now serialized asu32, but the format version is still4, so older cached data will deserialize incorrectly. Please increment the format version to invalidate stale marshaled objects.✅ Suggested change
-pub const FORMAT_VERSION: u32 = 4; +pub const FORMAT_VERSION: u32 = 5;crates/jit/tests/common.rs (1)
91-108: Forward-ref string annotations are silently dropped.The
_val_is_constflag is ignored at line 94, so when annotations use forward-reference strings like"int"(which compile toLOAD_CONST), the value lookup at line 102 will fail since it only checkscode.names, notcode.constants. This causes forward-ref annotations to be silently omitted.Suggested fix
- let (_val_is_const, val_idx) = chunk[1]; + let (val_is_const, val_idx) = chunk[1]; // Key should be a const string (parameter name) if key_is_const && let ConstantData::Str { value } = &code.constants[key_idx] { let param_name = value.to_string_lossy().into_owned(); - // Value should be a name (type name) - if let Some(type_name) = code.names.get(val_idx) { - annotations - .insert(param_name, StackValue::String(type_name.clone())); - } + // Value can be a name (type ref) or a const string (forward ref) + let type_name = if val_is_const { + match &code.constants.get(val_idx) { + Some(ConstantData::Str { value }) => { + Some(value.to_string_lossy().into_owned()) + } + _ => None, + } + } else { + code.names.get(val_idx).cloned() + }; + if let Some(type_name) = type_name { + annotations.insert(param_name, StackValue::String(type_name)); + } }extra_tests/snippets/syntax_assignment.py (1)
62-73: Silence F821 lint warnings on__annotate__usage.Static analysis flags
__annotate__as undefined on lines 70-71, but this is a runtime-injected name per PEP 649. Consider adding# noqa: F821comments to keep lint clean if these snippets are linted in CI.Suggested lint-safe tweak
-assert callable(__annotate__) -annotations = __annotate__(1) # 1 = FORMAT_VALUE +assert callable(__annotate__) # noqa: F821 +annotations = __annotate__(1) # noqa: F821 # 1 = FORMAT_VALUEcrates/vm/src/builtins/type.rs (1)
958-988: Verify__annotations__assignment behavior against CPython 3.14.Per PEP 649/749, assigning to
cls.__annotations__in CPython 3.14 stores directly to__annotations__in the class dict and sets__annotate__toNone. The current implementation conditionally updates__annotations__vs__annotations_cache__based on whether__annotations__already exists, which may diverge from CPython's behavior where assignment always writes to__annotations__.CPython 3.14 PEP 649 class __annotations__ assignment behaviorcrates/codegen/src/compile.rs (1)
5928-5966: Always record executed simple annotations when conditional tracking is enabled.
Right now indices are only recorded for module scope or whenin_conditional_blockis true, which drops unconditional class annotations whenhas_conditional_annotationsis set. Record every executed simple annotation to keep indices aligned withcompile_module_annotate.🐛 Proposed fix
- let is_module = self.current_symbol_table().typ == CompilerScope::Module; - let is_conditional = is_module || self.in_conditional_block; - - if is_conditional { - // Get the current annotation index and increment - let annotation_index = self.next_conditional_annotation_index; - self.next_conditional_annotation_index += 1; - - // Add index to __conditional_annotations__ set - let cond_annotations_name = self.name("__conditional_annotations__"); - emit!(self, Instruction::LoadName(cond_annotations_name)); - self.emit_load_const(ConstantData::Integer { - value: annotation_index.into(), - }); - emit!(self, Instruction::SetAdd { i: 0_u32 }); - emit!(self, Instruction::PopTop); - } + // Record every executed simple annotation when conditional handling is enabled + let annotation_index = self.next_conditional_annotation_index; + self.next_conditional_annotation_index += 1; + + let cond_annotations_name = self.name("__conditional_annotations__"); + emit!(self, Instruction::LoadName(cond_annotations_name)); + self.emit_load_const(ConstantData::Integer { + value: annotation_index.into(), + }); + emit!(self, Instruction::SetAdd { i: 0_u32 }); + emit!(self, Instruction::PopTop);
🧹 Nitpick comments (5)
scripts/auto_mark_test.py (1)
67-74: Consider using__init__instead of mutable class attributes.
testsandunexpected_successesare mutable class attributes shared by all instances. Whileparse_results()mitigates this by reassigning new lists, this pattern can cause subtle bugs if usage changes.♻️ Suggested refactor
class TestResult: - tests_result: str = "" - tests = [] - unexpected_successes = [] # Tests that passed but were marked as expectedFailure - stdout = "" + def __init__(self): + self.tests_result: str = "" + self.tests: list[Test] = [] + self.unexpected_successes: list[Test] = [] # Tests that passed but were marked as expectedFailure + self.stdout: str = ""extra_tests/snippets/code_co_consts.py (1)
24-25: Consider definingCO_HAS_DOCSTRINGas a named constant.The magic number
0x4000000is used repeatedly throughout the file. Defining it as a constant at the top would improve readability and maintainability.Suggested improvement
+# CO_HAS_DOCSTRING flag added in Python 3.14 +CO_HAS_DOCSTRING = 0x4000000 + # Test function with docstring - docstring should be co_consts[0] def with_doc(): """This is a docstring""" return 1 assert with_doc.__code__.co_consts[0] == "This is a docstring", ( with_doc.__code__.co_consts ) assert with_doc.__doc__ == "This is a docstring" -# Check CO_HAS_DOCSTRING flag (0x4000000) -assert with_doc.__code__.co_flags & 0x4000000, hex(with_doc.__code__.co_flags) +# Check CO_HAS_DOCSTRING flag +assert with_doc.__code__.co_flags & CO_HAS_DOCSTRING, hex(with_doc.__code__.co_flags)crates/codegen/src/symboltable.rs (3)
383-412: Consider avoiding clone of class symbols for annotation scope.The
symbols.clone()at line 389 may be expensive for large class scopes. This clone is needed to avoid borrow conflicts, but consider whether passing a reference with appropriate lifetime management could work instead.However, given the complexity of the borrow checker constraints with the
with_appendpattern and the fact that annotation blocks are relatively rare, this is acceptable for correctness.
932-956: Consider consolidating the helper methods.Both
add_classdict_freevarandadd_conditional_annotations_freevarfollow identical patterns with only the name differing. Consider a single parameterized helper.♻️ Optional refactor 4E22 to reduce duplication
- fn add_classdict_freevar(&mut self) { - let table = self.tables.last_mut().unwrap(); - let name = "__classdict__"; - let symbol = table - .symbols - .entry(name.to_owned()) - .or_insert_with(|| Symbol::new(name)); - symbol.scope = SymbolScope::Free; - symbol - .flags - .insert(SymbolFlags::REFERENCED | SymbolFlags::FREE_CLASS); - } - - fn add_conditional_annotations_freevar(&mut self) { - let table = self.tables.last_mut().unwrap(); - let name = "__conditional_annotations__"; - let symbol = table - .symbols - .entry(name.to_owned()) - .or_insert_with(|| Symbol::new(name)); - symbol.scope = SymbolScope::Free; - symbol - .flags - .insert(SymbolFlags::REFERENCED | SymbolFlags::FREE_CLASS); - } + fn add_class_freevar(&mut self, name: &str) { + let table = self.tables.last_mut().unwrap(); + let symbol = table + .symbols + .entry(name.to_owned()) + .or_insert_with(|| Symbol::new(name)); + symbol.scope = SymbolScope::Free; + symbol + .flags + .insert(SymbolFlags::REFERENCED | SymbolFlags::FREE_CLASS); + }Then call as
self.add_class_freevar("__classdict__")andself.add_class_freevar("__conditional_annotations__").
1048-1055: Clarify the rationale for module-only re-scan.The comment explains that class scope should not re-scan to preserve class-local symbol resolution, but the rationale for why module scope needs re-scanning could be clearer. Consider adding a comment explaining that module-level annotations may reference global builtins (like
str,int) that need to be registered in the module scope's symbol table for proper code generation.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@crates/codegen/src/symboltable.rs`:
- Around line 1093-1137: The parent scope's annotation_block is only saved for
Module/Class, causing parent annotation_block to be taken (and lost) when the
parent is a Function; change the logic to always save the parent's
annotation_block before calling enter_scope_with_parameters/scan_statements and
restore it afterward. Concretely, replace the matches-based
should_save_annotation_block and conditional take with an unconditional let
saved_annotation_block =
self.tables.last_mut().unwrap().annotation_block.take(); then after leave_scope
(and after optional type param leave) restore it with if let Some(block) =
saved_annotation_block { self.tables.last_mut().unwrap().annotation_block =
Some(block); }, keeping existing calls to enter_type_param_block,
scan_type_params, enter_scope_with_parameters, scan_statements, and leave_scope
intact.
- Around line 2165-2176: The guard that prevents assignment to "__debug__"
currently checks role against SymbolUsage::Parameter,
SymbolUsage::AnnotationParameter, and SymbolUsage::Assigned but omits
SymbolUsage::AnnotationAssigned, so annotated assignments like `__debug__: int =
1` are allowed; update the match in the block that returns SymbolTableError (the
check comparing name == "__debug__" and matches!(role, ...)) to include
SymbolUsage::AnnotationAssigned alongside the other variants so annotated
assignments produce the same "cannot assign to __debug__" error using the
existing SymbolTableError and location variables.
- Around line 885-930: enter_annotation_scope currently clears
self.current_varnames and leave_annotation_scope overwrites table.varnames,
which drops the pre-seeded "format" and the enclosing function's varnames; fix
by saving/restoring the parent's current_varnames around annotation scopes and
seeding current_varnames from the annotation block's existing varnames: in
enter_annotation_scope, before taking current.annotation_block, save let
parent_varnames = core::mem::take(&mut self.current_varnames), then when you
push the annotation table set self.current_varnames = table.varnames.clone() (or
merge so "format" is preserved); in leave_annotation_scope, before storing
table.varnames = core::mem::take(&mut self.current_varnames) restore the saved
parent_varnames back into self.current_varnames so enclosing scope names are not
lost; keep the "format" seed in the annotation_table.varnames when creating it
and ensure add_classdict_freevar/add_conditional_annotations_freevar logic (and
future_annotations checks) still runs while the annotation table is active.
In `@extra_tests/snippets/code_co_consts.py`:
- Around line 99-101: The test for cls_with_doc.method is missing an assertion
that the CO_HAS_DOCSTRING flag is set; update the test to assert that
cls_with_doc.method.__code__.co_flags & CO_HAS_DOCSTRING != 0 (using the
CO_HAS_DOCSTRING symbol) in addition to the existing checks of co_consts[0] and
__doc__ so it matches the other "with_doc" test cases.
♻️ Duplicate comments (7)
crates/vm/src/frame.rs (1)
1171-1193: LGTM! Past review concern properly addressed.The implementation now correctly handles errors from custom mappings:
- For
PyDict: usesget_item_optwith?to propagate errors while returningNonefor missing keys- For non-dict mappings: explicitly matches on the error type, only treating
KeyErroras "not found" and propagating other exceptionsThis resolves the previous concern about swallowing non-KeyError exceptions when using
.ok().crates/jit/tests/common.rs (1)
91-105: Handle const-string annotation values in annotate extraction.Line 94 ignores
val_is_const, so forward-ref annotations compiled asLOAD_CONST "int"are dropped (or can mis-map by index intocode.names). This can yield missing annotations and JIT panics for annotated args.🛠️ Suggested fix
- let (_val_is_const, val_idx) = chunk[1]; + let (val_is_const, val_idx) = chunk[1]; @@ - // Value should be a name (type name) - if let Some(type_name) = code.names.get(val_idx) { - annotations - .insert(param_name, StackValue::String(type_name.clone())); - } + // Value can be a name or a const string (forward ref) + let type_name = if val_is_const { + match &code.constants[val_idx] { + ConstantData::Str { value } => { + Some(value.to_string_lossy().into_owned()) + } + _ => None, + } + } else { + code.names.get(val_idx).cloned() + }; + if let Some(type_name) = type_name { + annotations.insert(param_name, StackValue::String(type_name)); + }extra_tests/snippets/syntax_assignment.py (1)
70-71: Add# noqa: F821to silence static analysis warnings for__annotate__.The static analyzer flags
__annotate__as undefined because it's a runtime-injected name per PEP 649. This will cause linting failures in CI.crates/codegen/src/compile.rs (1)
5928-5966: Unconditional class annotations are still skipped in conditional tracking.When
has_conditional_annotationsis true, indices are only recorded foris_module || in_conditional_block(Line 5950-5953). In class scope, unconditional annotations at the top level won’t be recorded, so__annotate__will filter them out. This matches a previously reported issue.🐛 Suggested fix (record every executed simple annotation)
- let is_module = self.current_symbol_table().typ == CompilerScope::Module; - let is_conditional = is_module || self.in_conditional_block; - - if is_conditional { - // Get the current annotation index and increment - let annotation_index = self.next_conditional_annotation_index; - self.next_conditional_annotation_index += 1; - - // Add index to __conditional_annotations__ set - let cond_annotations_name = self.name("__conditional_annotations__"); - emit!(self, Instruction::LoadName(cond_annotations_name)); - self.emit_load_const(ConstantData::Integer { - value: annotation_index.into(), - }); - emit!(self, Instruction::SetAdd { i: 0_u32 }); - emit!(self, Instruction::PopTop); - } + // Record every executed simple annotation when conditional handling is enabled. + let annotation_index = self.next_conditional_annotation_index; + self.next_conditional_annotation_index += 1; + + let cond_annotations_name = self.name("__conditional_annotations__"); + emit!(self, Instruction::LoadName(cond_annotations_name)); + self.emit_load_const(ConstantData::Integer { + value: annotation_index.into(), + }); + emit!(self, Instruction::SetAdd { i: 0_u32 }); + emit!(self, Instruction::PopTop);crates/compiler-core/src/marshal.rs (1)
7-7: Bump marshal FORMAT_VERSION for widened CodeFlags.CodeFlags are now serialized as u32, but FORMAT_VERSION remains 4. This will misread older caches. Please bump the format version to invalidate stale data.
🛠️ Proposed fix
-pub const FORMAT_VERSION: u32 = 4; +pub const FORMAT_VERSION: u32 = 5;Also applies to: 205-205, 663-663
crates/vm/src/builtins/type.rs (2)
879-904: Cache clearing fix applied correctlyThe previous review comment suggested unconditionally clearing
__annotations_cache__when__annotate__is updated. Line 901 now performsattrs.swap_remove(identifier!(vm, __annotations_cache__))unconditionally after setting the new value, which addresses the stale cache concern.
958-990: Verify alignment with CPython 3.14__annotations__assignment semanticsThe conditional logic writes to either
__annotations__or__annotations_cache__depending on whether__annotations__already exists. Per the previous review comment, CPython 3.14 (PEP 649) always stores assigned values directly in__annotations__within the class__dict__, not conditionally to a cache.Consider simplifying to always write to
__annotations__on assignment:🩹 Suggested fix
fn set___annotations__(&self, value: Option<PyObjectRef>, vm: &VirtualMachine) -> PyResult<()> { if self.slots.flags.has_feature(PyTypeFlags::IMMUTABLETYPE) { return Err(vm.new_type_error(format!( "cannot set '__annotations__' attribute of immutable type '{}'", self.name() ))); } let mut attrs = self.attributes.write(); - // conditional update based on __annotations__ presence - let has_annotations = attrs.contains_key(identifier!(vm, __annotations__)); - - if has_annotations { - // If __annotations__ is in dict, update it - if let Some(value) = value { - attrs.insert(identifier!(vm, __annotations__), value); - } else if attrs - .swap_remove(identifier!(vm, __annotations__)) - .is_none() - { - return Err(vm.new_attribute_error("__annotations__".to_owned())); - } - // Also clear __annotations_cache__ - attrs.swap_remove(identifier!(vm, __annotations_cache__)); - } else { - // Otherwise update only __annotations_cache__ - if let Some(value) = value { - attrs.insert(identifier!(vm, __annotations_cache__), value); - } else if attrs - .swap_remove(identifier!(vm, __annotations_cache__)) - .is_none() - { - return Err(vm.new_attribute_error("__annotations__".to_owned())); - } - } + if let Some(value) = value { + attrs.insert(identifier!(vm, __annotations__), value); + } else if attrs.swap_remove(identifier!(vm, __annotations__)).is_none() + && attrs.swap_remove(identifier!(vm, __annotations_cache__)).is_none() + { + return Err(vm.new_attribute_error("__annotations__".to_owned())); + } + // Clear cache on any update + attrs.swap_remove(identifier!(vm, __annotations_cache__)); // Always clear __annotate_func__ and __annotate__ attrs.swap_remove(identifier!(vm, __annotate_func__)); attrs.swap_remove(identifier!(vm, __annotate__)); Ok(()) }CPython 3.14 PEP 649 class __annotations__ setter behavior
🧹 Nitpick comments (4)
extra_tests/snippets/code_co_consts.py (1)
1-34: Consider extracting the magic number to a named constant.The
CO_HAS_DOCSTRINGflag value0x4000000is used multiple times throughout the file. Extracting it to a named constant at the top would improve readability and maintainability.♻️ Suggested improvement
+# CO_HAS_DOCSTRING flag introduced in Python 3.14 +CO_HAS_DOCSTRING = 0x4000000 + # Test function with docstring - docstring should be co_consts[0] def with_doc():Then replace all occurrences of
0x4000000withCO_HAS_DOCSTRING.crates/vm/src/exception_group.rs (1)
351-364: The kwargs validation logic is correct, but Line 362 is redundant.The explicit kwargs rejection aligns with CPython's
BaseExceptionGroup.__init__behavior, and the decision not to callPyBaseException::slot_initis correctly documented—slot_newalready initializesargsto(message, exceptions_tuple).However, Line 362 (
let _ = (zelf, args, vm);) is unnecessary since all three variables are already used:zelf.class().name()in the error message,args.kwargsin the check, andvmfor error creation.🔧 Suggested cleanup
return Err(vm.new_type_error(format!( "{} does not take keyword arguments", zelf.class().name() ))); } // Do NOT call PyBaseException::slot_init here. // slot_new already set args to (message, exceptions_tuple). // Calling base init would overwrite with original args (message, exceptions_list). - let _ = (zelf, args, vm); Ok(())crates/vm/src/builtins/module.rs (2)
200-221: Consider clearing__annotations__unconditionally when setting__annotate__The current logic only clears
__annotations__when the new value is notNone(line 214). However, per PEP 649 semantics, setting__annotate__toNoneshould also invalidate any cached annotations since the annotation source is being explicitly removed.This mirrors the issue previously flagged in
type.rs. Consider unconditionally clearing__annotations__:🩹 Suggested fix
PySetterValue::Assign(value) => { if !vm.is_none(&value) && !value.is_callable() { return Err(vm.new_type_error("__annotate__ must be callable or None")); } let dict = zelf.dict(); dict.set_item(identifier!(vm, __annotate__), value.clone(), vm)?; - // Clear __annotations__ if value is not None - if !vm.is_none(&value) { - dict.del_item(identifier!(vm, __annotations__), vm).ok(); - } + // Clear cached __annotations__ on any change to __annotate__ + dict.del_item(identifier!(vm, __annotations__), vm).ok(); Ok(()) }
261-269: Improve robustness ofis_initializinghelperThe current implementation may panic or behave unexpectedly if
_initializingattribute access fails for reasons other than missing attribute. Consider handling the error case more explicitly:🩹 Suggested fix
fn is_initializing(dict: &PyDictRef, vm: &VirtualMachine) -> bool { if let Ok(Some(spec)) = dict.get_item_opt(vm.ctx.intern_str("__spec__"), vm) - && let Ok(initializing) = spec.get_attr(vm.ctx.intern_str("_initializing"), vm) { - return initializing.try_to_bool(vm).unwrap_or(false); + if let Ok(initializing) = spec.get_attr(vm.ctx.intern_str("_initializing"), vm) { + return initializing.try_to_bool(vm).unwrap_or(false); + } } false }The suggested fix is equivalent but may be clearer for future readers. The current code is functionally correct.
| // Save the parent's annotation_block before scanning function annotations, | ||
| // so function annotations don't interfere with parent scope annotations. | ||
| // This applies to both class scope (methods) and module scope (top-level functions). | ||
| let parent_scope_typ = self.tables.last().map(|t| t.typ); | ||
| let should_save_annotation_block = matches!( | ||
| parent_scope_typ, | ||
| Some(CompilerScope::Class) | Some(CompilerScope::Module) | ||
| ); | ||
| let saved_annotation_block = if should_save_annotation_block { | ||
| self.tables.last_mut().unwrap().annotation_block.take() | ||
| } else { | ||
| None | BD9E||
| }; | ||
|
|
||
| // For generic functions, enter type_param block FIRST so that | ||
| // annotation scopes are nested inside and can see type parameters. | ||
| if let Some(type_params) = type_params { | ||
| self.enter_type_param_block( | ||
| &format!("<generic parameters of {}>", name.as_str()), | ||
| self.line_index_start(type_params.range), | ||
| )?; | ||
| self.scan_type_params(type_params)?; | ||
| } | ||
| let has_return_annotation = if let Some(expression) = returns { | ||
| self.scan_annotation(expression)?; | ||
| true | ||
| } else { | ||
| false | ||
| }; | ||
| self.enter_scope_with_parameters( | ||
| name.as_str(), | ||
| parameters, | ||
| self.line_index_start(*range), | ||
| has_return_annotation, | ||
| )?; | ||
| self.scan_statements(body)?; | ||
| self.leave_scope(); | ||
| if type_params.is_some() { | ||
| self.leave_scope(); | ||
| } | ||
|
|
||
| // Restore parent's annotation_block after processing the function | ||
| if let Some(block) = saved_annotation_block { | ||
| self.tables.last_mut().unwrap().annotation_block = Some(block); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the CompilerScope enum and its definition
cd crates/codegen/src && rg -n "enum CompilerScope" -A 10 symboltable.rsRepository: RustPython/RustPython
Length of output: 312
🏁 Script executed:
# Let's look for how annotation_block is used across different scopes
cd crates/codegen/src && rg -n "annotation_block" symboltable.rs | head -30Repository: RustPython/RustPython
Length of output: 1914
🏁 Script executed:
# Get the full function context around lines 1093-1137
cd crates/codegen/src && sed -n '1050,1150p' symboltable.rsRepository: RustPython/RustPython
Length of output: 4277
🏁 Script executed:
cd crates/codegen/src && rg -n "fn enter_scope_with_parameters" -A 30 symboltable.rsRepository: RustPython/RustPython
Length of output: 1245
🏁 Script executed:
# Also check where annotation_block is set/taken for functions
cd crates/codegen/src && sed -n '2100,2150p' symboltable.rsRepository: RustPython/RustPython
Length of output: 2016
Unconditionally save/restore parent annotation_block to avoid clobbering in nested functions.
The save/restore guard only covers Module and Class parent scopes. When a function with annotation_block contains a nested function with annotations, the parent's block is taken by enter_scope_with_parameters (line 2123), losing the parent's annotations. This occurs specifically when a Function scope is the parent (not just Class/Module), which the current scope-type check misses.
🛠️ Proposed fix
- let parent_scope_typ = self.tables.last().map(|t| t.typ);
- let should_save_annotation_block = matches!(
- parent_scope_typ,
- Some(CompilerScope::Class) | Some(CompilerScope::Module)
- );
- let saved_annotation_block = if should_save_annotation_block {
- self.tables.last_mut().unwrap().annotation_block.take()
- } else {
- None
- };
+ let saved_annotation_block =
+ self.tables.last_mut().and_then(|t| t.annotation_block.take());🤖 Prompt for AI Agents
In `@crates/codegen/src/symboltable.rs` around lines 1093 - 1137, The parent
scope's annotation_block is only saved for Module/Class, causing parent
annotation_block to be taken (and lost) when the parent is a Function; change
the logic to always save the parent's annotation_block before calling
enter_scope_with_parameters/scan_statements and restore it afterward.
Concretely, replace the matches-based should_save_annotation_block and
conditional take with an unconditional let saved_annotation_block =
self.tables.last_mut().unwrap().annotation_block.take(); then after leave_scope
(and after optional type param leave) restore it with if let Some(block) =
saved_annotation_block { self.tables.last_mut().unwrap().annotation_block =
Some(block); }, keeping existing calls to enter_type_param_block,
scan_type_params, enter_scope_with_parameters, scan_statements, and leave_scope
intact.
| // Check for forbidden names like __debug__ | ||
| if name == "__debug__" | ||
| && matches!( | ||
| role, | ||
| SymbolUsage::Parameter | SymbolUsage::AnnotationParameter | SymbolUsage::Assigned | ||
| ) | ||
| { | ||
| return Err(SymbolTableError { | ||
| error: "cannot assign to __debug__".to_owned(), | ||
| location, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file and understand the context around the code
wc -l crates/codegen/src/symboltable.rsRepository: RustPython/RustPython
Length of output: 103
🏁 Script executed:
# Read the code around lines 2165-2176 with context
sed -n '2150,2190p' crates/codegen/src/symboltable.rsRepository: RustPython/RustPython
Length of output: 1588
🏁 Script executed:
# Find the SymbolUsage enum definition to understand all variants
rg "enum SymbolUsage" -A 15 crates/codegen/src/symboltable.rsRepository: RustPython/RustPython
Length of output: 337
🏁 Script executed:
# Search for AnnotationAssigned variant in the codebase
rg "AnnotationAssigned" crates/codegen/src/Repository: RustPython/RustPython
Length of output: 338
🏁 Script executed:
# Search for other __debug__ checks to understand the pattern
rg "__debug__" crates/codegen/src/symboltable.rsRepository: RustPython/RustPython
Length of output: 211
🏁 Script executed:
# Find where AnnotationAssigned is registered to understand the context
rg "AnnotationAssigned" -B 3 -A 3 crates/codegen/src/symboltable.rs | head -40Repository: RustPython/RustPython
Length of output: 983
🏁 Script executed:
# Look for tests related to __debug__ validation
find crates -name "*.rs" -type f | xargs rg "__debug__" -lRepository: RustPython/RustPython
Length of output: 161
🏁 Script executed:
# Check if there are test files for symboltable
fd "test" crates/codegen/src/ -type fRepository: RustPython/RustPython
Length of output: 235
🏁 Script executed:
# Search for __debug__ in test files
rg "__debug__" crates/compiler/test_fixtures/ -l 2>/dev/null | head -10Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check the compile.rs file to see how __debug__ is handled there
rg "__debug__" -B 3 -A 3 crates/codegen/src/compile.rsRepository: RustPython/RustPython
Length of output: 1068
🏁 Script executed:
# Let's also verify the context where AnnotationAssigned is used
sed -n '2000,2050p' crates/codegen/src/symboltable.rsRepository: RustPython/RustPython
Length of output: 1911
🏁 Script executed:
# Check the full pattern matching logic for SymbolUsage to understand what should be covered
rg "SymbolUsage::" crates/codegen/src/symboltable.rs | grep -E "(Parameter|Assigned|Annotation)" | head -20Repository: RustPython/RustPython
Length of output: 1550
🏁 Script executed:
# Let's look at the specific line where AnnotationAssigned is registered to confirm the context
sed -n '1750,1800p' crates/codegen/src/symboltable.rsRepository: RustPython/RustPython
Length of output: 2542
🏁 Script executed:
# Also check what the error message context is and verify this guard is about preventing assignment
sed -n '2160,2200p' crates/codegen/src/symboltable.rsRepository: RustPython/RustPython
Length of output: 1734
🏁 Script executed:
# Search for how Python handles __debug__ to confirm the semantics
rg "class.*annotation" crates/compiler/test_fixtures/ -i --max-count=5Repository: RustPython/RustPython
Length of output: 135
🏁 Script executed:
# Look for test files related to symboltable or __debug__
find crates -name "*.py" -o -name "*.rs" | xargs grep -l "test.*__debug__" 2>/dev/null | head -5Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Let's also check if there's documentation or examples of annotated assignments
rg "AnnotationAssigned" -B 5 -A 10 crates/codegen/src/symboltable.rs | head -50Repository: RustPython/RustPython
Length of output: 2137
🏁 Script executed:
# Verify the proposed diff is syntactically valid Rust
cat > /tmp/test_syntax.rs << 'EOF'
enum SymbolUsage {
Parameter,
AnnotationParameter,
Assigned,
AnnotationAssigned,
}
fn test() {
let role = SymbolUsage::Assigned;
// Current version
let _check1 = matches!(
role,
SymbolUsage::Parameter | SymbolUsage::AnnotationParameter | SymbolUsage::Assigned
);
// Proposed version
let _check2 = matches!(
role,
SymbolUsage::Parameter
| SymbolUsage::AnnotationParameter
| SymbolUsage::Assigned
| SymbolUsage::AnnotationAssigned
);
}
EOF
rustc --crate-type lib /tmp/test_syntax.rs 2>&1 | head -20Repository: RustPython/RustPython
Length of output: 109
__debug__ assignment guard misses annotated assignments.
SymbolUsage::AnnotationAssigned isn't covered, so __debug__: int = 1 would slip through. This should remain a syntax error.
🛠️ Proposed fix
- if name == "__debug__"
- && matches!(
- role,
- SymbolUsage::Parameter | SymbolUsage::AnnotationParameter | SymbolUsage::Assigned
- )
+ if name == "__debug__"
+ && matches!(
+ role,
+ SymbolUsage::Parameter
+ | SymbolUsage::AnnotationParameter
+ | SymbolUsage::Assigned
+ | SymbolUsage::AnnotationAssigned
+ )
{
return Err(SymbolTableError {
error: "cannot assign to __debug__".to_owned(),
location,
});
}📝 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.
| // Check for forbidden names like __debug__ | |
| if name == "__debug__" | |
| && matches!( | |
| role, | |
| SymbolUsage::Parameter | SymbolUsage::AnnotationParameter | SymbolUsage::Assigned | |
| ) | |
| { | |
| return Err(SymbolTableError { | |
| error: "cannot assign to __debug__".to_owned(), | |
| location, | |
| }); | |
| } | |
| // Check for forbidden names like __debug__ | |
| if name == "__debug__" | |
| && matches!( | |
| role, | |
| SymbolUsage::Parameter | |
| | SymbolUsage::AnnotationParameter | |
| | SymbolUsage::Assigned | |
| | SymbolUsage::AnnotationAssigned | |
| ) | |
| { | |
| return Err(SymbolTableError { | |
| error: "cannot assign to __debug__".to_owned(), | |
| location, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@crates/codegen/src/symboltable.rs` around lines 2165 - 2176, The guard that
prevents assignment to "__debug__" currently checks role against
SymbolUsage::Parameter, SymbolUsage::AnnotationParameter, and
SymbolUsage::Assigned but omits SymbolUsage::AnnotationAssigned, so annotated
assignments like `__debug__: int = 1` are allowed; update the match in the block
that returns SymbolTableError (the check comparing name == "__debug__" and
matches!(role, ...)) to include SymbolUsage::AnnotationAssigned alongside the
other variants so annotated assignments produce the same "cannot assign to
__debug__" error using the existing SymbolTableError and location variables.
| assert cls_with_doc.method.__code__.co_consts[0] == "Method docstring" | ||
| assert cls_with_doc.method.__doc__ == "Method docstring" | ||
|
|
There was a problem hiding this comment.
Missing CO_HAS_DOCSTRING flag assertion for consistency.
All other "with_doc" test cases verify the CO_HAS_DOCSTRING flag is set, but this class method test omits that check. Add the flag assertion for complete coverage.
🔧 Suggested fix
assert cls_with_doc.method.__code__.co_consts[0] == "Method docstring"
assert cls_with_doc.method.__doc__ == "Method docstring"
+assert cls_with_doc.method.__code__.co_flags & 0x4000000🤖 Prompt for AI Agents
In `@extra_tests/snippets/code_co_consts.py` around lines 99 - 101, The test for
cls_with_doc.method is missing an assertion that the CO_HAS_DOCSTRING flag is
set; update the test to assert that cls_with_doc.method.__code__.co_flags &
CO_HAS_DOCSTRING != 0 (using the CO_HAS_DOCSTRING symbol) in addition to the
existing checks of co_consts[0] and __doc__ so it matches the other "with_doc"
test cases.
|
Still have issues to fix, but merge this one as it is to encourage 3.14 update. |
close #6702
close #6543
Python 3.14 appears to be a release with a significant number of changes.
This PR currently includes PEP 649 and PEP 750, along with various other behavior changes and deprecations.__annotate__; this seems coupled to 3.14 changes in many libraries__init__behavior of exceptions is changedSummary by CodeRabbit
New Features
Bug Fixes
Behavior Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.