Skip to content

Resolve test_inspect bytecode parity gaps #7926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 2 commits into from
May 19, 2026
Merged

Resolve test_inspect bytecode parity gaps #7926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddecd7f
Resolve test_inspect bytecode parity gaps
youknowone May 19, 2026
13549d8
Address code review feedback
youknowone May 19, 2026
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
1 change: 0 additions & 1 deletion Lib/test/test_inspect/test_inspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,6 @@ def test_getsource_on_generated_class(self):
self.assertRaises(OSError, inspect.getsourcelines, A)
self.assertIsNone(inspect.getcomments(A))

@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: OSError not raised by getsource
def test_getsource_on_class_without_firstlineno(self):
__firstlineno__ = 1
class C:
Expand Down
177 changes: 132 additions & 45 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2723,7 +2723,7 @@ impl Compiler {
let mut parent_idx = stack_size - 2;
let mut parent = &self.code_stack[parent_idx];

let parent_scope = self
let mut parent_scope = self
.symbol_table_stack
.get(parent_idx)
.map(|table| table.typ);
Expand All @@ -2742,6 +2742,10 @@ impl Compiler {
// Use grandparent
parent_idx = stack_size - 3;
parent = &self.code_stack[parent_idx];
parent_scope = self
.symbol_table_stack
.get(parent_idx)
.map(|table| table.typ);
}

// Check if this is a global class/function
Expand Down Expand Up @@ -2779,9 +2783,12 @@ impl Compiler {
let parent_obj_name = &parent.metadata.name;

// Determine if parent is a function-like scope
let is_function_parent = parent.flags.contains(bytecode::CodeFlags::OPTIMIZED)
&& !parent_obj_name.starts_with("<") // Not a special scope like <lambda>, <listcomp>, etc.
&& parent_obj_name != "<module>"; // Not the module scope
let is_function_parent = matches!(
parent_scope,
Some(
CompilerScope::Function | CompilerScope::AsyncFunction | CompilerScope::Lambda
)
);

if is_function_parent {
// For functions, append .<locals> to parent qualname
Expand Down Expand Up @@ -3251,6 +3258,18 @@ impl Compiler {
CompilerScope::Annotation | CompilerScope::TypeParams
) {
SymbolScope::GlobalImplicit
} else if matches!(
name.as_ref(),
"__name__"
| "__module__"
| "__qualname__"
| "__firstlineno__"
| "__doc__"
| "__static_attributes__"
| "__classdictcell__"
| "__classcell__"
) {
SymbolScope::Unknown
} else {
return Err(self.error(CodegenErrorType::SyntaxError(format!(
"the symbol '{name}' must be present in the symbol table"
Expand Down Expand Up @@ -6613,39 +6632,20 @@ impl Compiler {
let (doc_str, body) = split_doc(body, &self.opts);

// Load __name__ and store as __module__
let dunder_name = self.name("__name__");
emit!(self, Instruction::LoadName { namei: dunder_name });
let dunder_module = self.name("__module__");
emit!(
self,
Instruction::StoreName {
namei: dunder_module
}
);
self.load_name("__name__")?;
self.store_name("__module__")?;

// Store __qualname__
self.emit_load_const(ConstantData::Str {
value: qualname.into(),
});
let qualname_name = self.name("__qualname__");
emit!(
self,
Instruction::StoreName {
namei: qualname_name
}
);
self.store_name("__qualname__")?;

// Store __firstlineno__ before __doc__
self.emit_load_const(ConstantData::Integer {
value: BigInt::from(firstlineno),
});
let firstlineno_name = self.name("__firstlineno__");
emit!(
self,
Instruction::StoreName {
namei: firstlineno_name
}
);
self.store_name("__firstlineno__")?;

// Set __type_params__ from the enclosing type-params closure when
// compiling a generic class body.
Expand Down Expand Up @@ -6677,8 +6677,7 @@ impl Compiler {
// Store __doc__ only if there's an explicit docstring.
if let Some(doc) = doc_str {
self.emit_load_const(ConstantData::Str { value: doc.into() });
let doc_name = self.name("__doc__");
emit!(self, Instruction::StoreName { namei: doc_name });
self.store_name("__doc__")?;
}

// 3. Compile the class body
Expand Down Expand Up @@ -6716,13 +6715,7 @@ impl Compiler {
.collect(),
});
self.set_no_location();
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In CPython, does the compiler always store the synthetic classcellinto the class locals namespace, even when the class body declaresglobal classcellornonlocal classcell?

💡 Result:

No—CPython does not always end up storing the synthetic classcell into the class-body locals namespace when the class body declares global classcell or nonlocal classcell. In CPython, the zero-argument super() mechanism is implemented by injecting a hidden class / classcell cell during compilation, but CPython also standardizes behavior so that class and (in 3.6+) classcell do not show up in the class locals while the class body executes [1]. Additionally, the generator/symbol-table machinery for class/classcell interacts “strangely” with explicit nonlocal class in nested namespaces: the injection behaves like a closure/free variable and can be accessed via nonlocal from a nested class, without being treated as a normal local in the class-body locals mapping [1]. This directly implies that declaring nonlocal classcell (analogous to the class case discussed) prevents classcell from being stored like a regular local entry in the class locals namespace. So, when you explicitly declare global classcell or nonlocal classcell, CPython’s compiler/symbol-table handling will not necessarily (and in these cases generally does not) place the synthetic classcell into the class locals mapping the way it would for a normal class body without such declarations; instead the injected cell remains hidden/handled specially for LOAD_BUILD_CLASS and zero-argument super() semantics [1].

Citations:

🏁 Script executed:

# First, check the file size and read the relevant sections
wc -l crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 101

🏁 Script executed:

# Read around line 6717
sed -n '6710,6725p' crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 690

🏁 Script executed:

# Read around line 6731
sed -n '6724,6738p' crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 557

🏁 Script executed:

# Find the store_name method definition to understand its behavior
rg -n "fn store_name" crates/codegen/src/compile.rs | head -5

Repository: RustPython/RustPython

Length of output: 134

🏁 Script executed:

# Search for any special handling of __classcell__ or __classdictcell__
rg -n "__classcell__|__classdictcell__" crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 661

🏁 Script executed:

# Read the store_name implementation
sed -n '3146,3160p' crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 684

🏁 Script executed:

# Search for any method that might do unconditional class-local stores
rg -n "store_.*local|store_cell|STORE_DEREF|STORE_GLOBAL" crates/codegen/src/compile.rs | head -20

Repository: RustPython/RustPython

Length of output: 380

🏁 Script executed:

# Get broader context around line 6717 - understand the class body compilation
sed -n '6690,6745p' crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 1960

🏁 Script executed:

# Find compile_name implementation
rg -n "fn compile_name" crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 154

🏁 Script executed:

# Search for enum NameUsage to understand Store variant
rg -n "enum NameUsage" crates/codegen/src/compile.rs -A 10

Repository: RustPython/RustPython

Length of output: 386

🏁 Script executed:

# Look for direct instruction emitters that might bypass name resolution
rg -n "Instruction::Store|PseudoInstruction::Store" crates/codegen/src/compile.rs | head -20

Repository: RustPython/RustPython

Length of output: 1639

🏁 Script executed:

# Check how __static_attributes__ is handled (line 6707 shows it uses store_name too)
# Let's see if there's any special handling for class protocol names
rg -n "__staticattr|__firstlineno__|__module__|__qualname__" crates/codegen/src/compile.rs | head -20

Repository: RustPython/RustPython

Length of output: 1309

🏁 Script executed:

# Confirm that __classdict__ uses direct STORE_DEREF
sed -n '6648,6656p' crates/codegen/src/compile.rs

Repository: RustPython/RustPython

Length of output: 502

🏁 Script executed:

# Check what the exact instruction types are for these stores
rg -n "Instruction::(StoreDeref|StoreName|StoreGlobal|StoreFast)" crates/codegen/src/compile.rs | grep -E "6(6|7)[0-9]{2}:" | head -20

Repository: RustPython/RustPython

Length of output: 234

Use unconditional class-local stores for __classcell__ and __classdictcell__ instead of routing through store_name().

These are class-creation protocol slots that must always be stored to the class namespace, regardless of global or nonlocal declarations in the class body. Routing them through store_name() causes them to honor such declarations, converting them to STORE_DEREF or STORE_GLOBAL, which prevents the class namespace from receiving the cells that __build_class__ expects. Note that __classdict__ is already correctly handled with a direct Instruction::StoreDeref (line 6652); apply the same pattern to both locations.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/codegen/src/compile.rs` at line 6717, Replace the call to
self.store_name("__classdictcell__") with an unconditional class-local store
like you do for "__classdict__" (emit Instruction::StoreDeref or the equivalent
direct store-deref bytecode) so the name is always written into the class
namespace regardless of global/nonlocal declarations; likewise find the parallel
location handling "__classcell__" and change its
self.store_name("__classcell__") to the same direct StoreDeref-style emission.
Locate these in compile.rs near the class-creation handling (the site where
"__classdict__" is already handled with Instruction::StoreDeref) and ensure both
special slots bypass store_name() and are stored unconditionally into the class
namespace.

let static_attrs_name = self.name("__static_attributes__");
emit!(
self,
Instruction::StoreName {
namei: static_attrs_name
}
);
self.store_name("__static_attributes__")?;
self.set_no_location();
}

Expand All @@ -6731,13 +6724,7 @@ impl Compiler {
let classdict_idx = u32::from(self.get_cell_var_index("__classdict__"));
emit!(self, PseudoInstruction::LoadClosure { i: classdict_idx });
self.set_no_location();
let classdictcell = self.name("__classdictcell__");
emit!(
self,
Instruction::StoreName {
namei: classdictcell
}
);
self.store_name("__classdictcell__")?;
self.set_no_location();
}

Expand All @@ -6751,8 +6738,7 @@ impl Compiler {
self.set_no_location();
emit!(self, Instruction::Copy { i: 1 });
self.set_no_location();
let classcell = self.name("__classcell__");
emit!(self, Instruction::StoreName { namei: classcell });
self.store_name("__classcell__")?;
self.set_no_location();
} else {
self.emit_load_const(ConstantData::None);
Expand Down Expand Up @@ -14008,6 +13994,23 @@ def f(buffer, pos, last_char):
})
}

fn localsplus_name(code: &CodeObject, idx: usize) -> Option<&str> {
if idx < code.varnames.len() {
return Some(code.varnames[idx].as_str());
}

let mut extra_idx = idx - code.varnames.len();
for cellvar in &code.cellvars {
if !code.varnames.iter().any(|varname| varname == cellvar) {
if extra_idx == 0 {
return Some(cellvar.as_str());
}
extra_idx -= 1;
}
}
code.freevars.get(extra_idx).map(|name| name.as_str())
}

fn has_common_constant(code: &CodeObject, expected: bytecode::CommonConstant) -> bool {
code.instructions.iter().any(|unit| match unit.op {
Instruction::LoadCommonConstant { idx } => {
Expand Down Expand Up @@ -20934,6 +20937,90 @@ class C:
}
}

#[test]
fn test_class_firstlineno_store_uses_name_resolution() {
let code = compile_exec(
"\
def f():
__firstlineno__ = 1
class C:
nonlocal __firstlineno__
return C
",
);
let class_code = find_code(&code, "C").expect("missing class code");

assert!(
class_code
.freevars
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.iter()
.any(|name| name == "__firstlineno__"),
"class should close over nonlocal __firstlineno__, got freevars={:?}",
class_code.freevars
);
assert!(
class_code.instructions.iter().any(|unit| match unit.op {
Instruction::StoreDeref { i } => {
let idx = i.get(OpArg::new(u32::from(u8::from(unit.arg)))).as_usize();
localsplus_name(class_code, idx) == Some("__firstlineno__")
}
_ => false,
}),
"CPython routes __firstlineno__ through name resolution and emits STORE_DEREF for __firstlineno__, got ops={:?} freevars={:?}",
class_code.instructions,
class_code.freevars
);
assert!(
!class_code.instructions.iter().any(|unit| {
matches!(
unit.op,
Instruction::StoreName { namei }
if class_code.names
[namei.get(OpArg::new(u32::from(u8::from(unit.arg)))) as usize]
.as_str()
== "__firstlineno__"
)
}),
"nonlocal __firstlineno__ should not be stored with STORE_NAME, got ops={:?}",
class_code.instructions
);
}

#[test]
fn test_lambda_parent_qualname_includes_locals() {
let code = compile_exec(
"\
def f():
return lambda: (lambda: None)
",
);
let mut lambda_qualnames = Vec::new();
fn collect_lambda_qualnames(code: &CodeObject, out: &mut Vec<String>) {
if code.obj_name == "<lambda>" {
out.push(code.qualname.to_string());
}
for constant in code.constants.iter() {
if let ConstantData::Code { code } = constant {
collect_lambda_qualnames(code, out);
}
}
}
collect_lambda_qualnames(&code, &mut lambda_qualnames);

assert!(
lambda_qualnames
.iter()
.any(|name| name == "f.<locals>.<lambda>"),
"missing outer lambda qualname, got {lambda_qualnames:?}"
);
assert!(
lambda_qualnames
.iter()
.any(|name| name == "f.<locals>.<lambda>.<locals>.<lambda>"),
"nested lambda parent should include .<locals> like CPython, got {lambda_qualnames:?}"
);
}

#[test]
fn test_future_annotations_class_uses_direct_annotation_store() {
let code = compile_exec(
Expand Down
Loading