| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -245,10 +245,7 @@ pub enum Instruction { | |
| YieldValue { | ||
| arg: Arg<u32>, | ||
| } = 118, | ||
| Resume { | ||
| arg: Arg<u32>, | ||
| } = 149, | ||
| // ==================== RustPython-only instructions (119-135) ==================== | ||
| // ==================== RustPython-only instructions (119-133) ==================== | ||
| // Ideally, we want to be fully aligned with CPython opcodes, but we still have some leftovers. | ||
| // So we assign random IDs to these opcodes. | ||
| Break { | ||
| Expand Down Expand Up | @@ -277,10 +274,106 @@ pub enum Instruction { | |
| target: Arg<Label>, | ||
| } = 130, | ||
| JumpIfNotExcMatch(Arg<Label>) = 131, | ||
| SetExcInfo = 134, | ||
| Subscript = 135, | ||
| SetExcInfo = 132, | ||
| Subscript = 133, | ||
| // End of custom instructions | ||
| Resume { | ||
| arg: Arg<u32>, | ||
| } = 149, | ||
| BinaryOpAddFloat = 150, // Placeholder | ||
| BinaryOpAddInt = 151, // Placeholder | ||
| BinaryOpAddUnicode = 152, // Placeholder | ||
| BinaryOpMultiplyFloat = 153, // Placeholder | ||
| BinaryOpMultiplyInt = 154, // Placeholder | ||
| BinaryOpSubtractFloat = 155, // Placeholder | ||
| BinaryOpSubtractInt = 156, // Placeholder | ||
| BinarySubscrDict = 157, // Placeholder | ||
| BinarySubscrGetitem = 158, // Placeholder | ||
| BinarySubscrListInt = 159, // Placeholder | ||
| BinarySubscrStrInt = 160, // Placeholder | ||
| BinarySubscrTupleInt = 161, // Placeholder | ||
| CallAllocAndEnterInit = 162, // Placeholder | ||
| CallBoundMethodExactArgs = 163, // Placeholder | ||
| CallBoundMethodGeneral = 164, // Placeholder | ||
| CallBuiltinClass = 165, // Placeholder | ||
| CallBuiltinFast = 166, // Placeholder | ||
| CallBuiltinFastWithKeywords = 167, // Placeholder | ||
| CallBuiltinO = 168, // Placeholder | ||
| CallIsinstance = 169, // Placeholder | ||
| CallLen = 170, // Placeholder | ||
| CallListAppend = 171, // Placeholder | ||
| CallMethodDescriptorFast = 172, // Placeholder | ||
| CallMethodDescriptorFastWithKeywords = 173, // Placeholder | ||
| CallMethodDescriptorNoargs = 174, // Placeholder | ||
| CallMethodDescriptorO = 175, // Placeholder | ||
| CallNonPyGeneral = 176, // Placeholder | ||
| CallPyExactArgs = 177, // Placeholder | ||
| CallPyGeneral = 178, // Placeholder | ||
| CallStr1 = 179, // Placeholder | ||
| CallTuple1 = 180, // Placeholder | ||
| CallType1 = 181, // Placeholder | ||
| CompareOpFloat = 182, // Placeholder | ||
| CompareOpInt = 183, // Placeholder | ||
| CompareOpStr = 184, // Placeholder | ||
| ContainsOpDict = 185, // Placeholder | ||
| ContainsOpSet = 186, // Placeholder | ||
| ForIterGen = 187, // Placeholder | ||
| ForIterList = 188, // Placeholder | ||
| ForIterRange = 189, // Placeholder | ||
| ForIterTuple = 190, // Placeholder | ||
| LoadAttrClass = 191, // Placeholder | ||
| LoadAttrGetattributeOverridden = 192, // Placeholder | ||
| LoadAttrInstanceValue = 193, // Placeholder | ||
| LoadAttrMethodLazyDict = 194, // Placeholder | ||
| LoadAttrMethodNoDict = 195, // Placeholder | ||
| LoadAttrMethodWithValues = 196, // Placeholder | ||
| LoadAttrModule = 197, // Placeholder | ||
| LoadAttrNondescriptorNoDict = 198, // Placeholder | ||
| LoadAttrNondescriptorWithValues = 199, // Placeholder | ||
| LoadAttrProperty = 200, // Placeholder | ||
| LoadAttrSlot = 201, // Placeholder | ||
| LoadAttrWithHint = 202, // Placeholder | ||
| LoadGlobalBuiltin = 203, // Placeholder | ||
| LoadGlobalModule = 204, // Placeholder | ||
| LoadSuperAttrAttr = 205, // Placeholder | ||
| LoadSuperAttrMethod = 206, // Placeholder | ||
| ResumeCheck = 207, // Placeholder | ||
| SendGen = 208, // Placeholder | ||
| StoreAttrInstanceValue = 209, // Placeholder | ||
| StoreAttrSlot = 210, // Placeholder | ||
| StoreAttrWithHint = 211, // Placeholder | ||
| StoreSubscrDict = 212, // Placeholder | ||
| StoreSubscrListInt = 213, // Placeholder | ||
| ToBoolAlwaysTrue = 214, // Placeholder | ||
| ToBoolBool = 215, // Placeholder | ||
| ToBoolInt = 216, // Placeholder | ||
| ToBoolList = 217, // Placeholder | ||
| ToBoolNone = 218, // Placeholder | ||
| ToBoolStr = 219, // Placeholder | ||
| UnpackSequenceList = 220, // Placeholder | ||
| UnpackSequenceTuple = 221, // Placeholder | ||
| UnpackSequenceTwoTuple = 222, // Placeholder | ||
| InstrumentedResume = 236, // Placeholder | ||
| InstrumentedEndFor = 237, // Placeholder | ||
| InstrumentedEndSend = 238, // Placeholder | ||
| InstrumentedReturnValue = 239, // Placeholder | ||
| InstrumentedReturnConst = 240, // Placeholder | ||
| InstrumentedYieldValue = 241, // Placeholder | ||
| InstrumentedLoadSuperAttr = 242, // Placeholder | ||
| InstrumentedForIter = 243, // Placeholder | ||
| InstrumentedCall = 244, // Placeholder | ||
| InstrumentedCallKw = 245, // Placeholder | ||
| InstrumentedCallFunctionEx = 246, // Placeholder | ||
| InstrumentedInstruction = 247, // Placeholder | ||
| InstrumentedJumpForward = 248, // Placeholder | ||
| InstrumentedJumpBackward = 249, // Placeholder | ||
| InstrumentedPopJumpIfTrue = 250, // Placeholder | ||
| InstrumentedPopJumpIfFalse = 251, // Placeholder | ||
| InstrumentedPopJumpIfNone = 252, // Placeholder | ||
| InstrumentedPopJumpIfNotNone = 253, // Placeholder | ||
| InstrumentedLine = 254, // Placeholder | ||
|
Comment thread
Comment on lines
+280
to
+374
Copy link
Copy Markdown
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality⚠️ Potential issue | 🟠 Major 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# First, let's check the file size and structure
wc -l crates/compiler-core/src/bytecode/instruction.rsRepository: RustPython/RustPython Length of output: 118 🏁 Script executed: #!/bin/bash
# Read the specific lines mentioned in the review (280-374)
sed -n '280,374p' crates/compiler-core/src/bytecode/instruction.rs | cat -nRepository: RustPython/RustPython Length of output: 6574 🏁 Script executed: #!/bin/bash
# Search for how label_arg and unconditional_branch are implemented
rg -A 5 "fn label_arg|fn unconditional_branch" crates/compiler-core/src/bytecode/instruction.rsRepository: RustPython/RustPython Length of output: 1402 🏁 Script executed: #!/bin/bash
# Search for Arg<Label> pattern to understand expected structure
rg "Arg<Label>" crates/compiler-core/src/bytecode/instruction.rs | head -20Repository: RustPython/RustPython Length of output: 729 🏁 Script executed: #!/bin/bash
# Check if InstrumentedJumpForward, InstrumentedJumpBackward are defined with or without Arg
rg "InstrumentedJump" crates/compiler-core/src/bytecode/instruction.rsRepository: RustPython/RustPython Length of output: 287 🏁 Script executed: #!/bin/bash
# Search for where these instrumented opcodes are actually used or created
rg "InstrumentedJumpForward|InstrumentedJumpBackward|InstrumentedPopJump" --type rs -B 3 -A 3Repository: RustPython/RustPython Length of output: 91 🏁 Script executed: #!/bin/bash
# Check the context around these definitions and look for any match statements
sed -n '200,280p' crates/compiler-core/src/bytecode/instruction.rs | cat -nRepository: RustPython/RustPython Length of output: 2534 🏁 Script executed: #!/bin/bash
# Look for the enum definition structure to see if there's a pattern
sed -n '1,200p' crates/compiler-core/src/bytecode/instruction.rs | grep -A 2 "JumpForward\|JumpBackward" | head -30Repository: RustPython/RustPython Length of output: 296 🏁 Script executed: #!/bin/bash
# Search for any documentation or comments about instrumented instructions
rg -B 5 "InstrumentedJump|InstrumentedPopJump" crates/compiler-core/src/bytecode/instruction.rsRepository: RustPython/RustPython Length of output: 1288 🏁 Script executed: #!/bin/bash
# Check if there's a match arm or implementation that handles these instrumented opcodes specially
rg "match.*self" crates/compiler-core/src/bytecode/instruction.rs -A 100 | grep -A 5 "InstrumentedJump"Repository: RustPython/RustPython Length of output: 47 🏁 Script executed: #!/bin/bash
# Look for any code that might convert between regular and instrumented versions
rg "Instrumented.*Jump|Jump.*Instrumented" crates/compiler-core/src/bytecode/Repository: RustPython/RustPython Length of output: 1331 🏁 Script executed: #!/bin/bash
# Search for documentation or comments about these Instrumented instructions
rg -B 10 "Instrumented" crates/compiler-core/src/bytecode/instruction.rs | head -50Repository: RustPython/RustPython Length of output: 2724 🏁 Script executed: #!/antml/bash
# Look at the entire file context around Instrumented opcodes to understand their purpose
sed -n '1,50p' crates/compiler-core/src/bytecode/instruction.rsRepository: RustPython/RustPython Length of output: 1631 🏁 Script executed: #!/bin/bash
# Check if these are part of a newer enum or alternative implementation
git log --oneline --all -S "InstrumentedJumpForward" | head -5Repository: RustPython/RustPython Length of output: 151 🏁 Script executed: #!/bin/bash
# Look for any comments or documentation about what "Placeholder" means
rg "Placeholder" crates/compiler-core/src/bytecode/instruction.rs | head -20Repository: RustPython/RustPython Length of output: 788 🏁 Script executed: #!/bin/bash
# Check the CPython 3.13 instrumentation opcodes to understand the intent
# These might be based on CPython's instrumented opcodes
rg -B 5 -A 5 "instrumented_start|instrumented_end" crates/compiler-core/src/bytecode/instruction.rsRepository: RustPython/RustPython Length of output: 1085 🏁 Script executed: #!/antml/bash
# Look for any usage of these Instrumented opcodes in the codebase
find crates -name "*.rs" -type f -exec grep -l "InstrumentedJump" {} \;Repository: RustPython/RustPython Length of output: 113 🏁 Script executed: #!/bin/bash
# Check if there's any code generation or transformation that would convert Jump to InstrumentedJump
rg "to_instrumented|from_instrumented|instrument_" crates/compiler-core/Repository: RustPython/RustPython Length of output: 47 Instrumented jump opcodes lack label targets and control-flow integration. InstrumentedJumpForward/Backward and InstrumentedPopJump* (248–253) are defined as no-argument variants and excluded from both label_arg() and unconditional_branch(). If these are meant as instrumented versions of control-flow operations (similar to CPython 3.13's instrumented opcodes), they should carry Arg<Label> and be reflected in control-flow analysis. Currently, all are marked // Placeholder with stack_effect of 0 and no usage in the codebase. Clarify whether these should be functional control-flow opcodes or pure instrumentation markers only, and complete the implementation accordingly. 🤖 Prompt for AI AgentsIn `@crates/compiler-core/src/bytecode/instruction.rs` around lines 280 - 374, Instrumented jump opcodes (InstrumentedJumpForward, InstrumentedJumpBackward, InstrumentedPopJumpIfTrue, InstrumentedPopJumpIfFalse, InstrumentedPopJumpIfNone, InstrumentedPopJumpIfNotNone) are currently no-arg placeholders but should either be real control-flow ops or explicit no-op instrumentation; update them accordingly: if they represent instrumented control-flow, change their variants to carry Arg<Label>, update their stack_effects, and add them to label_arg() and unconditional_branch()/conditional-branch logic so control-flow analysis recognizes their targets (refer to these enum variants and the functions label_arg() and unconditional_branch()); if they are only markers, leave them no-arg but document that intent and add explicit comments/tests and ensure they are never used in control-flow analysis (e.g., assert they are ignored) so behavior is clear and consistent across the codebase.
Sorry, something went wrong.
All reactions
|
||
| // Pseudos (needs to be moved to `PseudoInstruction` enum. | ||
| LoadClosure(Arg<NameIdx>) = 253, // TODO: Move to pseudos | ||
| LoadClosure(Arg<NameIdx>) = 255, // TODO: Move to pseudos | ||
| } | ||
|
|
||
| const _: () = assert!(mem::size_of::<Instruction>() == 1); | ||
| Expand All | @@ -305,6 +398,12 @@ impl TryFrom<u8> for Instruction { | |
| // Resume has a non-contiguous opcode (149) | ||
| let resume_id = u8::from(Self::Resume { arg: Arg::marker() }); | ||
|
|
||
| let specialized_start = u8::from(Self::BinaryOpAddFloat); | ||
| let specialized_end = u8::from(Self::UnpackSequenceTwoTuple); | ||
|
|
||
| let instrumented_start = u8::from(Self::InstrumentedResume); | ||
| let instrumented_end = u8::from(Self::InstrumentedLine); | ||
|
|
||
| // TODO: Remove this; This instruction needs to be pseudo | ||
| let load_closure = u8::from(Self::LoadClosure(Arg::marker())); | ||
|
|
||
| Expand Down Expand Up | @@ -345,6 +444,8 @@ impl TryFrom<u8> for Instruction { | |
| || value == resume_id | ||
| || value == load_closure | ||
| || custom_ops.contains(&value) | ||
| || (specialized_start..=specialized_end).contains(&value) | ||
| || (instrumented_start..=instrumented_end).contains(&value) | ||
| { | ||
| Ok(unsafe { mem::transmute::<u8, Self>(value) }) | ||
| } else { | ||
| Expand Down Expand Up | @@ -589,6 +690,98 @@ impl InstructionMetadata for Instruction { | |
| Self::PopJumpIfNone { .. } => 0, | ||
| Self::PopJumpIfNotNone { .. } => 0, | ||
| Self::LoadClosure(_) => 1, | ||
| Self::BinaryOpAddFloat => 0, | ||
| Self::BinaryOpAddInt => 0, | ||
| Self::BinaryOpAddUnicode => 0, | ||
| Self::BinaryOpMultiplyFloat => 0, | ||
| Self::BinaryOpMultiplyInt => 0, | ||
| Self::BinaryOpSubtractFloat => 0, | ||
| Self::BinaryOpSubtractInt => 0, | ||
| Self::BinarySubscrDict => 0, | ||
| Self::BinarySubscrGetitem => 0, | ||
| Self::BinarySubscrListInt => 0, | ||
| Self::BinarySubscrStrInt => 0, | ||
| Self::BinarySubscrTupleInt => 0, | ||
| Self::CallAllocAndEnterInit => 0, | ||
| Self::CallBoundMethodExactArgs => 0, | ||
| Self::CallBoundMethodGeneral => 0, | ||
| Self::CallBuiltinClass => 0, | ||
| Self::CallBuiltinFast => 0, | ||
| Self::CallBuiltinFastWithKeywords => 0, | ||
| Self::CallBuiltinO => 0, | ||
| Self::CallIsinstance => 0, | ||
| Self::CallLen => 0, | ||
| Self::CallListAppend => 0, | ||
| Self::CallMethodDescriptorFast => 0, | ||
| Self::CallMethodDescriptorFastWithKeywords => 0, | ||
| Self::CallMethodDescriptorNoargs => 0, | ||
| Self::CallMethodDescriptorO => 0, | ||
| Self::CallNonPyGeneral => 0, | ||
| Self::CallPyExactArgs => 0, | ||
| Self::CallPyGeneral => 0, | ||
| Self::CallStr1 => 0, | ||
| Self::CallTuple1 => 0, | ||
| Self::CallType1 => 0, | ||
| Self::CompareOpFloat => 0, | ||
| Self::CompareOpInt => 0, | ||
| Self::CompareOpStr => 0, | ||
| Self::ContainsOpDict => 0, | ||
| Self::ContainsOpSet => 0, | ||
| Self::ForIterGen => 0, | ||
| Self::ForIterList => 0, | ||
| Self::ForIterRange => 0, | ||
| Self::ForIterTuple => 0, | ||
| Self::LoadAttrClass => 0, | ||
| Self::LoadAttrGetattributeOverridden => 0, | ||
| Self::LoadAttrInstanceValue => 0, | ||
| Self::LoadAttrMethodLazyDict => 0, | ||
| Self::LoadAttrMethodNoDict => 0, | ||
| Self::LoadAttrMethodWithValues => 0, | ||
| Self::LoadAttrModule => 0, | ||
| Self::LoadAttrNondescriptorNoDict => 0, | ||
| Self::LoadAttrNondescriptorWithValues => 0, | ||
| Self::LoadAttrProperty => 0, | ||
| Self::LoadAttrSlot => 0, | ||
| Self::LoadAttrWithHint => 0, | ||
| Self::LoadGlobalBuiltin => 0, | ||
| Self::LoadGlobalModule => 0, | ||
| Self::LoadSuperAttrAttr => 0, | ||
| Self::LoadSuperAttrMethod => 0, | ||
| Self::ResumeCheck => 0, | ||
| Self::SendGen => 0, | ||
| Self::StoreAttrInstanceValue => 0, | ||
| Self::StoreAttrSlot => 0, | ||
| Self::StoreAttrWithHint => 0, | ||
| Self::StoreSubscrDict => 0, | ||
| Self::StoreSubscrListInt => 0, | ||
| Self::ToBoolAlwaysTrue => 0, | ||
| Self::ToBoolBool => 0, | ||
| Self::ToBoolInt => 0, | ||
| Self::ToBoolList => 0, | ||
| Self::ToBoolNone => 0, | ||
| Self::ToBoolStr => 0, | ||
| Self::UnpackSequenceList => 0, | ||
| Self::UnpackSequenceTuple => 0, | ||
| Self::UnpackSequenceTwoTuple => 0, | ||
| Self::InstrumentedResume => 0, | ||
| Self::InstrumentedEndFor => 0, | ||
| Self::InstrumentedEndSend => 0, | ||
| Self::InstrumentedReturnValue => 0, | ||
| Self::InstrumentedReturnConst => 0, | ||
| Self::InstrumentedYieldValue => 0, | ||
| Self::InstrumentedLoadSuperAttr => 0, | ||
| Self::InstrumentedForIter => 0, | ||
| Self::InstrumentedCall => 0, | ||
| Self::InstrumentedCallKw => 0, | ||
| Self::InstrumentedCallFunctionEx => 0, | ||
| Self::InstrumentedInstruction => 0, | ||
| Self::InstrumentedJumpForward => 0, | ||
| Self::InstrumentedJumpBackward => 0, | ||
| Self::InstrumentedPopJumpIfTrue => 0, | ||
| Self::InstrumentedPopJumpIfFalse => 0, | ||
| Self::InstrumentedPopJumpIfNone => 0, | ||
| Self::InstrumentedPopJumpIfNotNone => 0, | ||
| Self::InstrumentedLine => 0, | ||
| } | ||
| } | ||
|
|
||
| Expand Down | ||
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThe bytecode already has to be purged because LoadClosure is no longer 253. So I've moved these as well
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualitysure, this will also totally reordered on 3.14
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.