Skip to content

Commit c8b4d63

Browse files
ShaharNavehShaharNaveh
authored
Newtype for LoadAttr oparg (#7047)
1 parent d54cf8f commit c8b4d63

5 files changed

Lines changed: 90 additions & 35 deletions

File tree

‎crates/codegen/src/compile.rs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ use rustpython_compiler_core::{
3030
bytecode::{
3131
self, AnyInstruction, Arg as OpArgMarker, BinaryOperator, BuildSliceArgCount, CodeObject,
3232
ComparisonOperator, ConstantData, ConvertValueOparg, Instruction, IntrinsicFunction1,
33-
Invert, LoadSuperAttr, OpArg, OpArgType, PseudoInstruction, SpecialMethod, UnpackExArgs,
34-
encode_load_attr_arg,
33+
Invert, LoadAttr, LoadSuperAttr, OpArg, OpArgType, PseudoInstruction, SpecialMethod,
34+
UnpackExArgs,
3535
},
3636
};
3737
use rustpython_wtf8::Wtf8Buf;
@@ -7799,14 +7799,20 @@ impl Compiler {
77997799
/// Emit LOAD_ATTR for attribute access (method=false).
78007800
/// Encodes: (name_idx << 1) | 0
78017801
fn emit_load_attr(&mut self, name_idx: u32) {
7802-
let encoded = encode_load_attr_arg(name_idx, false);
7802+
let encoded = LoadAttr::builder()
7803+
.name_idx(name_idx)
7804+
.is_method(false)
7805+
.build();
78037806
self.emit_arg(encoded, |arg| Instruction::LoadAttr { idx: arg })
78047807
}
78057808

78067809
/// Emit LOAD_ATTR with method flag set (for method calls).
78077810
/// Encodes: (name_idx << 1) | 1
78087811
fn emit_load_attr_method(&mut self, name_idx: u32) {
7809-
let encoded = encode_load_attr_arg(name_idx, true);
7812+
let encoded = LoadAttr::builder()
7813+
.name_idx(name_idx)
7814+
.is_method(true)
7815+
.build();
78107816
self.emit_arg(encoded, |arg| Instruction::LoadAttr { idx: arg })
78117817
}
78127818

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,12 @@ use rustpython_wtf8::{Wtf8, Wtf8Buf};
1717
pub use crate::bytecode::{
1818
instruction::{
1919
AnyInstruction, Arg, Instruction, InstructionMetadata, PseudoInstruction, StackEffect,
20-
decode_load_attr_arg, encode_load_attr_arg,
2120
},
2221
oparg::{
2322
BinaryOperator, BuildSliceArgCount, CommonConstant, ComparisonOperator, ConvertValueOparg,
24-
IntrinsicFunction1, IntrinsicFunction2, Invert, Label, LoadSuperAttr, MakeFunctionFlags,
25-
NameIdx, OpArg, OpArgByte, OpArgState, OpArgType, RaiseKind, ResumeType, SpecialMethod,
26-
UnpackExArgs,
23+
IntrinsicFunction1, IntrinsicFunction2, Invert, Label, LoadAttr, LoadSuperAttr,
24+
MakeFunctionFlags, NameIdx, OpArg, OpArgByte, OpArgState, OpArgType, RaiseKind, ResumeType,
25+
SpecialMethod, UnpackExArgs,
2726
},
2827
};
2928

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

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
BorrowedConstant, Constant, InstrDisplayContext,
66
oparg::{
77
BinaryOperator, BuildSliceArgCount, CommonConstant, ComparisonOperator,
8-
ConvertValueOparg, IntrinsicFunction1, IntrinsicFunction2, Invert, Label,
8+
ConvertValueOparg, IntrinsicFunction1, IntrinsicFunction2, Invert, Label, LoadAttr,
99
LoadSuperAttr, MakeFunctionFlags, NameIdx, OpArg, OpArgByte, OpArgType, RaiseKind,
1010
SpecialMethod, UnpackExArgs,
1111
},
@@ -168,7 +168,7 @@ pub enum Instruction {
168168
i: Arg<u32>,
169169
} = 79,
170170
LoadAttr {
171-
idx: Arg<NameIdx>,
171+
idx: Arg<LoadAttr>,
172172
} = 80,
173173
LoadCommonConstant {
174174
idx: Arg<CommonConstant>,
@@ -815,17 +815,17 @@ impl InstructionMetadata for Instruction {
815815
Self::ListAppend { i } => w!(LIST_APPEND, i),
816816
Self::ListExtend { i } => w!(LIST_EXTEND, i),
817817
Self::LoadAttr { idx } => {
818-
let encoded = idx.get(arg);
819-
let (name_idx, is_method) = decode_load_attr_arg(encoded);
820-
let attr_name = name(name_idx);
821-
if is_method {
818+
let oparg = idx.get(arg);
819+
let oparg_u32 = u32::from(oparg);
820+
let attr_name = name(oparg.name_idx());
821+
if oparg.is_method() {
822822
write!(
823823
f,
824824
"{:pad$}({}, {}, method=true)",
825-
"LOAD_ATTR", encoded, attr_name
825+
"LOAD_ATTR", oparg_u32, attr_name
826826
)
827827
} else {
828-
write!(f, "{:pad$}({}, {})", "LOAD_ATTR", encoded, attr_name)
828+
write!(f, "{:pad$}({}, {})", "LOAD_ATTR", oparg_u32, attr_name)
829829
}
830830
}
831831
Self::LoadBuildClass => w!(LOAD_BUILD_CLASS),
@@ -1292,17 +1292,3 @@ impl<T: OpArgType> fmt::Debug for Arg<T> {
12921292
write!(f, "Arg<{}>", core::any::type_name::<T>())
12931293
}
12941294
}
1295-
1296-
/// Encode LOAD_ATTR oparg: bit 0 = method flag, bits 1+ = name index.
1297-
#[inline]
1298-
pub const fn encode_load_attr_arg(name_idx: u32, is_method: bool) -> u32 {
1299-
(name_idx << 1) | (is_method as u32)
1300-
}
1301-
1302-
/// Decode LOAD_ATTR oparg: returns (name_idx, is_method).
1303-
#[inline]
1304-
pub const fn decode_load_attr_arg(oparg: u32) -> (u32, bool) {
1305-
let is_method = (oparg & 1) == 1;
1306-
let name_idx = oparg >> 1;
1307-
(name_idx, is_method)
1308-
}

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,3 +754,68 @@ impl From<LoadSuperAttrBuilder> for LoadSuperAttr {
754754
builder.build()
755755
}
756756
}
757+
758+
#[derive(Clone, Copy)]
759+
pub struct LoadAttr(u32);
760+
761+
impl LoadAttr {
762+
#[must_use]
763+
pub const fn new(value: u32) -> Self {
764+
Self(value)
765+
}
766+
767+
#[must_use]
768+
pub fn builder() -> LoadAttrBuilder {
769+
LoadAttrBuilder::default()
770+
}
771+
772+
#[must_use]
773+
pub const fn name_idx(self) -> u32 {
774+
self.0 >> 1
775+
}
776+
777+
#[must_use]
778+
pub const fn is_method(self) -> bool {
779+
(self.0 & 1) == 1
780+
}
781+
}
782+
783+
impl OpArgType for LoadAttr {}
784+
785+
impl From<u32> for LoadAttr {
786+
fn from(value: u32) -> Self {
787+
Self::new(value)
788+
}
789+
}
790+
791+
impl From<LoadAttr> for u32 {
792+
fn from(value: LoadAttr) -> Self {
793+
value.0
794+
}
795+
}
796+
797+
#[derive(Clone, Copy, Default)]
798+
pub struct LoadAttrBuilder {
799+
name_idx: u32,
800+
is_method: bool,
801+
}
802+
803+
impl LoadAttrBuilder {
804+
#[must_use]
805+
pub const fn build(self) -> LoadAttr {
806+
let value = (self.name_idx << 1) | (self.is_method as u32);
807+
LoadAttr::new(value)
808+
}
809+
810+
#[must_use]
811+
pub const fn name_idx(mut self, value: u32) -> Self {
812+
self.name_idx = value;
813+
self
814+
}
815+
816+
#[must_use]
817+
pub const fn is_method(mut self, value: bool) -> Self {
818+
self.is_method = value;
819+
self
820+
}
821+
}

‎crates/vm/src/frame.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
function::{PyCell, PyCellRef, PyFunction},
1111
tuple::{PyTuple, PyTupleRef},
1212
},
13-
bytecode::{self, Instruction, LoadSuperAttr},
13+
bytecode::{self, Instruction, LoadAttr, LoadSuperAttr},
1414
convert::{IntoObject, ToPyResult},
1515
coroutine::Coro,
1616
exceptions::ExceptionCtor,
@@ -2890,12 +2890,11 @@ impl ExecutingFrame<'_> {
28902890
Ok(None)
28912891
}
28922892

2893-
fn load_attr(&mut self, vm: &VirtualMachine, oparg: u32) -> FrameResult {
2894-
let (name_idx, is_method) = bytecode::decode_load_attr_arg(oparg);
2895-
let attr_name = self.code.names[name_idx as usize];
2893+
fn load_attr(&mut self, vm: &VirtualMachine, oparg: LoadAttr) -> FrameResult {
2894+
let attr_name = self.code.names[oparg.name_idx() as usize];
28962895
let parent = self.pop_value();
28972896

2898-
if is_method {
2897+
if oparg.is_method() {
28992898
// Method call: push [method, self_or_null]
29002899
let method = PyMethod::get(parent.clone(), attr_name, vm)?;
29012900
match method {

0 commit comments

Comments
 (0)