forbid implications from non-representations to representations

Filters of type representation are intended to describe
the internal data of an object.
Thus a logical implication to such a filter makes sense only
if the implying filters are also representations.

As had be noticed by @fingolfin,
there was a now forbidden hidden implication in the library,
from `IsHandledByNiceBasis` to `IsAttributeStoringRep`.
Logically, one wants this rather the other way round,
that is, the mechanism relies on stored data and thus should be *restricted*
to situations where `IsAttributeStoringRep` is present.
I have now changed this setup accordingly, that is,
`IsAttributeStoringRep` is no longer implied by `IsHandledByNiceBasis`.
As a consequence, one `Objectify` call in the library had to be changed
such that `IsAttributeStoringRep` gets explicitly set.

(The same might happen also in packages.)

lib/basis.gi

@@ -687,11 +687,14 @@ InstallGlobalFunction( "InstallHandlingByNiceBasis",
     filter:= ValueGlobal( name );
 
     # Install the detection of the filter.
+    # The mechanism is safe only if the domain can store
+    # its nice variant, thus we will install it only for cases where
+    # 'IsAttributeStoringRep' is guaranteed.
     entry:= First( NiceBasisFiltersInfo,
                    x -> IsIdenticalObj( filter, x[1] ) );
     entry[3] := record.detect;
+    filter:= filter and IsFreeLeftModule and IsAttributeStoringRep;
     InstallTrueMethod( IsHandledByNiceBasis, filter );
-    filter:= IsFreeLeftModule and filter;
 
     # Install the methods.
     InstallMethod( NiceFreeLeftModuleInfo,

lib/filter.g

@@ -294,6 +294,21 @@ end);
 ##
 
 BIND_GLOBAL( "InstallTrueMethod", function ( tofilt, from )
+    local i;
+
+    # Check whether 'tofilt' involves or implies representations.
+    for i in TRUES_FLAGS( WITH_IMPS_FLAGS( FLAGS_FILTER( tofilt ) ) ) do
+      if INFO_FILTERS[i] = FNUM_REP_KERN or INFO_FILTERS[i] = FNUM_REP then
+        # This is allowed only if 'from' consists of representations.
+        for i in TRUES_FLAGS( WITH_IMPS_FLAGS( FLAGS_FILTER( from ) ) ) do
+          if not ( INFO_FILTERS[i] = FNUM_REP_KERN or
+                   INFO_FILTERS[i] = FNUM_REP ) then
+            Error( "<tofilt> must not involve representation filters" );
+          fi;
+        od;
+        break;
+      fi;
+    od;
 
     InstallTrueMethodNewFilter( tofilt, from );
 

lib/module.gd

@@ -247,9 +247,7 @@ DeclareProperty( "IsFullMatrixModule", IsFreeLeftModule, 20 );
 ##  </ManSection>
 ##  <#/GAPDoc>
 ##
-DeclareCategory( "IsHandledByNiceBasis",
-    IsFreeLeftModule and IsAttributeStoringRep );
-#T individually choose for each repres. in this category?
+DeclareCategory( "IsHandledByNiceBasis", IsFreeLeftModule );
 #T why not `DeclareFilter' ?
 
 

lib/vspchom.gi

@@ -2169,7 +2169,8 @@ InstallMethod( Hom,
                                 IsFreeLeftModule
                             and IsFullHomModule
                             and IsLinearMappingsModule
-                            and IsGeneralMappingCollection ),
+                            and IsGeneralMappingCollection
+                            and IsAttributeStoringRep ),
                    rec() );
 
     SetLeftActingDomain( M, F );

tst/testinstall/method.tst

@@ -9,6 +9,12 @@ Error, required filters [ "IsInt", "IsRat", "IsCyc", "IsExtAElement",
     , "IsMultiplicativeElementWithInverse", "IsZDFRE", "IsAssociativeElement",
   "IsAdditivelyCommutativeElement", "IsCommutativeElement", "IsCyclotomic" ]
 for 1st argument do not match a declaration of Size
+gap> InstallTrueMethod( IsInternalRep, IsList );
+Error, <tofilt> must not involve new representation filters
+gap> InstallTrueMethod( IsDenseCoeffVectorRep, IsList );
+Error, <tofilt> must not involve new representation filters
+gap> InstallTrueMethod( IsInternalRep, IsSmallIntRep );
+gap> InstallTrueMethod( IsList and IsInternalRep, IsList and IsSmallIntRep );
 
 # Check names are set correctly
 gap> cheese := NewOperation("cheese", [IsObject]);
@@ -27,4 +33,4 @@ gap> List(Concatenation(funcs, ranks), NameFunction);
 [ "cheese method", "cheese for a list", "func3", 
   "Priority calculation for cheese", 
   "Priority calculation for cheese for a list", "rank3" ]
-gap> STOP_TEST("method.tst", 1);
+gap> STOP_TEST("method.tst");