Skip to content

Fix int [r]divmod, Add rfloordiv#1239

Merged
windelbouwman merged 3 commits into
RustPython:masterfrom
youknowone:fix-divmod
Aug 13, 2019
Merged

Fix int [r]divmod, Add rfloordiv#1239
windelbouwman merged 3 commits into
RustPython:masterfrom
youknowone:fix-divmod

Conversation

@youknowone
Copy link
Copy Markdown
Member

@youknowone youknowone commented Aug 12, 2019

This PR allows import datetime

If anyone interested in, the workflow is captured as an example patch for sprint.
https://www.slideshare.net/YunWonJeong/pycon-kr-2019-sprint-rustpython-by-example

Comment thread vm/src/obj/objint.rs Outdated
if i2.is_zero() {
Err(vm.new_zero_division_error("integer division by zero".to_string()))
} else {
let modulo = (i1 % i2 + i2) % i2;
Copy link
Copy Markdown
Contributor

@windelbouwman windelbouwman Aug 12, 2019

Choose a reason for hiding this comment

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

Did you have a look at div_rem ? It might be more efficient for big integers. Another option might be div_mod_floor.

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.

div_rem was the original implementation which didn't match to python. I fixed them to div_floor and div_mod_floor, thanks.

Comment thread vm/src/obj/objint.rs Outdated
fn rfloordiv(&self, other: PyObjectRef, vm: &VirtualMachine) -> PyResult {
if objtype::isinstance(&other, &vm.ctx.int_type()) {
let v1 = get_value(&other);
let (div, _) = divmod(vm, v1, &self.value)?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you check on div_floor? This might be faster than full divmod.

@youknowone youknowone force-pushed the fix-divmod branch 2 times, most recently from d0e8ee5 to cde8802 Compare August 13, 2019 01:47
@windelbouwman windelbouwman merged commit c7bee80 into RustPython:master Aug 13, 2019
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