Better define the result of MappingPermListList

This fixes some bugs introduced in moving MappingPermListList into
the kernel:

* Crash when given negative integers.
* Crash when given non-integer types.
* Sometimes (incorrectly) returns a permutation when given two
  lists where no perm exists mapping one to the other.

This function also alters the permutation generated by
MappingPermListList to only move points in one of the input lists.
This fixes a bug in RepresentativeAction, which assumed this was
the case.

lib/permutat.g

@@ -510,7 +510,7 @@ end );
 ##  gap> PermList([1,2,4,5,3]);
 ##  (3,4,5)
 ##  gap> MappingPermListList([2,5,1,6],[7,12,8,2]);
-##  (1,8,5,12,11,10,9,6,2,7,4,3)
+##  (1,8,5,12,6,2,7)
 ##  gap> RestrictedPerm((1,2)(3,4),[3..5]);
 ##  (3,4)
 ##  ]]></Example>
@@ -560,10 +560,11 @@ end);
 ##  <Ref Func="MappingPermListList"/> returns the permutation <C>p</C> from the
 ##  previous sentence, i.e.  <A>src</A><C>[</C><M>i</M><C>]^</C><M>p =</M>
 ##  <A>dst</A><C>[</C><M>i</M><C>]</C>.
-##  The permutation <M>\pi</M> fixes all points larger than the maximum of
-##  the entries in <A>src</A> and <A>dst</A>.
+##  The permutation <M>\pi</M> fixes any point which is not in <A>src</A> or
+##  <A>dst</A>.
 ##  If there are several such permutations, it is not specified which of them
-##  <Ref Func="MappingPermListList"/> returns.
+##  <Ref Func="MappingPermListList"/> returns. If there is no such
+##  permutation, then <Ref Func="MappingPermListList"/> returns <K>fail</K>.
 ##  </Description>
 ##  </ManSection>
 ##  <#/GAPDoc>

src/permutat.c

@@ -4443,34 +4443,58 @@ Obj FuncMappingPermListList(Obj self, Obj src, Obj dst)
     Obj out;
     Obj tabdst, tabsrc;
     Int x;
+    Obj obj;
     Int mytabs[DEGREELIMITONSTACK+1];
     Int mytabd[DEGREELIMITONSTACK+1];
 
