Skip to content

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

Merged
bakpakin merged 2 commits intojanet-lang:masterfrom
cideM:docstring-fixes
Jan 14, 2025
Merged

Fix docstrings for int/u64, int/s64 and int/to-number#1544
bakpakin merged 2 commits intojanet-lang:masterfrom
cideM:docstring-fixes

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