X Tutup
Skip to content

Make inner oparg values private #7050

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.

Sign up for GitHub

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

Merged
youknowone merged 1 commit into from
Feb 8, 2026
Merged

Make inner oparg values private #7050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ impl Compiler {
arg: OpArgMarker::marker(),
}
.into(),
arg: OpArg(u32::from(bytecode::ResumeType::AtFuncStart)),
arg: OpArg::new(u32::from(bytecode::ResumeType::AtFuncStart)),
target: BlockIdx::NULL,
location,
end_location,
Expand Down Expand Up @@ -7397,7 +7397,7 @@ impl Compiler {
let return_none = init_collection.is_none();
// Create empty object of proper type:
if let Some(init_collection) = init_collection {
self._emit(init_collection, OpArg(0), BlockIdx::NULL)
self._emit(init_collection, OpArg::new(0), BlockIdx::NULL)
}

let mut loop_labels = vec![];
Expand Down Expand Up @@ -7593,7 +7593,7 @@ impl Compiler {
// Step 4: Create the collection (list/set/dict)
// For generator expressions, init_collection is None
if let Some(init_collection) = init_collection {
self._emit(init_collection, OpArg(0), BlockIdx::NULL);
self._emit(init_collection, OpArg::new(0), BlockIdx::NULL);
// SWAP to get iterator on top
emit!(self, Instruction::Swap { index: 2 });
}
Expand Down Expand Up @@ -7767,7 +7767,7 @@ impl Compiler {
}

fn emit_no_arg<I: Into<AnyInstruction>>(&mut self, ins: I) {
self._emit(ins, OpArg::null(), BlockIdx::NULL)
self._emit(ins, OpArg::NULL, BlockIdx::NULL)
}

fn emit_arg<A: OpArgType, T: EmitArg<A>, I: Into<AnyInstruction>>(
Expand Down Expand Up @@ -8455,7 +8455,7 @@ impl EmitArg<bytecode::Label> for BlockIdx {
self,
f: impl FnOnce(OpArgMarker<bytecode::Label>) -> I,
) -> (AnyInstruction, OpArg, BlockIdx) {
(f(OpArgMarker::marker()).into(), OpArg::null(), self)
(f(OpArgMarker::marker()).into(), OpArg::NULL, self)
}
}

Expand Down
40 changes: 20 additions & 20 deletions crates/codegen/src/ir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ impl CodeInfo {
for info in &mut block.instructions {
let target = info.target;
if target != BlockIdx::NULL {
let new_arg = OpArg(block_to_offset[target.idx()].0);
let new_arg = OpArg::new(block_to_offset[target.idx()].0);
recompile_extended_arg |= new_arg.instr_size() != info.arg.instr_size();
info.arg = new_arg;
}
Expand Down Expand Up @@ -443,7 +443,7 @@ impl CodeInfo {
continue;
};

let tuple_size = instr.arg.0 as usize;
let tuple_size = u32::from(instr.arg) as usize;
if tuple_size == 0 || i < tuple_size {
i += 1;
continue;
Expand All @@ -458,7 +458,7 @@ impl CodeInfo {
let load_instr = &block.instructions[j];
match load_instr.instr.real() {
Some(Instruction::LoadConst { .. }) => {
let const_idx = load_instr.arg.0 as usize;
let const_idx = u32::from(load_instr.arg) as usize;
if let Some(constant) =
self.metadata.consts.get_index(const_idx).cloned()
{
Expand All @@ -470,7 +470,7 @@ impl CodeInfo {
}
Some(Instruction::LoadSmallInt { .. }) => {
// arg is the i32 value stored as u32 (two's complement)
let value = load_instr.arg.0 as i32;
let value = u32::from(load_instr.arg) as i32;
elements.push(ConstantData::Integer {
value: BigInt::from(value),
});
Expand Down Expand Up @@ -502,7 +502,7 @@ impl CodeInfo {

// Replace BUILD_TUPLE with LOAD_CONST
block.instructions[i].instr = Instruction::LoadConst { idx: Arg::marker() }.into();
block.instructions[i].arg = OpArg(const_idx as u32);
block.instructions[i].arg = OpArg::new(const_idx as u32);

i += 1;
}
Expand All @@ -529,27 +529,27 @@ impl CodeInfo {
match (curr_instr, next_instr) {
// LoadFast + LoadFast -> LoadFastLoadFast (if both indices < 16)
(Instruction::LoadFast(_), Instruction::LoadFast(_)) => {
let idx1 = curr.arg.0;
let idx2 = next.arg.0;
let idx1 = u32::from(curr.arg);
let idx2 = u32::from(next.arg);
if idx1 < 16 && idx2 < 16 {
let packed = (idx1 << 4) | idx2;
Some((
Instruction::LoadFastLoadFast { arg: Arg::marker() },
OpArg(packed),
OpArg::new(packed),
))
} else {
None
}
}
// StoreFast + StoreFast -> StoreFastStoreFast (if both indices < 16)
(Instruction::StoreFast(_), Instruction::StoreFast(_)) => {
let idx1 = curr.arg.0;
let idx2 = next.arg.0;
let idx1 = u32::from(curr.arg);
let idx2 = u32::from(next.arg);
if idx1 < 16 && idx2 < 16 {
let packed = (idx1 << 4) | idx2;
Some((
Instruction::StoreFastStoreFast { arg: Arg::marker() },
OpArg(packed),
OpArg::new(packed),
))
} else {
None
Expand Down Expand Up @@ -584,7 +584,7 @@ impl CodeInfo {
};

// Get the constant value
let const_idx = instr.arg.0 as usize;
let const_idx = u32::from(instr.arg) as usize;
let Some(constant) = self.metadata.consts.get_index(const_idx) else {
continue;
};
Expand All @@ -599,7 +599,7 @@ impl CodeInfo {
// Convert LOAD_CONST to LOAD_SMALL_INT
instr.instr = Instruction::LoadSmallInt { idx: Arg::marker() }.into();
// The arg is the i32 value stored as u32 (two's complement)
instr.arg = OpArg(small as u32);
instr.arg = OpArg::new(small as u32);
}
}
}
Expand All @@ -621,7 +621,7 @@ impl CodeInfo {
for block in &self.blocks {
for instr in &block.instructions {
if let Some(Instruction::LoadConst { .. }) = instr.instr.real() {
let idx = instr.arg.0 as usize;
let idx = u32::from(instr.arg) as usize;
if idx < nconsts {
used[idx] = true;
}
Expand Down Expand Up @@ -658,9 +658,9 @@ impl CodeInfo {
for block in &mut self.blocks {
for instr in &mut block.instructions {
if let Some(Instruction::LoadConst { .. }) = instr.instr.real() {
let old_idx = instr.arg.0 as usize;
let old_idx = u32::from(instr.arg) as usize;
if old_idx < nconsts {
instr.arg = OpArg(old_to_new[old_idx] as u32);
instr.arg = OpArg::new(old_to_new[old_idx] as u32);
}
}
}
Expand Down Expand Up @@ -801,7 +801,7 @@ impl CodeInfo {
let display_arg = if ins.target == BlockIdx::NULL {
ins.arg
} else {
OpArg(ins.target.0)
OpArg::new(ins.target.0)
};
let instr_display = instr.display(display_arg, self);
eprint!("{instr_display}: {depth} {effect:+} => ");
Expand Down Expand Up @@ -1253,10 +1253,10 @@ pub(crate) fn label_exception_targets(blocks: &mut [Block]) {
const RESUME_OPARG_LOCATION_MASK: u32 = 0x3;
const RESUME_OPARG_DEPTH1_MASK: u32 = 0x4;

if (arg.0 & RESUME_OPARG_LOCATION_MASK) != RESUME_AT_FUNC_START {
if (u32::from(arg) & RESUME_OPARG_LOCATION_MASK) != RESUME_AT_FUNC_START {
if last_yield_except_depth == 1 {
blocks[bi].instructions[i].arg =
OpArg(arg.0 | RESUME_OPARG_DEPTH1_MASK);
OpArg::new(u32::from(arg) | RESUME_OPARG_DEPTH1_MASK);
}
last_yield_except_depth = -1;
}
Expand Down Expand Up @@ -1311,7 +1311,7 @@ pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], varnames_len: u32) {
// LOAD_CLOSURE → LOAD_FAST (with varnames offset)
PseudoInstruction::LoadClosure(idx) => {
let new_idx = varnames_len + idx.get(info.arg);
info.arg = OpArg(new_idx);
info.arg = OpArg::new(new_idx);
info.instr = Instruction::LoadFast(Arg::marker()).into();
}
// Jump pseudo ops are resolved during block linearization
Expand Down
8 changes: 4 additions & 4 deletions crates/compiler-core/src/bytecode/instruction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1249,15 +1249,15 @@ impl<T: OpArgType> Arg<T> {

#[inline]
pub fn new(arg: T) -> (Self, OpArg) {
(Self(PhantomData), OpArg(arg.into()))
(Self(PhantomData), OpArg::new(arg.into()))
}

#[inline]
pub fn new_single(arg: T) -> (Self, OpArgByte)
where
T: Into<u8>,
{
(Self(PhantomData), OpArgByte(arg.into()))
(Self(PhantomData), OpArgByte::new(arg.into()))
}

#[inline(always)]
Expand All @@ -1267,15 +1267,15 @@ impl<T: OpArgType> Arg<T> {

#[inline(always)]
pub fn try_get(self, arg: OpArg) -> Result<T, MarshalError> {
T::try_from(arg.0).map_err(|_| MarshalError::InvalidBytecode)
T::try_from(u32::from(arg)).map_err(|_| MarshalError::InvalidBytecode)
}

/// # Safety
/// T::from_op_arg(self) must succeed
#[inline(always)]
pub unsafe fn get_unchecked(self, arg: OpArg) -> T {
// SAFETY: requirements forwarded from caller
unsafe { T::try_from(arg.0).unwrap_unchecked() }
unsafe { T::try_from(u32::from(arg)).unwrap_unchecked() }
}
}

Expand Down
30 changes: 21 additions & 9 deletions crates/compiler-core/src/bytecode/oparg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,26 @@ pub trait OpArgType: Copy + Into<u32> + TryFrom<u32> {}
/// Opcode argument that may be extended by a prior ExtendedArg.
#[derive(Copy, Clone, PartialEq, Eq)]
#[repr(transparent)]
pub struct OpArgByte(pub u8);
pub struct OpArgByte(u8);

impl OpArgByte {
pub const fn null() -> Self {
Self(0)
pub const NULL: Self = Self::new(0);

#[must_use]
pub const fn new(value: u8) -> Self {
Self(value)
}
}

impl From<u8> for OpArgByte {
fn from(raw: u8) -> Self {
Self(raw)
Self::new(raw)
}
}

impl From<OpArgByte> for u8 {
fn from(value: OpArgByte) -> Self {
value.0
}
}

Expand All @@ -35,11 +44,14 @@ impl fmt::Debug for OpArgByte {
/// Full 32-bit op_arg, including any possible ExtendedArg extension.
#[derive(Copy, Clone, Debug)]
#[repr(transparent)]
pub struct OpArg(pub u32);
pub struct OpArg(u32);

impl OpArg {
pub const fn null() -> Self {
Self(0)
pub const NULL: Self = Self::new(0);

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

/// Returns how many CodeUnits a instruction with this op_arg will be encoded as
Expand All @@ -65,7 +77,7 @@ impl OpArg {

impl From<u32> for OpArg {
fn from(raw: u32) -> Self {
Self(raw)
Self::new(raw)
}
}

Expand Down Expand Up @@ -94,7 +106,7 @@ impl OpArgState {
#[inline(always)]
pub fn extend(&mut self, arg: OpArgByte) -> OpArg {
self.state = (self.state << 8) | u32::from(arg.0);
OpArg(self.state)
self.state.into()
}

#[inline(always)]
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ impl ExecutingFrame<'_> {
{
// YIELD_VALUE arg: 0 = direct yield, >= 1 = yield-from/await
// OpArgByte.0 is the raw byte value
if prev_unit.arg.0 >= 1 {
if u8::from(prev_unit.arg) >= 1 {
// In yield-from/await context, delegate is on top of stack
return Some(self.top_value());
}
Expand Down
X Tutup