search

TritonVM / tasm-lang

Writing tasm with Rust syntax
15 stars 2 forks source link

Implement `ast::BinOp::Rem` #26

Closed einar-triton closed 1 year ago

einar-triton commented 1 year ago

This is inspired by the ast::BinOp::Div, and adds some previously missing tests for Div and Mul as well.

Sword-Smith commented 1 year ago

Looks good. We only want this for u32-based types though. So we shouldn't support remainder for BFieldElement and XFieldElement.

I might cherry-pick some stuff from this branch if that's OK with you, as I'm currenly working on u64 division where I could use the u32-remainder :)

einar-triton commented 1 year ago

Looks good. We only want this for u32-based types though. So we shouldn't support remainder for BFieldElement and XFieldElement.

Gotcha.

I might cherry-pick some stuff from this branch if that's OK with you, as I'm currenly working on u64 division where I could use the u32-remainder :)

Naturally.

einar-triton commented 1 year ago

All tests pass. Clippy OK.

Please review.

Sword-Smith commented 1 year ago

I used your Rem implementation and credited you with it in one of the commits.

I managed though to find a way of doing generic u64-division, but that function also returns the remainder. So I could use that function for both Rem and Div ofu64. So that deprecates the work you've done forRemofu64`.