Skip to content

Commit

Permalink
Clear stored inverse when calling TrimPerm
Browse files Browse the repository at this point in the history
This is required if TrimPerm changes the TNUM of the permutation,
as a permutation and its stored inverse must have the same TNUM.
This is also required if TrimPerm changes the permutation (although
this would break many other things if anyone did it)
  • Loading branch information
ChrisJefferson authored and fingolfin committed Jul 17, 2019
1 parent ac5c014 commit 1bd244f
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/permutat.cc
Expand Up @@ -2238,6 +2238,7 @@ Obj Array2Perm (

void TrimPerm(Obj perm, UInt m)
{
CLEAR_STOREDINV_PERM(perm);
if (TNUM_OBJ(perm) == T_PERM2) {
GAP_ASSERT(m <= DEG_PERM2(perm));
ResizeBag(perm, SIZEBAG_PERM2(m));
Expand Down
19 changes: 18 additions & 1 deletion src/permutat.h
Expand Up @@ -76,7 +76,12 @@ EXPORT_INLINE const UInt4 * CONST_ADDR_PERM4(Obj perm)

EXPORT_INLINE Obj STOREDINV_PERM(Obj perm)
{
return ADDR_OBJ(perm)[0];
Obj inv = ADDR_OBJ(perm)[0];
/* Check inv has the same TNAM as perm
This is checked below in SET_STOREDINV_PERM, but could
be invalidated if either perm or inv is changed in-place */
GAP_ASSERT(!inv || (TNUM_OBJ(perm) == TNUM_OBJ(inv)));
return inv;
}

/* SET_STOREDINV_PERM should only be used in neither perm, nor inv has
Expand All @@ -98,6 +103,18 @@ EXPORT_INLINE void SET_STOREDINV_PERM(Obj perm, Obj inv)
}
}

/* Clear the stored inverse. This is required if 'perm' changes TNUM.
Also clears the stored inverse of the stored inverse (which should be
perm). */
EXPORT_INLINE void CLEAR_STOREDINV_PERM(Obj perm)
{
Obj inv = ADDR_OBJ(perm)[0];
if (inv) {
ADDR_OBJ(inv)[0] = 0;
ADDR_OBJ(perm)[0] = 0;
}
}


#define IMAGE(i,pt,dg) (((i) < (dg)) ? (pt)[(i)] : (i))

Expand Down
40 changes: 40 additions & 0 deletions tst/testbugfix/2019-07-15-StoredInv.tst
@@ -0,0 +1,40 @@
# When TRIM_PERM causes a permutation 'p' to change TNAM, the stored inverse
# must be cleared, as the stored inverse of 'p' must have the
# same TNAM as 'p'

# Make p a Perm4
gap> p := (1,2,3,4)*(2^16,2^16+1)*(2^16,2^16+1);
(1,2,3,4)
gap> IsPerm4Rep(p);
true

# Force inverse calculation
gap> q := p^-1;
(1,4,3,2)
gap> IsPerm4Rep(q);
true

# Now trim
gap> TRIM_PERM(p, 4);
gap> IsPerm2Rep(p);
true

# Check inverse is also a perm2
gap> IsPerm2Rep(p^-1);
true

# But this has not changed q
gap> IsPerm4Rep(q);
true

# and it's inverse is the correct type
gap> IsPerm4Rep(q^-1);
true

# Check some calculations that use the inverse
gap> List([1..5], x -> x/p);
[ 4, 1, 2, 3, 5 ]

# And on q (to ensure it's inverse is not still 'p')
gap> List([1..5], x -> x/q);
[ 2, 3, 4, 1, 5 ]

0 comments on commit 1bd244f

Please sign in to comment.