Fix bug in IsFinitelyGeneratedGroup method

There was an incorrect method for IsFinitelyGeneratedGroup which assumed that a group given by an infinite generating set is not finitely generated, which is of course false: any finitely generated infinite group is generated by its set of elements.
gap-system · Mar 21, 2018 · cecd48d · cecd48d
1 parent f26f6e3
commit cecd48d
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git a/lib/grp.gi b/lib/grp.gi
@@ -18,9 +18,13 @@
 #M  IsFinitelyGeneratedGroup( <G> ) . . test if a group is finitely generated
 ##
 InstallImmediateMethod( IsFinitelyGeneratedGroup,
-    IsGroup and HasGeneratorsOfGroup, 0,
-    G -> IsFinite( GeneratorsOfGroup( G ) ) );
-
+    IsGroup and HasGeneratorsOfGroup,
+    function( G )
+    if IsFinite( GeneratorsOfGroup( G ) ) then
+      return true;
+    fi;
+    TryNextMethod();
+    end );
 
 #############################################################################
 ##

diff --git a/tst/testbugfix/2018-03-21-IsFinitelyGeneratedGroup.tst b/tst/testbugfix/2018-03-21-IsFinitelyGeneratedGroup.tst
@@ -0,0 +1,17 @@
+# There was an incorrect method for IsFinitelyGeneratedGroup which assumed
+# that a group given by an infinite generating set is not finitely generated,
+# which is of course false: any finitely generated infinite group is generated
+# by its set of elements.
+#
+# Test that this is not the case anymore:
+gap> F:=FreeGroup(1);;
+gap> G:=SubgroupShell(F);
+Group(<free, no generators known>)
+gap> SetGeneratorsOfGroup(G, Enumerator(F));
+
+# We are defensive: GAP should either not know that G is finitely generated
+# (that's the situation right now), or, if it ever is improved to handle this
+# case, it should correctly determine that G = F is finitely generated (even
+# cyclic)
+gap> not HasIsFinitelyGeneratedGroup(G) or IsFinitelyGeneratedGroup(G);
+true