GeezerVison

where the magic numbers came from? will this be changed if we change instruction enum?

Oh, I just ran:

import opcode

for name in (
    "hasarg",
    "hasconst",
    "hasname",
    "hasjump",
    "hasjrel",
    "hasjabs",
    "hasfree",
    "haslocal",
    "hasexc",
):
    lst = getattr(opcode, name)
    print(f"{name}\n{lst}")

And copied the results xD

But it's calculated based on https://github.com/python/cpython/blob/bcee1c322115c581da27600f2ae55e5439c027eb/Include/internal/pycore_opcode_metadata.h#L966-L1190

I've put a comment at https://github.com/ShaharNaveh/RustPython/blob/a7b9356da2a85cbc02df1f0fac9eda6a26869b6d/stdlib/src/opcode.rs#L46-L47 stating this:)

We should have our own metadata table, but I think that's a bit out of scope for this PR

At

I tried doing something like:

 #[repr(u8)] 
 pub enum Instruction { 
     Nop = 30, 
     ImportName { 
         idx: Arg<NameIdx>, 
     } = 75, 
     ...
}

Based on:

• Nop: https://github.com/python/cpython/blob/bcee1c322115c581da27600f2ae55e5439c027eb/Include/opcode_ids.h#L43
• ImportName: https://github.com/python/cpython/blob/bcee1c322115c581da27600f2ae55e5439c027eb/Include/opcode_ids.h#L88

But this change alone was enough to break RustPython entirely.

@youknowone can you please point me to where any of:

are being called? I've tried looking into it and got a bit lost

Thank you for explanation. Since we occasionally break bytecode compatibility, the numbers need to be regenerated.
Could you also attach the updating code around enum Instruction not to let it be desynced?

The From<Instruction> part is used form marshal. cc @coolreader18

Since we occasionally break bytecode compatibility

Is there a reason for that? or it's something that's desired but not implemented yet?

the numbers need to be regenerated. Could you also attach the updating code around enum Instruction not to let it be desynced?

I haven't written any code to do that (yet, but was planning to). As I just changed those two enum members and I couldn't even do cargo run :/

The From<Instruction> part is used form marshal. cc @coolreader18

ty:) will look into it.

@youknowone I have opened #6174 so we can discuss the next steps there and to not block this PR

Is there a reason for that? or it's something that's desired but not implemented yet?

No specific reason. We just add or remove instructions by following CPython update. The order changing is unintended side effect. Python doesn't guarantee bytecode compatibiltity anyway.

Does giving specific numbers to each instruction make it easier to be maintained? I guess so.

After #6174, then can we rewrite 63 | 66 to Instruction::Abc as u8 | Instruction::Def as u8 and so on?

I didn't know it can be over 256, huh

Those are pseudo opcodes:)
Real opcodes can't be more than 256

Is there a reason for that? or it's something that's desired but not implemented yet?

No specific reason. We just add or remove instructions by following CPython update. The order changing is unintended side effect. Python doesn't guarantee bytecode compatibiltity anyway.

Does giving specific numbers to each instruction make it easier to be maintained? I guess so.

After #6174, then can we rewrite 63 | 66 to Instruction::Abc as u8 | Instruction::Def as u8 and so on?