Skip to content

math.big: change how to hanle negatives in div_mod(), add tests#24713

Merged
spytheman merged 1 commit into
vlang:masterfrom
tankf33der:div_mod_sign
Jun 15, 2025
Merged

math.big: change how to hanle negatives in div_mod(), add tests#24713
spytheman merged 1 commit into
vlang:masterfrom
tankf33der:div_mod_sign

Conversation

@tankf33der

@tankf33der tankf33der commented Jun 13, 2025

Copy link
Copy Markdown
Contributor

Tests against gmp show that div_mod() behaves incorrectly.
Everything should be in truncate format regardless of input parameters.
This patch fixes this issue.

  1. Now /, % and div_mod works as mpz_tdiv_q, mpz_tdiv_r and mpz_tdiv_qr respectively. Tested against gmp and flint libraries.
  2. Added tests in big_test.v compared against julia lang.

@huly-for-github

Copy link
Copy Markdown

Connected to Huly®: V_0.6-23067

@spytheman spytheman left a comment

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.

Excellent work.

Comment thread vlib/math/big/big_test.v
Comment thread vlib/math/big/big_test.v
Comment thread vlib/math/big/big_test.v
@spytheman

Copy link
Copy Markdown
Contributor

The CI failure seems unrelated, but I'll need to check more thoroughly 🤔 .

@spytheman

Copy link
Copy Markdown
Contributor

The CI failure was indeed unrelated - after the third retry it worked, and it also happened for another PR.

@spytheman spytheman merged commit 6e271b2 into vlang:master Jun 15, 2025
72 of 74 checks passed
@tankf33der

Copy link
Copy Markdown
Contributor Author

Thank you for merge.

I decided to add a comment for history and science - my function in julia that I used to verify tests is divrem().

Successfully passing 100M loop against gmp's mpz_tdiv_qr gives me more confidence that I implemented everything correctly.

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