Skip to content

Commit

Permalink
Restrict Size method for cyclic groups
Browse files Browse the repository at this point in the history
This method needs to test whether a group element is the identity; but this
can lead to an infinite recursion if the input is an fp group, as then this
equality check may end up trying to compute the group size. This currently
actually happens when all packages are loaded (with only the default set of
filters, another Size method is selected for cyclic fp groups).

We fix this by adding the `CanEasilyCompareElements` filter to the argument
filters of the method. Strictly speaking, we'd also want to check for
something like `CanEasilyComputerElementOrder`, but that doesn't exist.
  • Loading branch information
fingolfin committed Jul 17, 2019
1 parent b5373a3 commit 2e9ace1
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/grp.gi
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ InstallMethod( IsCyclic,

InstallMethod( Size,
"for a cyclic group",
[ IsGroup and IsCyclic and HasGeneratorsOfGroup ],
[ IsGroup and IsCyclic and HasGeneratorsOfGroup and CanEasilyCompareElements ],
{} -> -RankFilter(HasGeneratorsOfGroup),
function(G)
local gens;
Expand Down
10 changes: 10 additions & 0 deletions tst/testbugfix/2019-07-17-cyclic-size.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Fix a bug that only occurs if all packages are loaded
gap> F:=FreeGroup(2);;
gap> F:=F/[F.1/F.2];;
gap> H:=Group(F.1);;
gap> IsCyclic(H);
true
gap> HasIsFinite(H);
false
gap> Size(H); # should be instant, not infinite recursion / loop
infinity

0 comments on commit 2e9ace1

Please sign in to comment.