Skip to content

Conversation

@ChrisJefferson
Copy link
Contributor

This is a test PR, I'm curious how many things break.

This adds support for ranges outside the usual 'small integer' type. It is not finished and likely to break all kinds of things.

Also, many functions are not yet supported with 'large ranges' -- but they should produce clean error messages.

I'm mainly curious if people think this is an interesting direction to take.

FOR INFO: Partly as a test to see how well it works, this was mostly written by 'Claude Code'. I have checked tests initially pass but I haven't carefully checked all the code -- I would obviously do this before a future merge, along with a lot more testing.

@ChrisJefferson ChrisJefferson added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Jan 22, 2026
# length
gap> f(-2^60,2^60-1);
Error, Range: the length of a range must be a small integer
[ Error, Length of range is too large
Copy link
Contributor

Choose a reason for hiding this comment

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

looks as if the opening bracket gets printed before the error occurs, is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's definately a bug. This code still needs quite a bit of polishing, but I was curious if the basic idea looks reasonable.

@ChrisJefferson
Copy link
Contributor Author

One package which uses ranges is cvec, here is an example of what happens if you try to use large ranges in cvec (that's a horrible error message, need improvement, but basically when cvec tries to get the step of a range as an 'Int', then an error is thrown).

gap> c := CVEC_NewCVecClass(2,1,2);;
gap> v := CVec([0*Z(2),Z(2)^0], c);
<cvec over GF(2,1) of length 2>
gap> v{[2^64..2^64+1]};
Error, GET_INC_RANGE: <range> this function only supports small ranges (use GET_INC_RANGE_BIGINT) (not a list (range,ssort)) in
  CVEC_SLICE_LIST( v, w, r, [ 1 .. Length( r ) ] ); at /home/caj/files/reps/gap/gap/pkg/cvec/gap/cvec.gi:1155 called from
<function "{} for a cvec and a range">( <arguments> )
 called from read-eval loop at *stdin*:5
type 'quit;' to quit to outer loop

@fingolfin
Copy link
Member

I was not aware of this PR but discussed such changes with @ChrisJefferson on Slack, so here is a summary of my stance:

Anyway, from my POV the main obstacle of supporting "large ranges" by modifying the existing T_RANGE... types are:

  1. at least one package (cvec) handles those in its kext and thus making changes there has to be orchestrated carefully.
  2. the GAP kernel handles "small" (size fits into an immediate int) and "large" lists quite differently in many places
  3. a lot of the T_RANGE code in src/range.c intrinsically uses that everything fits into an int.

As such I don't find it very appealing to modify the existing T_RANGE code to use GAP intobj instead of machine ints. Instead I'd favor handling them via a new type. And I would suggest implementing most of it in GAP; the kernel would of course still need to be modified at the places were it turns range literals into range objects.

[...]

[...] there are a lot of things to trip over. Like it now must install a ISBB_LIST method; must adjust the type flags for T_RANGE (to have it not imply IsSmallList anymore) and a ton of other subtle points.

[...]

as I wrote, i would not want to [modify or] duplicate T_RANGE into T_RANGE_BIG. Rather, I'd write the new type in pure GAP. Easy to debug, few chances for crashes. The only kernel change that would be needed is in the interpreter / executor when turning a range literal into a range object; that would need to know about the new type in some way (e.g. by invoking a new kernel operation ConstructRange(start,step,end) which would check whether the arguments lead to a small range (then construct that) or else go to method disptch


Note that there are a few place in the kernel that do special things for T_RANGE, such as "list slicing" for compressed vectors, and those places would not know how to handle a "big range". But that's IMHO fine: assuming we implement things so that the "big range" type never is used to represent a "small range" (just like T_INTPOS never is used to represent a value that could fit into a T_INT), then indeed there is no point in supporting list slicing with such big ranges: no compressed vector will have a length exceeding $2^{60}$!

@@ -302,18 +412,20 @@ static Obj Elm0vRange(Obj list, Int pos)
static Obj ElmRange(Obj list, Int pos)
Copy link
Member

Choose a reason for hiding this comment

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

Note that this kernel function only deals with "small" positions.

Is the idea of the "big ranges" in this PR to support ranges where the end points are "large" (= don't fit into immediate ints), but the overall length is still "small" (so that it whole range would still be allowed in the IsSmallList filter) ? Or is it meant to also represent ranges were the length is also large? In the latter case, a lot more needs to be done than what is in this PR.

If one restricts to ranges with large endpoints but small length, I guess this PR could be made to work, but from a user's perspective that seems like a somewhat strange restriction.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the example that motivated this pull request, both the upper bound and the length are large.

Comment on lines +13 to +14
# Basic large integer ranges
gap> result := [2^64..2^64 + 10];;
Copy link
Member

Choose a reason for hiding this comment

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

More interesting: try result:=[1..2^100]; and then try things like Length(result) or IsBound(result[2^100])

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

Labels

do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants