Skip to content

Fix docstrings for int/u64, int/s64 and int/to-number #1544

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

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

cideM
Copy link
Contributor

@cideM cideM commented Jan 11, 2025

All credits go to @sogaiu (see this Zulip chat).

The current doc strings for int/u64 and int/s64 say that these functions only support strings. But as far as I can tell from the diff they always support both numbers and strings, and the test suites cover those cases. I therefore updated the docstrings to indicate that.

int/to-number used to only supports int32 values but this commit added support for up to JANET_INTMAX_INT64.

This isn't the most ground breaking commit but I was certainly a bit confused about what I can do with these functions when I was dealing with bxoring 64bit integers for Advent of Code.

cideM added 2 commits January 11, 2025 22:46
Update the cfun_to_number docstring to indicate that it handles values
up to JANET_INTMAX_INT64 (75845c0),
rather than up to int32, what the current docstring says.
Update the docstrings of the u64 and s64 functions to indicate that they
work on numbers as well strings. Previously the docstring only mentioned
string support.
@sogaiu
Copy link
Contributor

sogaiu commented Jan 14, 2025

As an additional data point, it looks like:

(int/to-number (+ (int/u64 (math/pow 2 53)) 1))

leads to an error currently:

error: cannot convert <core/u64 9007199254740993> to a number, must be in the range [-9007199254740992, 9007199254740992]
  in int/to-number [src/core/inttypes.c] on line 206
  in thunk [repl] (tail call) on line 19, column 1

FWIW, one less seems to work:

(int/to-number (int/u64 (math/pow 2 53)))
# =>
9007199254740992

@bakpakin bakpakin merged commit 2b49903 into janet-lang:master Jan 14, 2025
12 of 13 checks passed
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.

3 participants