Use `num_enum` crate for oparg types by ShaharNaveh ยท Pull Request #6980 ยท RustPython/RustPython

๐Ÿงฉ Analysis chain

๐Ÿ Script executed:

# First, let's see the file structure and understand UnpackExArgs
cd crates/compiler-core && head -600 src/bytecode/oparg.rs | tail -100

Repository: RustPython/RustPython

Length of output: 2741


๐Ÿ Script executed:

# Also check the full definition of UnpackExArgs
cd crates/compiler-core && rg -B5 -A5 "struct UnpackExArgs"

Repository: RustPython/RustPython

Length of output: 476


๐Ÿ Script executed:

# Search for MarshalError definition
cd crates/compiler-core && rg "enum MarshalError|struct MarshalError" -A10

Repository: RustPython/RustPython

Length of output: 477


๐Ÿ Script executed:

# Look for all usages of UnpackExArgs with From/Into
cd crates/compiler-core && rg "UnpackExArgs" -B2 -A2

Repository: RustPython/RustPython

Length of output: 2134


๐Ÿ Script executed:

# Find where UnpackExArgs is constructed or converted from u32
cd crates/compiler-core && rg "UnpackExArgs.*from|::from.*UnpackExArgs" -B3 -A3

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Search for bytecode reading/marshalling code that might use this conversion
cd crates/compiler-core && rg "oparg|OpArg" src/bytecode/instruction.rs | head -30

Repository: RustPython/RustPython

Length of output: 1763


๐Ÿ Script executed:

# Check if MarshalError is imported in oparg.rs
cd crates/compiler-core && head -50 src/bytecode/oparg.rs

Repository: RustPython/RustPython

Length of output: 1221


๐Ÿ Script executed:

# Look for Arg<UnpackExArgs> usage to understand the data flow
cd crates/compiler-core && rg "Arg<UnpackExArgs>" -B5 -A5

Repository: RustPython/RustPython

Length of output: 592


๐Ÿ Script executed:

# Check if there's a blanket impl or if TryFrom<u32> is actually required
cd crates/compiler-core && rg "impl.*OpArgType" -B2 -A2

Repository: RustPython/RustPython

Length of output: 3737


๐Ÿ Script executed:

# Search for actual usage of From for UnpackExArgs
cd crates/compiler-core && rg "\.into\(\)|From::<UnpackExArgs>" -B3 -A3

Repository: RustPython/RustPython

Length of output: 3571


๐Ÿ Script executed:

# Check how Arg<T> handles conversion and if it uses TryFrom
cd crates/compiler-core && rg "struct Arg|impl.*Arg" src/bytecode/instruction.rs | head -20

Repository: RustPython/RustPython

Length of output: 259


๐Ÿ Script executed:

# Look for where Arg values are decoded from bytecode
cd crates/compiler-core && rg "decode|from_bytes|read" src/ -l | grep -E "(decode|marshal|bytecode)" | head -5

Repository: RustPython/RustPython

Length of output: 145


๐Ÿ Script executed:

# Let's check if this code actually compiles by looking for any compiler errors or warnings
cd crates/compiler-core && cargo check --lib 2>&1 | head -50

Repository: RustPython/RustPython

Length of output: 151


๐Ÿ Script executed:

# Look at the trait definition more carefully - specifically the TryFrom requirement
cd crates/compiler-core && rg -A5 "pub trait OpArgType"

Repository: RustPython/RustPython

Length of output: 407


๐Ÿ Script executed:

# Check if there's a blanket impl of TryFrom<u32> for types with From<u32>
cd crates/compiler-core && rg "impl.*TryFrom" src/bytecode/ -B2 -A3 | head -40

Repository: RustPython/RustPython

Length of output: 1455


๐Ÿ Script executed:

# Search for where bytecode is actually deserialized and how opargs are read
cd crates/compiler-core && rg "read|decode" src/bytecode.rs -B2 -A5 | head -50

Repository: RustPython/RustPython

Length of output: 1959


๐Ÿ Script executed:

# Check if there's any blanket impl converting From to TryFrom
cd crates/compiler-core && rg "impl.*From.*TryFrom|blanket" src/ -B2 -A5

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Let's look at what MakeFunctionFlags does - it has explicit TryFrom
cd crates/compiler-core && rg -B10 "impl TryFrom<u32> for MakeFunctionFlags" src/bytecode/oparg.rs | head -30

Repository: RustPython/RustPython

Length of output: 399


๐Ÿ Script executed:

# Check the full context of UnpackExArgs - does it have a TryFrom impl elsewhere?
cd crates/compiler-core && rg -B20 -A20 "struct UnpackExArgs"

