fix has_gens for certain matrix groups

[ThomasBreuer]

Be careful if the matrix group is not suported by GAP.

resolves #5661

[lgoettgens]

This kind of violates against the groups conformance tests (cf https://github.com/Nemocas/AbstractAlgebra.jl/blob/fccef26d4c7d226ed8a141d5dbe55e9c415b0b3e/ext/TestExt/Groups-conformance-tests.jl#L61-L73) as there it is tested that gens throws if has_gens returns false. For GL(2, 3) that would not be the case

[fingolfin]

IMHO, the group conformance tests are simply wrong and need to be adjusted. This part has to be removed:

          else
              # TODO: throw something more specific
              @test_throws ErrorException gens(G)
              @test_throws ErrorException ngens(G)

Because it contradicts the has_gens documentation, which says:

Note that the result of this function when applied to a specific group instance can change over time, as side
effect of a generating set becoming available for the group.

In particular, one of the function which may have this side effect is gens, another is ngens.

This is not to say we shouldn't also adjust this PR, though.

[fingolfin]

See Nemocas/AbstractAlgebra.jl#2275

[lgoettgens]

Suggested change

	function has_gens(G::MatrixGroup)
	isdefined(G,:gens) && return true
	isdefined(G, :X) || return false
	return GAP.Globals.HasGeneratorsOfGroup(GapObj(G))::Bool
	end
	function has_gens(G::MatrixGroup)
	isdefined(G, :gens) && return true
	F = codomain(_ring_iso(G))
	GAP.Globals.IsBaseRingSupportedForClassicalMatrixGroup(F, GapObj(G.descr)) || return false
	return GAP.Globals.HasGeneratorsOfGroup(GapObj(G))::Bool
	end

what about something like this?

[ThomasBreuer]

This kind of violates against the groups conformance tests [...] as there it is tested that gens throws if has_gens returns false.

Hmm, all has_something functions that are related to GAP objects have the meaning "is the something value already stored".
The value may change in the sense that first no something is stored and later it gets stored, typically by explicitly calling something.

There are two docstrings for has_gens(::Group), and both of them seem to fit to this interpretation.

[ThomasBreuer]

@lgoettgens @fingolfin

The question seems to be what we want:

If has_gens shall express that "calling gens(G) will be successful (if and) only if has_gens(G) returns true" then we will have to change the documentation and the implementation.

But is this really what we want?
What about situations with finitely presented groups where certain subgroups initially do not know generators?
Asking GAP for generators of the subgroup will cause a coset enumeration --if this is successful then generators can be constructed.
Here is an example.

gap> F:= FreeGroup( 2 );;
gap> G:= F / [ F.1^2, F.2^3, (F.1*F.2)^6 ];
<fp group on the generators [ f1, f2 ]>
gap> U:= DerivedSubgroup( G );
Group(<fp, no generators known>)
gap> HasGeneratorsOfGroup( U );
false
gap> GeneratorsOfGroup( U );
[ f2*f1*f2^-1*f1^-1, f2^-1*f1*f2*f1^-1 ]