X Tutup
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions crates/codegen/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use rustpython_compiler_core::{
self, AnyInstruction, Arg as OpArgMarker, BinaryOperator, BuildSliceArgCount, CodeObject,
ComparisonOperator, ConstantData, ConvertValueOparg, Instruction, IntrinsicFunction1,
Invert, LoadAttr, LoadSuperAttr, OpArg, OpArgType, PseudoInstruction, SpecialMethod,
UnpackExArgs,
UnpackExArgs, oparg,
},
};
use rustpython_wtf8::Wtf8Buf;
Expand Down Expand Up @@ -8075,9 +8075,9 @@ impl Compiler {

// fn block_done()

fn arg_constant(&mut self, constant: ConstantData) -> u32 {
fn arg_constant(&mut self, constant: ConstantData) -> oparg::ConstIdx {
let info = self.current_code_info();
info.metadata.consts.insert_full(constant).0.to_u32()
info.metadata.consts.insert_full(constant).0.to_u32().into()
}

fn emit_load_const(&mut self, constant: ConstantData) {
Expand Down
16 changes: 10 additions & 6 deletions crates/codegen/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustpython_compiler_core::{
bytecode::{
AnyInstruction, Arg, CodeFlags, CodeObject, CodeUnit, CodeUnits, ConstantData,
ExceptionTableEntry, InstrDisplayContext, Instruction, InstructionMetadata, Label, OpArg,
PseudoInstruction, PyCodeLocationInfoKind, encode_exception_table,
PseudoInstruction, PyCodeLocationInfoKind, encode_exception_table, oparg,
},
varint::{write_signed_varint, write_varint},
};
Expand Down Expand Up @@ -695,15 +695,15 @@ impl CodeInfo {
}
(Instruction::LoadConst { consti }, Instruction::ToBool) => {
let consti = consti.get(curr.arg);
let constant = &self.metadata.consts[consti as usize];
let constant = &self.metadata.consts[consti.as_usize()];
if let ConstantData::Boolean { .. } = constant {
Some((curr_instr, OpArg::from(consti)))
Some((curr_instr, OpArg::from(consti.as_u32())))
} else {
None
}
}
(Instruction::LoadConst { consti }, Instruction::UnaryNot) => {
let constant = &self.metadata.consts[consti.get(curr.arg) as usize];
let constant = &self.metadata.consts[consti.get(curr.arg).as_usize()];
match constant {
ConstantData::Boolean { value } => {
let (const_idx, _) = self
Expand Down Expand Up @@ -1100,15 +1100,19 @@ impl CodeInfo {

impl InstrDisplayContext for CodeInfo {
type Constant = ConstantData;
fn get_constant(&self, i: usize) -> &ConstantData {
&self.metadata.consts[i]

fn get_constant(&self, consti: oparg::ConstIdx) -> &ConstantData {
&self.metadata.consts[consti.as_usize()]
}

fn get_name(&self, i: usize) -> &str {
self.metadata.names[i].as_ref()
}

fn get_varname(&self, i: usize) -> &str {
self.metadata.varnames[i].as_ref()
}

fn get_cell_name(&self, i: usize) -> &str {
self.metadata
.cellvars
Expand Down
40 changes: 32 additions & 8 deletions crates/compiler-core/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use bitflags::bitflags;
use core::{
cell::UnsafeCell,
hash, mem,
ops::Deref,
ops::{Deref, Index},
sync::atomic::{AtomicU8, AtomicU16, AtomicUsize, Ordering},
};
use itertools::Itertools;
Expand All @@ -32,7 +32,7 @@ pub use crate::bytecode::{
};

mod instruction;
mod oparg;
pub mod oparg;

/// Exception table entry for zero-cost exception handling
/// Format: (start, size, target, depth<<1|lasti)
Expand Down Expand Up @@ -293,6 +293,31 @@ impl ConstantBag for BasicBag {
}
}

#[derive(Clone)]
pub struct Constants<C: Constant>(Box<[C]>);

impl<C: Constant> Deref for Constants<C> {
type Target = [C];

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<C: Constant> Index<oparg::ConstIdx> for Constants<C> {
type Output = C;

fn index(&self, consti: oparg::ConstIdx) -> &Self::Output {
&self.0[consti.as_usize()]
}
}

impl<C: Constant> FromIterator<C> for Constants<C> {
fn from_iter<T: IntoIterator<Item = C>>(iter: T) -> Self {
Self(iter.into_iter().collect())
}
}

/// Primary container of a single code object. Each python function has
/// a code object. Also a module has a code object.
#[derive(Clone)]
Expand All @@ -312,7 +337,7 @@ pub struct CodeObject<C: Constant = ConstantData> {
/// Qualified name of the object (like CPython's co_qualname)
pub qualname: C::Name,
pub cell2arg: Option<Box<[i32]>>,
pub constants: Box<[C]>,
pub constants: Constants<C>,
pub names: Box<[C::Name]>,
pub varnames: Box<[C::Name]>,
pub cellvars: Box<[C::Name]>,
Expand Down Expand Up @@ -1020,8 +1045,7 @@ impl<C: Constant> CodeObject<C> {
CodeObject {
constants: self
.constants
.into_vec()
.into_iter()
.iter()
.map(|x| bag.make_constant(x.borrow_constant()))
.collect(),
names: map_names(self.names),
Expand Down Expand Up @@ -1095,7 +1119,7 @@ impl<C: Constant> fmt::Display for CodeObject<C> {
pub trait InstrDisplayContext {
type Constant: Constant;

fn get_constant(&self, i: usize) -> &Self::Constant;
fn get_constant(&self, consti: oparg::ConstIdx) -> &Self::Constant;

fn get_name(&self, i: usize) -> &str;

Expand All @@ -1107,8 +1131,8 @@ pub trait InstrDisplayContext {
impl<C: Constant> InstrDisplayContext for CodeObject<C> {
type Constant = C;

fn get_constant(&self, i: usize) -> &C {
&self.constants[i]
fn get_constant(&self, consti: oparg::ConstIdx) -> &C {
&self.constants[consti]
}

fn get_name(&self, i: usize) -> &str {
Expand Down
37 changes: 20 additions & 17 deletions crates/compiler-core/src/bytecode/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
bytecode::{
BorrowedConstant, Constant, InstrDisplayContext,
oparg::{
BinaryOperator, BuildSliceArgCount, CommonConstant, ComparisonOperator,
self, BinaryOperator, BuildSliceArgCount, CommonConstant, ComparisonOperator,
ConvertValueOparg, IntrinsicFunction1, IntrinsicFunction2, Invert, Label, LoadAttr,
LoadSuperAttr, MakeFunctionFlags, NameIdx, OpArg, OpArgByte, OpArgType, RaiseKind,
SpecialMethod, StoreFastLoadFast, UnpackExArgs,
Expand Down Expand Up @@ -186,7 +186,7 @@ pub enum Instruction {
idx: Arg<CommonConstant>,
} = 81,
LoadConst {
consti: Arg<u32>,
consti: Arg<oparg::ConstIdx>,
} = 82,
LoadDeref {
i: Arg<NameIdx>,
Expand Down Expand Up @@ -1124,22 +1124,25 @@ impl InstructionMetadata for Instruction {
let name = |i: u32| ctx.get_name(i as usize);
let cell_name = |i: u32| ctx.get_cell_name(i as usize);

let fmt_const =
|op: &str, arg: OpArg, f: &mut fmt::Formatter<'_>, idx: &Arg<u32>| -> fmt::Result {
let value = ctx.get_constant(idx.get(arg) as usize);
match value.borrow_constant() {
BorrowedConstant::Code { code } if expand_code_objects => {
write!(f, "{op:pad$}({code:?}):")?;
code.display_inner(f, true, level + 1)?;
Ok(())
}
c => {
write!(f, "{op:pad$}(")?;
c.fmt_display(f)?;
write!(f, ")")
}
let fmt_const = |op: &str,
arg: OpArg,
f: &mut fmt::Formatter<'_>,
consti: &Arg<oparg::ConstIdx>|
-> fmt::Result {
let value = ctx.get_constant(consti.get(arg));
match value.borrow_constant() {
BorrowedConstant::Code { code } if expand_code_objects => {
write!(f, "{op:pad$}({code:?}):")?;
code.display_inner(f, true, level + 1)?;
Ok(())
}
};
c => {
write!(f, "{op:pad$}(")?;
c.fmt_display(f)?;
write!(f, ")")
}
}
};

match self {
Self::BinarySlice => w!(BINARY_SLICE),
Expand Down
36 changes: 36 additions & 0 deletions crates/compiler-core/src/bytecode/oparg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,3 +872,39 @@ impl LoadAttrBuilder {
self
}
}

#[derive(Clone, Copy)]
pub struct ConstIdx(u32);

impl ConstIdx {
#[must_use]
pub const fn from_u32(value: u32) -> Self {
Self(value)
}

/// Returns the index as a `u32` value.
#[must_use]
pub const fn as_u32(self) -> u32 {
self.0
}

/// Returns the index as a `usize` value.
#[must_use]
pub const fn as_usize(self) -> usize {
self.0 as usize
}
}

impl From<u32> for ConstIdx {
fn from(value: u32) -> Self {
Self::from_u32(value)
}
}

impl From<ConstIdx> for u32 {
fn from(consti: ConstIdx) -> Self {
consti.as_u32()
}
}

impl OpArgType for ConstIdx {}
2 changes: 1 addition & 1 deletion crates/compiler-core/src/marshal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ pub fn deserialize_code<R: Read, Bag: ConstantBag>(
let len = rdr.read_u32()?;
let constants = (0..len)
.map(|_| deserialize_value(rdr, bag))
.collect::<Result<Box<[_]>>>()?;
.collect::<Result<_>>()?;

let mut read_names = || {
let len = rdr.read_u32()?;
Expand Down
5 changes: 2 additions & 3 deletions crates/jit/src/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -637,9 +637,8 @@ impl<'a, 'b> FunctionCompiler<'a, 'b> {
Ok(())
}
Instruction::LoadConst { consti } => {
let val = self.prepare_const(
bytecode.constants[consti.get(arg) as usize].borrow_constant(),
)?;
let val =
self.prepare_const(bytecode.constants[consti.get(arg)].borrow_constant())?;
self.stack.push(val);
Ok(())
}
Expand Down
12 changes: 6 additions & 6 deletions crates/jit/tests/common.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use core::ops::ControlFlow;
use rustpython_compiler_core::bytecode::{
CodeObject, ConstantData, Instruction, OpArg, OpArgState,
CodeObject, ConstantData, Constants, Instruction, OpArg, OpArgState,
};
use rustpython_jit::{CompiledCode, JitType};
use rustpython_wtf8::{Wtf8, Wtf8Buf};
Expand Down Expand Up @@ -77,7 +77,7 @@ fn extract_annotations_from_annotate_code(code: &CodeObject) -> HashMap<Wtf8Buf,

match instruction {
Instruction::LoadConst { consti } => {
stack.push((true, consti.get(arg) as usize));
stack.push((true, consti.get(arg).as_usize()));
}
Instruction::LoadName { namei } => {
stack.push((false, namei.get(arg) as usize));
Expand All @@ -99,7 +99,8 @@ fn extract_annotations_from_annotate_code(code: &CodeObject) -> HashMap<Wtf8Buf,

// Key should be a const string (parameter name)
if key_is_const
&& let ConstantData::Str { value } = &code.constants[key_idx]
&& let ConstantData::Str { value } =
&code.constants[(key_idx as u32).into()]
{
let param_name = value;
// Value can be a name (type ref) or a const string (forward ref)
Expand Down Expand Up @@ -185,16 +186,15 @@ impl StackMachine {
&mut self,
instruction: Instruction,
arg: OpArg,
constants: &[ConstantData],
constants: &Constants<ConstantData>,
names: &[String],
) -> ControlFlow<()> {
match instruction {
Instruction::Resume { .. } | Instruction::Cache | Instruction::NotTaken => {
// No-op for JIT tests
}
Instruction::LoadConst { consti } => {
let idx = consti.get(arg);
self.stack.push(constants[idx as usize].clone().into())
self.stack.push(constants[consti.get(arg)].clone().into())
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 | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# First, let's look at the exact line and surrounding context
echo "=== Line 197 and surrounding context ==="
sed -n '190,205p' crates/jit/tests/common.rs

# Show the function signature
echo -e "\n=== Function containing this code ==="
rg -n -B10 'Instruction::LoadConst' crates/jit/tests/common.rs | head -40

# Check type of consti and what .get() returns
echo -e "\n=== Looking for ConstIdx definition and methods ==="
fd -e rs | xargs rg -l 'struct ConstIdx\|type ConstIdx' 2>/dev/null | head -5

Repository: RustPython/RustPython

Length of output: 1918


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where Instruction enum is defined
echo "=== Finding Instruction enum definition ==="
fd -e rs | xargs rg -l 'enum Instruction' 2>/dev/null | head -3

# Find ConstIdx type
echo -e "\n=== Finding ConstIdx type ==="
fd -e rs | xargs rg 'struct ConstIdx|type ConstIdx' 2>/dev/null | head -10

# Look at the Instruction enum definition for LoadConst
echo -e "\n=== LoadConst variant definition ==="
fd -e rs | xargs rg -A5 'LoadConst.*consti' 2>/dev/null | head -20

Repository: RustPython/RustPython

Length of output: 1694


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find ConstIdx and its methods
echo "=== ConstIdx definition and methods ==="
cat crates/compiler-core/src/bytecode/oparg.rs | head -100

# Look for IndexOpArg trait or similar
echo -e "\n=== Looking for IndexOpArg trait/implementations ==="
rg -n 'trait.*OpArg|trait.*Index' crates/compiler-core/src/bytecode/oparg.rs | head -20
</script>

Repository: RustPython/RustPython

Length of output: 2597


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find ConstIdx struct definition
echo "=== ConstIdx struct definition ==="
rg -A 30 'pub struct ConstIdx' crates/compiler-core/src/bytecode/oparg.rs

# Find all methods on ConstIdx
echo -e "\n=== ConstIdx impl blocks ==="
rg -A 20 'impl.*ConstIdx' crates/compiler-core/src/bytecode/oparg.rs

Repository: RustPython/RustPython

Length of output: 1328


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the .get() method - it might be in IndexOpArg trait or similar
echo "=== Searching for .get() method on index types ==="
rg -B5 -A10 'fn get\(' crates/compiler-core/src/bytecode/oparg.rs

# Check what trait provides the .get() method
echo -e "\n=== IndexOpArg or similar trait ==="
rg 'trait.*Index|IndexOpArg' crates/compiler-core/src/bytecode/ -r
</script>

Repository: RustPython/RustPython

Length of output: 1008


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for IndexOpArg trait definition
echo "=== Finding IndexOpArg trait ==="
rg 'trait IndexOpArg' crates/compiler-core/src/bytecode/

# Search for the .get() method definition more broadly
echo -e "\n=== Searching for .get() method with OpArg parameter ==="
rg 'fn get\(.*OpArg' crates/compiler-core/src/

# Look at the actual Instruction enum to understand the field types
echo -e "\n=== Looking at Instruction enum LoadConst definition ==="
rg -A5 'LoadConst.*{' crates/compiler-core/src/bytecode/instruction.rs | head -20

Repository: RustPython/RustPython

Length of output: 97


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Let's look at what .get() is being called on consti
# First, let's see the full context around line 197 in common.rs
echo "=== Full context around line 197 ==="
sed -n '175,210p' crates/jit/tests/common.rs

# Search for the trait that provides .get() method
echo -e "\n=== Search for trait defining .get() method ==="
rg 'trait.*get|fn get.*OpArg' crates/compiler-core/src/bytecode/

# Let's check the Instruction type definition
echo -e "\n=== Instruction enum with LoadConst ==="
rg -B2 -A2 'LoadConst' crates/compiler-core/src/bytecode/instruction.rs | head -30

Repository: RustPython/RustPython

Length of output: 3191


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Now let's look at the Arg type and its .get() method
echo "=== Arg type definition and .get() method ==="
rg -B5 -A15 'impl.*Arg.*{' crates/compiler-core/src/bytecode/instruction.rs | grep -A15 'pub fn get'

# Let's see the full Arg definition
echo -e "\n=== Complete Arg struct definition ==="
rg -B10 -A30 'pub struct Arg' crates/compiler-core/src/bytecode/instruction.rs | head -50

Repository: RustPython/RustPython

Length of output: 215


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look at Arg type and its implementation
echo "=== Arg struct and impl ==="
rg -B5 -A20 'pub struct Arg' crates/compiler-core/src/bytecode/instruction.rs

# Find the .get method on Arg
echo -e "\n=== .get method on Arg ==="
rg -B2 -A10 'pub fn get\(self, arg: OpArg\)' crates/compiler-core/src/bytecode/instruction.rs

Repository: RustPython/RustPython

Length of output: 998


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check if ConstIdx implements Index trait for slices
echo "=== Check for Index impl with ConstIdx ==="
rg 'impl.*Index.*ConstIdx|impl.*Index.*Arg' crates/compiler-core/src/bytecode/

# Check what type self.code.constants is in vm/src/frame.rs
echo -e "\n=== Type of code.constants in frame.rs ==="
rg -B10 'self.code.constants\[consti' crates/vm/src/frame.rs | head -20

# Look at CodeObject to see the type of its constants field
echo -e "\n=== CodeObject definition ==="
rg -A10 'pub struct CodeObject' crates/compiler-core/src/

Repository: RustPython/RustPython

Length of output: 107


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Let's verify if the code actually compiles
echo "=== Attempting to compile crates/jit/tests/common.rs ==="
cd crates/jit && cargo check 2>&1 | grep -A20 "error\|warning" | head -40

# Also check what the actual error is on line 197
echo -e "\n=== Checking line 197 compilation directly ==="
cd - > /dev/null
rg -B3 -A3 'constants\[consti.get' crates/jit/tests/common.rs

# Check if there's a custom Index implementation anywhere
echo -e "\n=== All Index implementations in codebase ==="
rg 'impl.*Index.*for' crates/ --type rs | grep -v node_modules

Repository: RustPython/RustPython

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Simply check what type .get() returns and if it can index a slice
echo "=== Return type of Arg<ConstIdx>::get() ==="
echo "From the code we've seen:"
echo "- Arg<T>::get(self, arg: OpArg) -> T"
echo "- For Arg<ConstIdx>, this means .get() returns ConstIdx"
echo ""
echo "- ConstIdx does NOT implement Deref, Index, or any trait that would let it index a slice"
echo "- ConstIdx only has: as_u32(), as_usize(), and From<u32>"
echo ""

# Verify this by checking if ConstIdx can be used directly as slice index
echo "=== Looking at the two usages in crates/jit/tests/common.rs ==="
rg -n 'consti.get' crates/jit/tests/common.rs

# Check if constants in codebase is always a slice or Vec
echo -e "\n=== How constants is typed in function signature ==="
rg -B2 'constants:.*\[ConstantData\]' crates/jit/tests/common.rs

Repository: RustPython/RustPython

Length of output: 729


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify the exact current state of line 197
echo "=== Exact lines 195-200 from common.rs ==="
sed -n '195,200p' crates/jit/tests/common.rs

# Check git history to see if this is a recent change
echo -e "\n=== Recent changes to this file ==="
cd crates/jit/tests && git log --oneline -20 common.rs 2>/dev/null || echo "Git not available"
cd - > /dev/null

# Summary of type facts
echo -e "\n=== Type Analysis Summary ==="
echo "ConstIdx::get(arg: OpArg) returns: ConstIdx"
echo "ConstIdx has NO Index trait implementation"
echo "Rust slices require usize for indexing"
echo "Line 80 correctly uses: consti.get(arg).as_usize()"
echo "Line 197 incorrectly uses: constants[consti.get(arg)]"
echo ""
echo "This WILL NOT COMPILE because ConstIdx cannot be used to index a slice."

Repository: RustPython/RustPython

Length of output: 794


Fix type mismatch in LoadConst slice indexing.

At line 197, consti.get(arg) returns ConstIdx, which cannot directly index a slice. Slices require usize indexing. Use .as_usize() to convert:

Suggested fix
             Instruction::LoadConst { consti } => {
-                self.stack.push(constants[consti.get(arg)].clone().into())
+                self.stack.push(constants[consti.get(arg).as_usize()].clone().into())
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/jit/tests/common.rs` at line 197, The slice indexing uses a ConstIdx
returned by consti.get(arg) which can't index a slice; update the indexing in
the LoadConst site so you convert the ConstIdx to usize (e.g., call .as_usize()
on the ConstIdx) before indexing the constants slice; modify the expression that
builds the pushed value (the call in self.stack.push(...)) to index constants
with consti.get(arg).as_usize() (or an equivalent usize conversion) so the types
match.

}
Instruction::LoadName { namei } => self
.stack
Expand Down
8 changes: 3 additions & 5 deletions crates/vm/src/builtins/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,16 +538,14 @@ impl Constructor for PyCode {
.map_err(|e| vm.new_value_error(format!("invalid bytecode: {}", e)))?;

// Convert constants
let constants: Box<[Literal]> = args
let constants = args
.consts
.iter()
.map(|obj| {
// Convert PyObject to Literal constant
// For now, just wrap it
// Convert PyObject to Literal constant. For now, just wrap it
Literal(obj.clone())
})
.collect::<Vec<_>>()
.into_boxed_slice();
.collect();

// Create locations (start and end pairs)
let row = if args.firstlineno > 0 {
Expand Down
6 changes: 3 additions & 3 deletions crates/vm/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2351,8 +2351,8 @@ impl ExecutingFrame<'_> {
});
Ok(None)
}
Instruction::LoadConst { consti: idx } => {
self.push_value(self.code.constants[idx.get(arg) as usize].clone().into());
Instruction::LoadConst { consti } => {
self.push_value(self.code.constants[consti.get(arg)].clone().into());
// Mirror CPython's LOAD_CONST family transition. RustPython does
// not currently distinguish immortal constants at runtime.
let instr_idx = self.lasti() as usize - 1;
Expand All @@ -2364,7 +2364,7 @@ impl ExecutingFrame<'_> {
Ok(None)
}
Instruction::LoadConstMortal | Instruction::LoadConstImmortal => {
self.push_value(self.code.constants[u32::from(arg) as usize].clone().into());
self.push_value(self.code.constants[u32::from(arg).into()].clone().into());
Ok(None)
}
Instruction::LoadCommonConstant { idx } => {
Expand Down
Loading
X Tutup