+
     l = LEN_LIST(src);
     if (l != LEN_LIST(dst)) {
-        ErrorReturnVoid( "both arguments must be lists of equal length", 
-                     0L, 0L, "type 'return;' or 'quit;' to exit break loop" );
-        return 0L;
+        ErrorMayQuit( "Arguments must be lists of equal length", 0L, 0L);
     }
     d = 0;
     for (i = 1;i <= l;i++) {
-        x = INT_INTOBJ(ELM_LIST(src,i));
+        obj = ELM_LIST(src, i);
+        if (!IS_POS_INTOBJ(obj)) {
+            ErrorMayQuit("First argument must be a list of positive integers", 0L, 0L);
+        }
+        x = INT_INTOBJ(obj);
         if (x > d) d = x;
     }
     for (i = 1;i <= l;i++) {
-        x = INT_INTOBJ(ELM_LIST(dst,i));
+        obj = ELM_LIST(dst, i);
+        if (!IS_POS_INTOBJ(obj)) {
+            ErrorMayQuit("Second argument must be a list of positive integers", 0L, 0L);
+        }
+        x = INT_INTOBJ(obj);
         if (x > d) d = x;
     }
     if (d <= DEGREELIMITONSTACK) {
         /* Small case where we work on the stack: */
         memset(&mytabs,0,sizeof(mytabs));
         memset(&mytabd,0,sizeof(mytabd));
         for (i = 1;i <= l;i++) {
-            mytabs[INT_INTOBJ(ELM_LIST(src,i))] = i;
+            Int val = INT_INTOBJ(ELM_LIST(src, i));
+            if (mytabs[val]) {
+                // Already read where this value maps, check it is the same
+                if (ELM_LIST(dst, mytabs[val]) != ELM_LIST(dst, i)) {
+                    return Fail;
+                }
+            }
+            mytabs[val] = i;
         }
         for (i = 1;i <= l;i++) {
-            mytabd[INT_INTOBJ(ELM_LIST(dst,i))] = i;
+            Int val = INT_INTOBJ(ELM_LIST(dst, i));
+            if (mytabd[val]) {
+                // Already read where this value is mapped from, check it is
+                // the same
+                if (ELM_LIST(src, mytabd[val]) != ELM_LIST(src, i)) {
+                    return Fail;
+                }
+            }
+            mytabd[val] = i;
         }
+
         out = NEW_PLIST(T_PLIST_CYC,d);
         SET_LEN_PLIST(out,d);
         /* No garbage collection from here ... */
@@ -4479,28 +4503,52 @@ Obj FuncMappingPermListList(Obj self, Obj src, Obj dst)
             if (mytabs[i]) {   /* if i is in src */
                 SET_ELM_PLIST(out,i, ELM_LIST(dst,mytabs[i]));
             } else {
-                /* Skip things in dst: */
-                while (mytabd[next]) next++;
-                SET_ELM_PLIST(out,i,INTOBJ_INT(next));
-                next++;
+                if (mytabd[i]) {
+                    // Skip things in dst:
+                    while (mytabd[next] ||
+                           (mytabs[next] == 0 && mytabd[next] == 0))
+                        next++;
+                    SET_ELM_PLIST(out, i, INTOBJ_INT(next));
+                    next++;
+                }
+                else {    // map elements in neither list to themselves
+                    SET_ELM_PLIST(out, i, INTOBJ_INT(i));
+                }
             }
         }
         /* ... to here! No CHANGED_BAG needed since this is a new object! */
     } else {
         /* Version with intermediate objects: */
-
         tabsrc = NEW_PLIST(T_PLIST,d);
         SET_LEN_PLIST(tabsrc,0);
         /* No garbage collection from here ... */
         for (i = 1;i <= l;i++) {
-            SET_ELM_PLIST(tabsrc,INT_INTOBJ(ELM_LIST(src,i)),INTOBJ_INT(i));
+            Int val = INT_INTOBJ(ELM_LIST(src, i));
+            if (ELM_PLIST(tabsrc, val)) {
+                if (ELM_LIST(dst, INT_INTOBJ(ELM_PLIST(tabsrc, val))) !=
+                    ELM_LIST(dst, i)) {
+                    return Fail;
+                }
+            }
+            else {
+                SET_ELM_PLIST(tabsrc, val, INTOBJ_INT(i));
+            }
         }
         /* ... to here! No CHANGED_BAG needed since this is a new object! */
         tabdst = NEW_PLIST(T_PLIST,d);
         SET_LEN_PLIST(tabdst,0);
         /* No garbage collection from here ... */
         for (i = 1;i <= l;i++) {
-            SET_ELM_PLIST(tabdst,INT_INTOBJ(ELM_LIST(dst,i)),INTOBJ_INT(i));
+            int val = INT_INTOBJ(ELM_LIST(dst, i));
+            if (ELM_PLIST(tabdst, val)) {
+                if (ELM_LIST(src, INT_INTOBJ(ELM_PLIST(tabdst, val))) !=
+                    ELM_LIST(src, i)) {
+                    return Fail;
+                }
+            }
+            else {
+                SET_ELM_PLIST(tabdst, val, INTOBJ_INT(i));
+            }
         }
         /* ... to here! No CHANGED_BAG needed since this is a new object! */
         out = NEW_PLIST(T_PLIST_CYC,d);
@@ -4512,10 +4560,19 @@ Obj FuncMappingPermListList(Obj self, Obj src, Obj dst)
                 SET_ELM_PLIST(out,i,
                     ELM_LIST(dst,INT_INTOBJ(ELM_PLIST(tabsrc,i))));
             } else {
-                /* Skip things in dst: */
-                while (ELM_PLIST(tabdst,next)) next++;
-                SET_ELM_PLIST(out,i,INTOBJ_INT(next));
-                next++;
+                if (ELM_PLIST(tabdst, i)) {
+                    // Skip things in dst:
+                    while (ELM_PLIST(tabdst, next) ||
+                           (ELM_PLIST(tabdst, next) == 0 &&
+                            ELM_PLIST(tabsrc, next) == 0)) {
+                        next++;
+                    }
+                    SET_ELM_PLIST(out, i, INTOBJ_INT(next));
+                    next++;
+                }
+                else {
+                    SET_ELM_PLIST(out, i, INTOBJ_INT(i));
+                }
             }
         }
         /* ... to here! No CHANGED_BAG needed since this is a new object! */

