Enhanced list indexing

[stevelinton]

This is a cleaned up and rebased version of two old pull requests #83 and #28 (Thanks to @fingolfin and @markuspf for fixing the mess I'd made). It extends support for indexing into lists using indexes other than positive small integers. All such indexing ends up with GAP-level methods. It also adds the syntax l[i1,i2,....] which is mapped to l[[i1,i2,...]]. There is infrastructure in the kernel to allow for fast methods for the case of two indices (without creating the length 2 list) but this is currently not exported to GAP level.

Tests and documentation are still needed.

[fingolfin]

Really nice, thanks. Regarding tests: I guess that would involve InstallMethod invocations to setup so that one can test some non-trivial examples. That in turn makes it a bit more difficult to re-run tests, I guess (as InstallMethod would be called again and again)? That's certainly not an objection to anything, just an observation.

Anyway: I got a crash in the testsuite with this branch:

$ make testinstall
mkdir -p dev/log
( echo 'SetUserPreference("UseColorsInTerminal",false); \
        ReadGapRoot( "tst/testutil.g" ); \
            ReadGapRoot( "tst/testinstall.g" );' | ./bin/gap-default64.sh -b -m 100m -o 750m -A -q -x 80 -r  > \
            `date -u +dev/log/testinstall1_%Y-%m-%d-%H-%M` )
( echo 'SetUserPreference("UseColorsInTerminal",false); \
        ReadGapRoot( "tst/testutil.g" ); LoadAllPackages(); \
            ReadGapRoot( "tst/testinstall.g" );' | ./bin/gap-default64.sh -b -m 100m -o 750m -A -q -x 80 -r  > \
            `date -u +dev/log/testinstall2_%Y-%m-%d-%H-%M` )
Syntax warning: New syntax used -- intentional? in /Users/mhorn/Projekte/GAP/g\
ap.github/pkg/homalg/gap/HomalgFunctor.gi line 6301
    M := First( arg, IsStructureObjectOrObjectOrMorphism );
    ^
Syntax warning: arg used not as the last argument in /Users/mhorn/Projekte/GAP\
/gap.github/pkg/singular/gap/singular.g line 2529
GapInterface := function ( func, arg, out )
                                    ^
Syntax warning: New syntax used -- intentional? in /Users/mhorn/Projekte/GAP/g\
ap.github/pkg/fr-2.1.1/gap/group.gi line 1848
    Error := function(arg)
        ^
Assertion failed: (1 <= CountExpr), function PopExpr, file ../../src/code.c, line 356.
/bin/sh: line 3: 84361 Done                    echo 'SetUserPreference("UseColorsInTerminal",false); \
        ReadGapRoot( "tst/testutil.g" ); LoadAllPackages(); \
            ReadGapRoot( "tst/testinstall.g" );'
     84362 Abort trap: 6           | ./bin/gap-default64.sh -b -m 100m -o 750m -A -q -x 80 -r > `date -u +dev/log/testinstall2_%Y-%m-%d-%H-%M`
Makefile:223: recipe for target 'testinstall' failed
make: *** [testinstall] Error 134

[stevelinton]

Yes, testing will need a little care. Can we safely assume that no one will run tests hundreds of times in the same copy of GAP? If so I can create a new filter to install methods on inside the tests.

Thanks for the crash report. I'll try LoadAllPackages().

The crash looks to be related to the partial variadic stuff rather than this, but maybe I've accidentally broken the error handling.

[fingolfin]

It may or may not be related. I do get those warnings in master, too, of course, but not the crash. I did not yet try to track down what happens, just thought I should report it.

[fingolfin]

Here is a small test case to trigger the crash:

function(M)
    local i, a;
    M{a}{a}[i][i];
end;

[stevelinton]

Thanks I’ll get into it.

S

On 9 Jul 2015, at 17:21, Max Horn [email protected] wrote:

Here is a small test case to trigger the crash:

function(M)
local i, a;
M{a}{a}[i][i];
end;

—
Reply to this email directly or view it on GitHub.

[fingolfin]

Tests pass now, and this looks pretty good overall.

One minor remark: The test checks whether functions using the new construct are printed correctly. But it only tests the multi-integer-index variant, i.e. a[1,2,3], but not a["abc"], a[[1,2,3]], a[fail, "x"], etc.

[stevelinton]

I believe this is ready to merge

[coveralls]

Changes Unknown when pulling f447fc1 on stevelinton:sl/multi-index into ** on gap-system:master**.