Repository: RustPython/RustPython

Length of output: 1872


๐Ÿ Script executed:

# Check if the code actually compiles by looking for any build artifacts or recent commits
cd crates/compiler-core && ls -la target/debug/*.rlib 2>/dev/null | head -5 || echo "No build artifacts found"

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Let's check if Rust's blanket impl from From to TryFrom exists and understand the exact behavior
cd crates/compiler-core && python3 << 'EOF'
# Check what TryFrom actually requires
print("Checking Rust's blanket impl...")
print("In Rust, when you impl From<T> for U, TryFrom<T> for U is automatically available")
print("because there's a blanket impl: impl<F, T> TryFrom<F> for T where T: From<F>")
print("This blanket impl calls from() and wraps it in Ok()")
print("\nSo UnpackExArgs satisfies OpArgType trait requirements via blanket impl")
print("BUT the issue is: silent truncation of high bytes is not validated")
EOF

Repository: RustPython/RustPython

Length of output: 447


๐Ÿ Script executed:

# Let's verify this is actually compiled and working by checking the actual module exports
cd crates/compiler-core && rg "pub use.*UnpackExArgs" src/ -B2 -A2

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Check if there are any runtime validation of opargs elsewhere
cd crates/compiler-core && rg "UnpackEx|oparg.*valid|validate" src/ -B2 -A2 | head -40

Repository: RustPython/RustPython

Length of output: 2036


๐Ÿ Script executed:

# Let's look at a real bytecode reader to understand the data flow
cd crates/compiler-core && rg -l "read.*oparg|parse.*instruction" src/ | head -3 | xargs cat 2>/dev/null | head -100

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Check how opcodes are actually written to bytecode - are high bits ever legitimately set?
cd crates/compiler-core && rg "encode.*oparg|write.*oparg" src/ -B3 -A3 | head -50

Repository: RustPython/RustPython

Length of output: 728


๐Ÿ Script executed:

# Look at the UNPACK_EX opcode in Python to understand what values it should accept
cd crates/compiler-core && rg -i "unpack.ex|118" src/bytecode/ -B2 -A2 | head -30

Repository: RustPython/RustPython

Length of output: 739


๐Ÿ Script executed:

# Check if there's any documentation about the bytecode format
cd crates/compiler-core && find . -name "*.md" -o -name "*.txt" | xargs grep -l "oparg\|bytecode" 2>/dev/null | head -3

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Verify that u16::try_from pattern would work and what error it produces
python3 << 'EOF'
print("Checking the proposed fix logic:")
print("1. If value is > 0xFFFF (u16::MAX), try_from fails")
print("2. This would correctly reject malformed bytecode")
print("3. Lower 16 bits are extracted as u16, then converted to [u8; 2]")
print("\nExample:")
print("  value = 0x1234_5678 -> u16::try_from fails (good!)")
print("  value = 0x0000_1234 -> u16 = 0x1234 -> bytes = [0x34, 0x12]")
print("\nThe review comment's proposed fix is sound")
EOF

Repository: RustPython/RustPython

Length of output: 432


๐Ÿ Script executed:

# Let's verify no direct usage of From::from() for UnpackExArgs that would break
cd crates/compiler-core && rg "From::<UnpackExArgs>|from\(\)" src/ -B2 -A2 | head -30

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Check how the bytecode marshalling actually works - does it use the trait generically?
cd crates/compiler-core && rg "decode.*instruction|read.*oparg" src/bytecode.rs -B5 -A10 | head -60

Repository: RustPython/RustPython

Length of output: 47


๐Ÿ Script executed:

# Let's look at the actual bytecode format to understand if high bits should ever be non-zero
python3 << 'EOF'
print("Understanding UNPACK_EX bytecode format:")
print("- Python bytecode uses variable-length encoding with EXTENDED_ARG")
print("- UNPACK_EX stores 'before' and 'after' counts in lower 16 bits")
print("- Each count is an 8-bit value (0-255)")
print("- So the valid range is 0x0000-0xFFFF, where bytes map to [before, after]")
print("\nIf high 16 bits (bytes 2-3) are non-zero, it's malformed bytecode")
print("The proposed u16::try_from would correctly reject values > 0xFFFF")
EOF

Repository: RustPython/RustPython

Length of output: 481


Validate high bits when decoding UnpackExArgs.

From<u32> silently drops the upper two bytes. Since UnpackExArgs only needs 16 bits (two u8 fields), adopt a checked TryFrom<u32> implementation to reject malformed bytecode instead of truncating silently.

Aligns with Rust best practices for error handling and the existing pattern in this codebase (e.g., MakeFunctionFlags).