Visitar URL original
Upgrade test_code and fix code bugs by youknowone · Pull Request #7440 · RustPython/RustPython · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Fix test_code: compiler and code object improvements
- Add CO_NESTED flag (0x10) for nested function scopes
- Emit LOAD_SMALL_INT for integers 0..=255 instead of LOAD_CONST
- Eliminate dead constant expression statements (no side effects)
- Ensure None in co_consts for functions with no other constants
- Add code.__replace__() for copy.replace() support
- Mark test_co_lnotab and test_invalid_bytecode as expectedFailure
  • Loading branch information
youknowone committed Mar 17, 2026
commit c57f4decd5905df4e5c11b8b142c7364cac509fd
6 changes: 3 additions & 3 deletions Lib/test/test_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,6 @@ def func():
with self.assertRaises(ValueError):
co.replace(co_nlocals=co.co_nlocals + 1)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_shrinking_localsplus(self):
# Check that PyCode_NewWithPosOnlyArgs resizes both
# localsplusnames and localspluskinds, if an argument is a cell.
Expand All @@ -429,7 +427,7 @@ def func():
new_code = code = func.__code__.replace(co_linetable=b'')
self.assertEqual(list(new_code.co_lines()), [])

# TODO: RUSTPYTHON
# TODO: RUSTPYTHON; co_lnotab intentionally not implemented (deprecated since 3.12)
@unittest.expectedFailure
def test_co_lnotab_is_deprecated(self): # TODO: remove in 3.14
def func():
Expand Down Expand Up @@ -493,6 +491,8 @@ def spam9():
res = _testinternalcapi.code_returns_only_none(func.__code__)
self.assertFalse(res)

# TODO: RUSTPYTHON; replace() rejects invalid bytecodes for safety
@unittest.expectedFailure
def test_invalid_bytecode(self):
def foo():
pass
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_codeop.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ def assertInvalid(self, str, symbol='single', is_syntax=1):
except OverflowError:
self.assertTrue(not is_syntax)

@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: <code object <module> at 0xc99532080 file "<input>", line 1> != <code object <module> at 0xc99532f80 file "<input>", line 1>
def test_valid(self):
av = self.assertValid

Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,6 @@ def test_bug_46724(self):
# Test that negative operargs are handled properly
self.do_disassembly_test(bug46724, dis_bug46724)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_kw_names(self):
# Test that value is displayed for keyword argument names:
self.do_disassembly_test(wrap_func_w_kwargs, dis_kw_names)
Expand Down Expand Up @@ -1179,7 +1178,6 @@ def test_disassemble_str(self):
self.do_disassembly_test(fn_with_annotate_str, dis_fn_with_annotate_str)
self.do_disassembly_test(compound_stmt_str, dis_compound_stmt_str)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_disassemble_bytes(self):
self.do_disassembly_test(_f.__code__.co_code, dis_f_co_code)

Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_marshal.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ def test_exceptions(self):
self.assertEqual(StopIteration, new)

class CodeTestCase(unittest.TestCase):
@unittest.expectedFailure # TODO: RUSTPYTHON
def test_code(self):
co = ExceptionTestCase.test_exceptions.__code__
new = marshal.loads(marshal.dumps(co))
Expand Down
64 changes: 49 additions & 15 deletions crates/codegen/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,14 @@ impl Compiler {
),
};

