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

Extend checked integer arithmetic #14300

Merged
merged 1 commit into from Dec 9, 2015

Conversation

Projects
None yet
5 participants
@eschnett
Contributor

eschnett commented Dec 7, 2015

Provide checked_{div, rem, fld, mod, cld} to complete the set of checked_* integer functions. Note that their regular counterparts also check for overflow, so that they currently have identical behaviour.

Rename the intrinsic sdiv, udiv, srem, urem to checked_*, since this is what they do.

Provide non-checking intrinsics (under the old, now unused names) for efficiency.

Implement mod in Julia instead of in C++ (significantly shorter). Use a more efficient algorithm based on a single div instead of on two rem.

Add tests ensuring that the new checked_* functions throw when they need to.

Add tests checking the return types of all integer arithmetic operators (regular and checked), since I was bitten during development.

Update documentation.

Note: The tests include the flipsign function, assuming that PR #14299 is present.

Show outdated Hide outdated test/int.jl Outdated
Show outdated Hide outdated src/APInt-C.cpp Outdated
@eschnett

This comment has been minimized.

Show comment
Hide comment
@eschnett

eschnett Dec 7, 2015

Contributor

Corrected AP-Int functions.

Contributor

eschnett commented Dec 7, 2015

Corrected AP-Int functions.

@eschnett

This comment has been minimized.

Show comment
Hide comment
@eschnett

eschnett Dec 7, 2015

Contributor

Documented div exception types in test cases, mirroring documentation in Base.

Contributor

eschnett commented Dec 7, 2015

Documented div exception types in test cases, mirroring documentation in Base.

Extend checked integer arithmetic
Provide checked_{div, rem, fld, mod, cld} to complete the set of checked_* integer functions. Note that their regular counterparts also check for overflow, so that they currently have identical behaviour.
Rename the intrinsic sdiv, udiv, srem, urem to checked_*, since this is what they do.
Provide non-checking intrinsics (under the old, now unused names) for efficiency.
Implement mod in Julia instead of in C++ (significantly shorter). Use a more efficient algorithm based on a single div instead of on two rem.
Add tests ensuring that the new checked_* functions throw when they need to.
Add tests checking the return types of all integer arithmetic operators (regular and checked), since I was bitten during development.
Update documentation.

Note: The tests include flipsign, assuming that PR #14299 is present.
@ScottPJones

This comment has been minimized.

Show comment
Hide comment
@ScottPJones

ScottPJones Dec 8, 2015

Contributor

This looks very nice to me - in my copious spare time I'm reimplementing some of the decNumber library in Julia (I've got the C code mostly all wrapped, except for the arbitrary precision format(s), but would rather have pure Julia, and not be dealing with references and pointers), and actually having the faster unchecked versions would be quite useful for that.
Having the other checked functions is also very useful. Good stuff! 👍

Contributor

ScottPJones commented Dec 8, 2015

This looks very nice to me - in my copious spare time I'm reimplementing some of the decNumber library in Julia (I've got the C code mostly all wrapped, except for the arbitrary precision format(s), but would rather have pure Julia, and not be dealing with references and pointers), and actually having the faster unchecked versions would be quite useful for that.
Having the other checked functions is also very useful. Good stuff! 👍

@eschnett

This comment has been minimized.

Show comment
Hide comment
@eschnett

eschnett Dec 8, 2015

Contributor

I'll proposed the unchecked versions as soon as this lands; otherwise, I'm afraid there'll be merge conflicts.

Contributor

eschnett commented Dec 8, 2015

I'll proposed the unchecked versions as soon as this lands; otherwise, I'm afraid there'll be merge conflicts.

@jakebolewski

This comment has been minimized.

Show comment
Hide comment
@jakebolewski

jakebolewski Dec 8, 2015

Member

If the intention is to have a @checked macro similar to @fastmath, wouldn't it be better to have a CheckedMath module and provide a similar API to fast math operations (div_checked as opposed to checked_div)?

Member

jakebolewski commented Dec 8, 2015

If the intention is to have a @checked macro similar to @fastmath, wouldn't it be better to have a CheckedMath module and provide a similar API to fast math operations (div_checked as opposed to checked_div)?

@eschnett

This comment has been minimized.

Show comment
Hide comment
@eschnett

eschnett Dec 8, 2015

Contributor

Yes, we can add a module. It probably should live in Base since it requires new intrinsics. We can also rename things; I was following the existing convention. My plan is to first introduce the functionality, and add an @checked macro later.

Contributor

eschnett commented Dec 8, 2015

Yes, we can add a module. It probably should live in Base since it requires new intrinsics. We can also rename things; I was following the existing convention. My plan is to first introduce the functionality, and add an @checked macro later.

@jakebolewski

This comment has been minimized.

Show comment
Hide comment
@jakebolewski
Member

jakebolewski commented Dec 9, 2015

LGTM

eschnett added a commit that referenced this pull request Dec 9, 2015

Merge pull request #14300 from eschnett/eschnett/checked
Extend checked integer arithmetic

@eschnett eschnett merged commit a81101b into JuliaLang:master Dec 9, 2015

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eschnett eschnett deleted the eschnett:eschnett/checked branch Dec 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment