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

Bigint #71

Merged
merged 13 commits into from
Mar 19, 2019
Merged

Bigint #71

merged 13 commits into from
Mar 19, 2019

Conversation

jfcap
Copy link

@jfcap jfcap commented Mar 14, 2019

Fixes #67

Bigint for Janet implementation proposal

  • added core/int64 & core/uint64 bigint boxed types with basic arithmetic and logic methods.
  • added (u)int64 typed arrays back.

@bakpakin
Copy link
Member

Another case that triggers an exception is INT64_MIN / -1, which affects all signed integer types on most architectures - we should probably try to detect this.

@jfcap
Copy link
Author

jfcap commented Mar 14, 2019

Fixed in jfcap@c5217da

janet:0:> (def min (bigint/int64 "-0x8000000000000000"))
-9223372036854775808
janet:47:> (:/ min -1)
error: INT64_MIN divided by -1
  in <cfunction>
  in _thunk [repl] (tailcall) at (48:58)
janet:73:> (:% min -1)
error: INT64_MIN divided by -1
  in <cfunction>
  in _thunk [repl] (tailcall) at (74:84)

for better compatibility with default janet number reader
@bakpakin
Copy link
Member

Thanks for all the changes! I will give these changes a spin soon and probably merge everything if there are no immediate issues.

Just a minor thing about names -- "bigint" usually implies python/GMP style integers of unlimited precision. These 64 bit integers have a different role, mainly providing an interface to C APIs where 64 bits integers are used. Perhaps we could rename the module to just int64, rename the constructors s and u (signed and unsigned), and name the types something like s64 and u64, linux kernel / rust style? I think it would better signal the intent of the module to newcomers.

@jfcap
Copy link
Author

jfcap commented Mar 17, 2019

Feel free to rename everything as you think it's better.

A lot of refactoring larger integer types. Fix a number
of casting errors, but mostly rename things. Also try to
limit use of template-like macros as they bloat the binary
if not used in moderation. We were able to reduce the size of
typed array code as well by using a single view types.
@bakpakin
Copy link
Member

A bit of an explanation for the changes I have made:

We can still use and create typed arrays with 64 bit integers, even if JANET_NO_INT_TYPES (previously JANET_NO_BIGINT) is set. However, we can't get and set in them from Janet without the int library enabled. This also means that the marshaling format is consistent, regardless of how compiler defines are set.

I have also made typed array views all share a single abstract type. This is kind of a large change for this branch, but it made code cleaner. The main difference for a user is typed arrays now all print out as <ta/view 0x12345678> when printed and share a type :ta/view. This has ups and downs, but I want to avoid creating many types, especially in the core.

@bakpakin bakpakin merged commit bad0406 into janet-lang:master Mar 19, 2019
@jfcap
Copy link
Author

jfcap commented Mar 19, 2019

Thanks for the merge.

After your changes there is some "dead code" in pp.c

        case JANET_ABSTRACT: {
#ifdef JANET_BIGINT
            JanetBigintType bt = janet_is_bigint(x);
            if (bt == JANET_BIGINT_TYPE_int64) {
                int64_to_string_b(buffer, *(int64_t *)janet_unwrap_abstract(x));
                return;
            }
            if (bt == JANET_BIGINT_TYPE_uint64) {
                uint64_to_string_b(buffer, *(uint64_t *)janet_unwrap_abstract(x));
                return;
            }
#endif

The good way to correct this is probably to implement your proposal #73

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