From cecd48d58c99f7ca5dd15c1c096a21efee141df7 Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Wed, 21 Mar 2018 13:53:08 +0100
Subject: [PATCH] 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.
---
 lib/grp.gi                                      | 10 +++++++---
 .../2018-03-21-IsFinitelyGeneratedGroup.tst     | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)
 create mode 100644 tst/testbugfix/2018-03-21-IsFinitelyGeneratedGroup.tst

diff --git a/lib/grp.gi b/lib/grp.gi
index 2324202bff..4d2412025b 100644
--- 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
new file mode 100644
index 0000000000..5d7d61c0e2
--- /dev/null
+++ 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