X Tutup
Skip to content

Commit ea352cc

Browse files
ShaharNavehShaharNaveh
authored
Make inner oparg values private (#7050)
1 parent f777416 commit ea352cc

File tree

5 files changed

+51
-39
lines changed

5 files changed

+51
-39
lines changed

‎crates/codegen/src/compile.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,7 @@ impl Compiler {
11651165
arg: OpArgMarker::marker(),
11661166
}
11671167
.into(),
1168-
arg: OpArg(u32::from(bytecode::ResumeType::AtFuncStart)),
1168+
arg: OpArg::new(u32::from(bytecode::ResumeType::AtFuncStart)),
11691169
target: BlockIdx::NULL,
11701170
location,
11711171
end_location,
@@ -7397,7 +7397,7 @@ impl Compiler {
73977397
let return_none = init_collection.is_none();
73987398
// Create empty object of proper type:
73997399
if let Some(init_collection) = init_collection {
7400-
self._emit(init_collection, OpArg(0), BlockIdx::NULL)
7400+
self._emit(init_collection, OpArg::new(0), BlockIdx::NULL)
74017401
}
74027402

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

77697769
fn emit_no_arg<I: Into<AnyInstruction>>(&mut self, ins: I) {
7770-
self._emit(ins, OpArg::null(), BlockIdx::NULL)
7770+
self._emit(ins, OpArg::NULL, BlockIdx::NULL)
77717771
}
77727772

77737773
fn emit_arg<A: OpArgType, T: EmitArg<A>, I: Into<AnyInstruction>>(
@@ -8455,7 +8455,7 @@ impl EmitArg<bytecode::Label> for BlockIdx {
84558455
self,
84568456
f: impl FnOnce(OpArgMarker<bytecode::Label>) -> I,
84578457
) -> (AnyInstruction, OpArg, BlockIdx) {
8458-
(f(OpArgMarker::marker()).into(), OpArg::null(), self)
8458+
(f(OpArgMarker::marker()).into(), OpArg::NULL, self)
84598459
}
84608460
}
84618461

‎crates/codegen/src/ir.rs‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ impl CodeInfo {
284284
for info in &mut block.instructions {
285285
let target = info.target;
286286
if target != BlockIdx::NULL {
287-
let new_arg = OpArg(block_to_offset[target.idx()].0);
287+
let new_arg = OpArg::new(block_to_offset[target.idx()].0);
288288
recompile_extended_arg |= new_arg.instr_size() != info.arg.instr_size();
289289
info.arg = new_arg;
290290
}
@@ -443,7 +443,7 @@ impl CodeInfo {
443443
continue;
444444
};
445445

446-
let tuple_size = instr.arg.0 as usize;
446+
let tuple_size = u32::from(instr.arg) as usize;
447447
if tuple_size == 0 || i < tuple_size {
448448
i += 1;
449449
continue;
@@ -458,7 +458,7 @@ impl CodeInfo {
458458
let load_instr = &block.instructions[j];
459459
match load_instr.instr.real() {
460460
Some(Instruction::LoadConst { .. }) => {
461-
let const_idx = load_instr.arg.0 as usize;
461+
let const_idx = u32::from(load_instr.arg) as usize;
462462
if let Some(constant) =
463463
self.metadata.consts.get_index(const_idx).cloned()
464464
{
@@ -470,7 +470,7 @@ impl CodeInfo {
470470
}
471471
Some(Instruction::LoadSmallInt { .. }) => {
472472
// arg is the i32 value stored as u32 (two's complement)
473-
let value = load_instr.arg.0 as i32;
473+
let value = u32::from(load_instr.arg) as i32;
474474
elements.push(ConstantData::Integer {
475475
value: BigInt::from(value),
476476
});
@@ -502,7 +502,7 @@ impl CodeInfo {
502502

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

507507
i += 1;
508508
}
@@ -529,27 +529,27 @@ impl CodeInfo {
529529
match (curr_instr, next_instr) {
530530
// LoadFast + LoadFast -> LoadFastLoadFast (if both indices < 16)
531531
(Instruction::LoadFast(_), Instruction::LoadFast(_)) => {
532-
let idx1 = curr.arg.0;
533-
let idx2 = next.arg.0;
532+
let idx1 = u32::from(curr.arg);
533+
let idx2 = u32::from(next.arg);
534534
if idx1 < 16 && idx2 < 16 {
535535
let packed = (idx1 << 4) | idx2;
536536
Some((
537537
Instruction::LoadFastLoadFast { arg: Arg::marker() },
538-
OpArg(packed),
538+
OpArg::new(packed),
539539
))
540540
} else {
541541
None
542542
}
543543
}
544544
// StoreFast + StoreFast -> StoreFastStoreFast (if both indices < 16)
545545
(Instruction::StoreFast(_), Instruction::StoreFast(_)) => {
546-
let idx1 = curr.arg.0;
547-
let idx2 = next.arg.0;
546+
let idx1 = u32::from(curr.arg);
547+
let idx2 = u32::from(next.arg);
548548
if idx1 < 16 && idx2 < 16 {
549549
let packed = (idx1 << 4) | idx2;
550550
Some((
551551
Instruction::StoreFastStoreFast { arg: Arg::marker() },
552-
OpArg(packed),
552+
OpArg::new(packed),
553553
))
554554
} else {
555555
None
@@ -584,7 +584,7 @@ impl CodeInfo {
584584
};
585585

586586
// Get the constant value
587-
let const_idx = instr.arg.0 as usize;
587+
let const_idx = u32::from(instr.arg) as usize;
588588
let Some(constant) = self.metadata.consts.get_index(const_idx) else {
589589
continue;
590590
};
@@ -599,7 +599,7 @@ impl CodeInfo {
599599
// Convert LOAD_CONST to LOAD_SMALL_INT
600600
instr.instr = Instruction::LoadSmallInt { idx: Arg::marker() }.into();
601601
// The arg is the i32 value stored as u32 (two's complement)
602-
instr.arg = OpArg(small as u32);
602+
instr.arg = OpArg::new(small as u32);
603603
}
604604
}
605605
}
@@ -621,7 +621,7 @@ impl CodeInfo {
621621
for block in &self.blocks {
622622
for instr in &block.instructions {
623623
if let Some(Instruction::LoadConst { .. }) = instr.instr.real() {
624-
let idx = instr.arg.0 as usize;
624+
let idx = u32::from(instr.arg) as usize;
625625
if idx < nconsts {
626626
used[idx] = true;
627627
}
@@ -658,9 +658,9 @@ impl CodeInfo {
658658
for block in &mut self.blocks {
659659
for instr in &mut block.instructions {
660660
if let Some(Instruction::LoadConst { .. }) = instr.instr.real() {
661-
let old_idx = instr.arg.0 as usize;
661+
let old_idx = u32::from(instr.arg) as usize;
662662
if old_idx < nconsts {
663-
instr.arg = OpArg(old_to_new[old_idx] as u32);
663+
instr.arg = OpArg::new(old_to_new[old_idx] as u32);
664664
}
665665
}
666666
}
@@ -801,7 +801,7 @@ impl CodeInfo {
801801
let display_arg = if ins.target == BlockIdx::NULL {
802802
ins.arg
803803
} else {
804-
OpArg(ins.target.0)
804+
OpArg::new(ins.target.0)
805805
};
806806
let instr_display = instr.display(display_arg, self);
807807
eprint!("{instr_display}: {depth} {effect:+} => ");
@@ -1253,10 +1253,10 @@ pub(crate) fn label_exception_targets(blocks: &mut [Block]) {
12531253
const RESUME_OPARG_LOCATION_MASK: u32 = 0x3;
12541254
const RESUME_OPARG_DEPTH1_MASK: u32 = 0x4;
12551255

1256-
if (arg.0 & RESUME_OPARG_LOCATION_MASK) != RESUME_AT_FUNC_START {
1256+
if (u32::from(arg) & RESUME_OPARG_LOCATION_MASK) != RESUME_AT_FUNC_START {
12571257
if last_yield_except_depth == 1 {
12581258
blocks[bi].instructions[i].arg =
1259-
OpArg(arg.0 | RESUME_OPARG_DEPTH1_MASK);
1259+
OpArg::new(u32::from(arg) | RESUME_OPARG_DEPTH1_MASK);
12601260
}
12611261
last_yield_except_depth = -1;
12621262
}
@@ -1311,7 +1311,7 @@ pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], varnames_len: u32) {
13111311
// LOAD_CLOSURE → LOAD_FAST (with varnames offset)
13121312
PseudoInstruction::LoadClosure(idx) => {
13131313
let new_idx = varnames_len + idx.get(info.arg);
1314-
info.arg = OpArg(new_idx);
1314+
info.arg = OpArg::new(new_idx);
13151315
info.instr = Instruction::LoadFast(Arg::marker()).into();
13161316
}
13171317
// Jump pseudo ops are resolved during block linearization

‎crates/compiler-core/src/bytecode/instruction.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,15 +1249,15 @@ impl<T: OpArgType> Arg<T> {
12491249

12501250
#[inline]
12511251
pub fn new(arg: T) -> (Self, OpArg) {
1252-
(Self(PhantomData), OpArg(arg.into()))
1252+
(Self(PhantomData), OpArg::new(arg.into()))
12531253
}
12541254

12551255
#[inline]
12561256
pub fn new_single(arg: T) -> (Self, OpArgByte)
12571257
where
12581258
T: Into<u8>,
12591259
{
1260-
(Self(PhantomData), OpArgByte(arg.into()))
1260+
(Self(PhantomData), OpArgByte::new(arg.into()))
12611261
}
12621262

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

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

12731273
/// # Safety
12741274
/// T::from_op_arg(self) must succeed
12751275
#[inline(always)]
12761276
pub unsafe fn get_unchecked(self, arg: OpArg) -> T {
12771277
// SAFETY: requirements forwarded from caller
1278-
unsafe { T::try_from(arg.0).unwrap_unchecked() }
1278+
unsafe { T::try_from(u32::from(arg)).unwrap_unchecked() }
12791279
}
12801280
}
12811281

‎crates/compiler-core/src/bytecode/oparg.rs‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,26 @@ pub trait OpArgType: Copy + Into<u32> + TryFrom<u32> {}
1212
/// Opcode argument that may be extended by a prior ExtendedArg.
1313
#[derive(Copy, Clone, PartialEq, Eq)]
1414
#[repr(transparent)]
15-
pub struct OpArgByte(pub u8);
15+
pub struct OpArgByte(u8);
1616

1717
impl OpArgByte {
18-
pub const fn null() -> Self {
19-
Self(0)
18+
pub const NULL: Self = Self::new(0);
19+
20+
#[must_use]
21+
pub const fn new(value: u8) -> Self {
22+
Self(value)
2023
}
2124
}
2225

2326
impl From<u8> for OpArgByte {
2427
fn from(raw: u8) -> Self {
25-
Self(raw)
28+
Self::new(raw)
29+
}
30+
}
31+
32+
impl From<OpArgByte> for u8 {
33+
fn from(value: OpArgByte) -> Self {
34+
value.0
2635
}
2736
}
2837

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

4049
impl OpArg {
41-
pub const fn null() -> Self {
42-
Self(0)
50+
pub const NULL: Self = Self::new(0);
51+
52+
#[must_use]
53+
pub const fn new(value: u32) -> Self {
54+
Self(value)
4355
}
4456

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

6678
impl From<u32> for OpArg {
6779
fn from(raw: u32) -> Self {
68-
Self(raw)
80+
Self::new(raw)
6981
}
7082
}
7183

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

100112
#[inline(always)]

‎crates/vm/src/frame.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ impl ExecutingFrame<'_> {
577577
{
578578
// YIELD_VALUE arg: 0 = direct yield, >= 1 = yield-from/await
579579
// OpArgByte.0 is the raw byte value
580-
if prev_unit.arg.0 >= 1 {
580+
if u8::from(prev_unit.arg) >= 1 {
581581
// In yield-from/await context, delegate is on top of stack
582582
return Some(self.top_value());
583583
}

0 commit comments

Comments
 (0)
X Tutup