tst/testbugfix/2017-06-19-repaction.tst

@@ -0,0 +1,10 @@
+# These functions all worked incorrectly when given symmetric or alternating groups
+# Which were not not defined on a domain of the form [1..n]
+gap> RepresentativeAction(SymmetricGroup([5,7,11,15]),[7,11],[5,15],OnTuples);
+(5,7)(11,15)
+gap> RepresentativeAction(AlternatingGroup([5,7,11,15]),[7,11],[5,15],OnTuples);
+(5,7)(11,15)
+gap> RepresentativeAction(SymmetricGroup([5,7,11,15]),[7,11],[5,15],OnSets);
+(5,7)(11,15)
+gap> RepresentativeAction(AlternatingGroup([5,7,11,15]),[7,11],[5,15],OnSets);
+(5,7)(11,15)

tst/testinstall/perm.tst

@@ -309,12 +309,52 @@ gap> List(permBig, SignPerm);
 #
 # MappingPermListList
 #
+gap> MappingPermListList([],[]);
+()
 gap> MappingPermListList([1,1], [2,2]);
 (1,2)
 gap> MappingPermListList([1,2], [2,1]);
 (1,2)
 gap> MappingPermListList([1,2], [3,4]);
 (1,3)(2,4)
+gap> MappingPermListList([2,4,6], [1,2,3]);
+(1,4,2)(3,6)
+gap> MappingPermListList([1,1], [1,2]);
+fail
+gap> MappingPermListList([1,2], [1,1]);
+fail
+gap> MappingPermListList([1,1000], [1,1000]);
+()
+gap> MappingPermListList([1,1000], [1,1001]);
+(1000,1001)
+gap> MappingPermListList([1002,1000], [1,1001]);
+(1,1000,1001,1002)
+gap> MappingPermListList([1,1], [1000,1000]);
+(1,1000)
+gap> MappingPermListList([1,1], [1000,1001]);
+fail
+gap> MappingPermListList([1,2], [1000,1000]);
+fail
+gap> MappingPermListList((), []);
+Error, Length: <list> must be a list (not a permutation (small))
+gap> MappingPermListList([], ());
+Error, Length: <list> must be a list (not a permutation (small))
+gap> MappingPermListList("cheese", "cake");
+Error, both arguments must be lists of equal length
+gap> MappingPermListList("cheese", "cakeba");
+Error, Lists can only contain positive integers
+gap> MappingPermListList([1,2], [3,[]]);
+Error, Lists can only contain positive integers
+gap> MappingPermListList([1,[]], [3,4]);
+Error, Lists can only contain positive integers
+gap> MappingPermListList([1,2], [3,0]);
+Error, Lists can only contain positive integers
+gap> MappingPermListList([1,0], [3,4]);
+Error, Lists can only contain positive integers
+gap> MappingPermListList([1,-1], [3,4]);
+Error, Lists can only contain positive integers
+gap> MappingPermListList([1,2], [3,4]);
+(1,3)(2,4)
 gap> (1,128000) = ();
 false
 gap> (1,128000) * (129000, 129002);