// Set CO_NESTED for scopes defined inside another function/class/etc.
// (i.e., not at module level)
let flags = if self.code_stack.len() > 1 {
flags | bytecode::CodeFlags::NESTED
} else {
flags
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Get private name from parent scope
let private = if !self.code_stack.is_empty() {
self.code_stack.last().unwrap().private.clone()
Expand Down Expand Up @@ -1202,7 +1210,8 @@ impl Compiler {
// enter_scope sets default values based on scope_type, but push_output
// allows callers to specify exact values
if let Some(info) = self.code_stack.last_mut() {
info.flags = flags;
// Preserve NESTED flag set by enter_scope
info.flags = flags | (info.flags & bytecode::CodeFlags::NESTED);
info.metadata.argcount = arg_count;
info.metadata.posonlyargcount = posonlyarg_count;
info.metadata.kwonlyargcount = kwonlyarg_count;
Expand Down Expand Up @@ -2179,18 +2188,26 @@ impl Compiler {
}
}
ast::Stmt::Expr(ast::StmtExpr { value, .. }) => {
self.compile_expression(value)?;
// Optimize away constant expressions with no side effects.
// In interactive mode, always compile (to print the result).
let dominated_by_interactive =
self.interactive && !self.ctx.in_func() && !self.ctx.in_class;
if !dominated_by_interactive && Self::is_const_expression(value) {
// Skip compilation entirely - the expression has no side effects
} else {
self.compile_expression(value)?;

if self.interactive && !self.ctx.in_func() && !self.ctx.in_class {
emit!(
self,
Instruction::CallIntrinsic1 {
func: bytecode::IntrinsicFunction1::Print
}
);
}
if dominated_by_interactive {
emit!(
self,
Instruction::CallIntrinsic1 {
func: bytecode::IntrinsicFunction1::Print
}
);
}

emit!(self, Instruction::PopTop);
emit!(self, Instruction::PopTop);
Comment on lines +2193 to +2209

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Interactive printing currently leaks into nested blocks

The condition on Line 2194 is true for nested module-level blocks (if/try/with), so expression statements inside those blocks can get auto-printed. This should be restricted to top-level interactive statements only.

Suggested fix
-                let dominated_by_interactive =
-                    self.interactive && !self.ctx.in_func() && !self.ctx.in_class;
+                let dominated_by_interactive = self.interactive
+                    && !self.ctx.in_func()
+                    && !self.ctx.in_class
+                    && self.current_code_info().in_conditional_block == 0;
📝 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.

Suggested change
let dominated_by_interactive =
self.interactive && !self.ctx.in_func() && !self.ctx.in_class;
if !dominated_by_interactive && Self::is_const_expression(value) {
// Skip compilation entirely - the expression has no side effects
} else {
self.compile_expression(value)?;
if self.interactive && !self.ctx.in_func() && !self.ctx.in_class {
emit!(
self,
Instruction::CallIntrinsic1 {
func: bytecode::IntrinsicFunction1::Print
}
);
}
if dominated_by_interactive {
emit!(
self,
Instruction::CallIntrinsic1 {
func: bytecode::IntrinsicFunction1::Print
}
);
}
emit!(self, Instruction::PopTop);
emit!(self, Instruction::PopTop);
let dominated_by_interactive = self.interactive
&& !self.ctx.in_func()
&& !self.ctx.in_class
&& self.current_code_info().in_conditional_block == 0;
if !dominated_by_interactive && Self::is_const_expression(value) {
// Skip compilation entirely - the expression has no side effects
} else {
self.compile_expression(value)?;
if dominated_by_interactive {
emit!(
self,
Instruction::CallIntrinsic1 {
func: bytecode::IntrinsicFunction1::Print
}
);
}
emit!(self, Instruction::PopTop);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/compile.rs` around lines 2193 - 2209, The
interactive-printing guard (dominated_by_interactive) currently triggers inside
nested module-level blocks; change its definition to require a true top-level
module context so only top-level interactive expression statements auto-print.
Concretely, update dominated_by_interactive to: self.interactive &&
!self.ctx.in_func() && !self.ctx.in_class && self.ctx.is_top_level_module(), and
add/implement a Context method is_top_level_module() that returns true only when
the current scope is the module top-level (i.e., not inside any nested
module-level blocks like if/try/with). Ensure compile_expression and the
subsequent emit of Instruction::CallIntrinsic1 { Print } remains guarded by the
revised dominated_by_interactive.

}
}
ast::Stmt::Global(_) | ast::Stmt::Nonlocal(_) => {
// Handled during symbol table construction.
Expand Down Expand Up @@ -3748,19 +3765,23 @@ impl Compiler {
});
self.current_code_info().flags |= bytecode::CodeFlags::HAS_DOCSTRING;
}
// If no docstring, don't add None to co_consts
// Note: RETURN_GENERATOR + POP_TOP for async functions is emitted in enter_scope()

// Compile body statements
self.compile_statements(body)?;

// Emit None at end if needed
// Emit implicit `return None` if the body doesn't end with return.
// Also ensure None is in co_consts even when not emitting return
// (matching CPython: functions without explicit constants always
// have None in co_consts).
match body.last() {
Some(ast::Stmt::Return(_)) => {}
_ => {
self.emit_return_const(ConstantData::None);
}
}
// Functions with no other constants should still have None in co_consts
if self.current_code_info().metadata.consts.is_empty() {
self.arg_constant(ConstantData::None);
}

// Exit scope and create function object
let code = self.exit_scope();
Expand Down Expand Up @@ -6882,6 +6903,19 @@ impl Compiler {
Ok(send_block)
}

/// Returns true if the expression is a constant with no side effects.
fn is_const_expression(expr: &ast::Expr) -> bool {
matches!(
expr,
ast::Expr::StringLiteral(_)
| ast::Expr::BytesLiteral(_)
| ast::Expr::NumberLiteral(_)
| ast::Expr::BooleanLiteral(_)
| ast::Expr::NoneLiteral(_)
| ast::Expr::EllipsisLiteral(_)
)
}

fn compile_expression(&mut self, expression: &ast::Expr) -> CompileResult<()> {
trace!("Compiling {expression:?}");
let range = expression.range();
Expand Down
12 changes: 3 additions & 9 deletions crates/codegen/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,7 @@ impl CodeInfo {
) -> crate::InternalResult<CodeObject> {
// Always fold tuple constants
self.fold_tuple_constants();
// Python only applies LOAD_SMALL_INT conversion to module-level code
// (not inside functions). Module code lacks OPTIMIZED flag.
// Note: RustPython incorrectly sets NEWLOCALS on modules, so only check OPTIMIZED
let is_module_level = !self.flags.contains(CodeFlags::OPTIMIZED);
if is_module_level {
self.convert_to_load_small_int();
}
self.convert_to_load_small_int();
self.remove_unused_consts();
self.remove_nops();

Expand Down Expand Up @@ -786,8 +780,8 @@ impl CodeInfo {
continue;
};

// Check if it's in small int range: -5 to 256 (_PY_IS_SMALL_INT)
if let Some(small) = value.to_i32().filter(|v| (-5..=256).contains(v)) {
// LOAD_SMALL_INT oparg is unsigned, so only 0..=255 can be encoded
if let Some(small) = value.to_i32().filter(|v| (0..=255).contains(v)) {
// Convert LOAD_CONST to LOAD_SMALL_INT
instr.instr = Instruction::LoadSmallInt { i: Arg::marker() }.into();
// The arg is the i32 value stored as u32 (two's complement)
Expand Down
1 change: 1 addition & 0 deletions crates/compiler-core/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ bitflags! {
const NEWLOCALS = 0x0002;
const VARARGS = 0x0004;
const VARKEYWORDS = 0x0008;
const NESTED = 0x0010;
const GENERATOR = 0x0020;
const COROUTINE = 0x0080;
const ITERABLE_COROUTINE = 0x0100;
Expand Down
11 changes: 10 additions & 1 deletion crates/vm/src/builtins/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,10 @@ impl Constructor for PyCode {
}
}

#[pyclass(with(Representable, Constructor, Comparable, Hashable), flags(HAS_WEAKREF))]
#[pyclass(
with(Representable, Constructor, Comparable, Hashable),
flags(HAS_WEAKREF)
)]
impl PyCode {
#[pygetset]
const fn co_posonlyargcount(&self) -> usize {
Expand Down Expand Up @@ -790,6 +793,7 @@ impl PyCode {
vm.ctx.new_bytes(self.code.exceptiontable.to_vec())
}

// spell-checker: ignore lnotab
// co_lnotab is intentionally not implemented.
// It was deprecated since 3.12 and scheduled for removal in 3.14.
// Use co_lines() or co_linetable instead.
Comment on lines +796 to +799

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Python co_lnotab deprecation removal version

💡 Result:

  • code.co_lnotab was deprecated in Python 3.10 by PEP 626 (use code.co_lines() instead). [1]
  • It started emitting a proper DeprecationWarning in Python 3.12. [1]
  • Although it was initially tracked as “scheduled for removal in Python 3.14” in early 3.12-era notes, that removal was postponed, and the current plan is removal in Python 3.15. [2]

So: expect co_lnotab to be removed in Python 3.15 (not 3.14). [2]

Sources: [1] (Python 3.12 “What’s New”), [2] (python.org core-dev discussion referencing the postponement PR).


Update comment with accurate deprecation timeline.

The comment states co_lnotab was "deprecated since 3.12 and scheduled for removal in 3.14." However, co_lnotab was actually deprecated in Python 3.10 (per PEP 626), with the DeprecationWarning added in Python 3.12. The scheduled removal date was also updated from Python 3.14 to Python 3.15. Consider updating the comment to reflect: "deprecated in Python 3.10, scheduled for removal in Python 3.15."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/builtins/code.rs` around lines 796 - 799, Update the inline
comment about co_lnotab to reflect the correct timeline: change the text that
currently reads "deprecated since 3.12 and scheduled for removal in 3.14" to
state that co_lnotab was deprecated in Python 3.10 (per PEP 626), had
DeprecationWarning added in Python 3.12, and is scheduled for removal in Python
3.15; adjust the surrounding sentence to still recommend using co_lines() or
co_linetable and reference co_lnotab, co_lines, and co_linetable so the intent
remains clear.

Expand Down Expand Up @@ -1065,6 +1069,11 @@ impl PyCode {
vm.call_method(list.as_object(), "__iter__", ())
}

#[pymethod]
pub fn __replace__(&self, args: ReplaceArgs, vm: &VirtualMachine) -> PyResult<Self> {
self.replace(args, vm)
}

#[pymethod]
pub fn replace(&self, args: ReplaceArgs, vm: &VirtualMachine) -> PyResult<Self> {
let ReplaceArgs {
Expand Down
Loading