Skip to content

int::try_to_primitive -> PyInt::try_to_primitive - #3142

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:int-primitive
Sep 27, 2021
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowone:int-primitive

Conversation

@youknowone

Copy link
Copy Markdown
Member

No description provided.

Comment thread vm/src/builtins/int.rs
impl_into_pyobject_int!(isize i8 i16 i32 i64 usize u8 u16 u32 u64 BigInt);

pub fn try_to_primitive<'a, I>(i: &'a BigInt, vm: &VirtualMachine) -> PyResult<I>
pub(crate) fn try_to_primitive<'a, I>(i: &'a BigInt, vm: &VirtualMachine) -> PyResult<I>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this used elsewhere in the crate after this change? If not, why not make it private?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vm/src/iterator.rs uses it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like that place can easily be changed to accommodate this change, either the case, this lgtm, I'll leave the choice of if you want to change it up to you.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still related to stdlib work. let me clean it up with it later.

@DimitrisJim DimitrisJim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks

@youknowone
youknowone merged commit 81603b5 into RustPython:main Sep 27, 2021
@youknowone
youknowone deleted the int-primitive branch September 27, 2021 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants