Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add divide by zero check to BigInteger mod #22774

Merged
merged 8 commits into from
Jul 20, 2023

Conversation

jabraham17
Copy link
Member

@jabraham17 jabraham17 commented Jul 19, 2023

Adds a divide by zero check to BigInteger.mod, BigInteger.% and BigInteger.%= to match the divide functions (#19384). While doing this work, implemented part of the change for argument names (#19005 (comment)).

Summary of changes

  • add a divide by zero check to mod
  • reimplement mod in terms of the divR with the proper rounding argument
  • add helper methods for modTrunc by a integral to reduce code duplication
  • deprecate argument names a/b on mod in favor of x/y

New tests

  • adds test/library/standard/BigInteger/modByZero.chpl
  • adds test/deprecated/BigInteger/argumentNames.chpl

Testing

  • paratest with futures
  • paratest + gasnet
  • built and checked docs

[Reviewed by @bmcdonald3]

closes #19384
closes cray/chapel-private#5094

these are used to implement mod operators with less code duplication

Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Copy link
Member

@bmcdonald3 bmcdonald3 left a comment

Choose a reason for hiding this comment

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

This looks good to me. The one thing that I was thinking of that may make sense would be to use boundsChecking, rather than chpl_checkDivByZero, since that is what we typically use for module-level halts, but I think since this one is specifically divide by zero checks, I feel fine with how you've gone here.

@jabraham17 jabraham17 merged commit d7c6c9c into chapel-lang:main Jul 20, 2023
7 checks passed
@jabraham17 jabraham17 deleted the mod-div-check branch July 20, 2023 15:44
jabraham17 added a commit that referenced this pull request Jan 11, 2024
Removes almost all of the remaining BigInteger deprecations

This PR removes the deprecations from the following PRs I authored
- #22775
- #22888/#22774
- #22818
- #22794
- #22788
- #22121

I also removed the remaining deprecations done by Yash Raj
- #18855
- #18827

I moved one test out of the deprecated/BigInteger directory,
`bigint_getlimbs.chpl`, since its not a deprecation test and actually
checks functionality.


Testing
- [x] paratest without comm
- [x] paratest with comm
- [x] built docs and checked them

[Reviewed by @ShreyasKhandekar]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants