-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Newtype ConstIdx, Constants #7358
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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}; | ||
|
|
@@ -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)); | ||
|
|
@@ -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) | ||
|
|
@@ -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()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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 -5Repository: 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 -20Repository: 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.rsRepository: 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 -20Repository: 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 -30Repository: 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 -50Repository: 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.rsRepository: 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_modulesRepository: 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.rsRepository: 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 At line 197, 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 |
||
| } | ||
| Instruction::LoadName { namei } => self | ||
| .stack | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.