This doesn't completely fix the problem, it just shifts the range that the problem occurs, because if a PyInt can't fit into a i32 it still overflows. I recommend changing base to be a PyIntRef and passing a &BigInt around instead of an i32.

Thank you for reviewing. I Change base to PyIntRef. And i also add test for minus base and Large value than max of i32.

If you want to prevent using BigInt here, you can change the signature of to_int to accept an Into<BigInt> instead of bigint itself. This will allow you to call the function with 10 and with a BigInt value as well.

@windelbouwman Thank you for reviewing. I agree that change signature of to_int will make to_int function more general.

But I have some questions. When we use base type to u32, we can easily pass the argument because u32 have copy trait. And when we use base type to pyIntRef, we can also pass reference of PyInt by as_bigint method.

But If i changed signature of to_int function. i cannot pass base to to_int function. Because PyInt does not have copy trait. Just simple modifying make this error.

error[E0507]: cannot move out of dereference of `pyobject::PyRef<obj::objint::PyInt>`
   --> vm/src/obj/objint.rs:721:34
    |
721 |                 to_int(vm, &val, base.value)
    |                                  ^^^^^^^^^^ move occurs because value has type `num_bigint::bigint::BigInt`, which does not implement the `Copy` trait

error: aborting due to previous error

And also change base type to BigInt occurs other error.

error[E0277]: the trait bound `num_bigint::bigint::BigInt: pyobject::TryFromObject` is not satisfied
   --> vm/src/obj/objint.rs:702:10
    |
702 | #[derive(FromArgs)]
    |          ^^^^^^^^ the trait `pyobject::TryFromObject` is not implemented for `num_bigint::bigint::BigInt`
    | 
   ::: vm/src/pyobject.rs:953:5
    |
953 |     fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self>;
    |     ---------------------------------------------------------------------------- required by `pyobject::TryFromObject::try_from_object`

Unfortunately, I'm not good at rust. so please give me an opinion. thx

Ah, okay, the copy trait is important here. I'm not a rust wizard either, so I think the current situation is probably ok. Maybe other people could have a look at this?

I think that the current solution is fine as well, passing an &BigInt rather than an Into<BigInt> removes the need for cloning as well.