Don't automatically call attribute setters on mutable objects

doc/ref/types.xml

@@ -485,7 +485,7 @@ except if the attribute had been specially constructed as
 <Q>mutable attribute</Q>.
 <P/>
 It depends on the representation of an object (see&nbsp;<Ref Sect="Representation"/>)
-which attribute values it stores.  An object in the representation
+which attribute values it stores.  An immutable object in the representation
 <C>IsAttributeStoringRep</C> stores <E>all</E> attribute values once they are
 computed.  Moreover, for an object in this representation, subsequent
 calls to an attribute will return the <E>same</E> object; this is achieved
@@ -496,6 +496,13 @@ value.  (These methods are called the <Q>system setter</Q> and the
 <Q>system getter</Q> of the attribute, respectively.)<Index>system
 getter</Index><Index>system setter</Index>
 <P/>
+Mutable objects in <C>IsAttributeStoringRep</C> are allowed, but
+attribute values are not automatically stored in them. Such objects
+are useful because values can be stored by explicitly calling the
+relevant setter, and because they may later be made immutable using
+<Ref Func="MakeImmutable"/>, at which point they will start storing
+all attribute values.
+<P/>
 Note also that it is impossible to get rid of a stored attribute
 value because the system may have drawn conclusions from the old
 attribute value, and just removing the value might leave the data
@@ -568,7 +575,8 @@ of the object <A>obj</A> is known.
 <Description>
 For an attribute <A>attr</A>, <C>Setter(<A>attr</A>)</C> 
 is called automatically when the attribute value has been
-computed for the first time.
+computed for an immutable object which does not already have a
+value stored for <A>attr</A>.
 One can also call the setter explicitly,
 for example, <C>Setter( Size )( <A>obj</A>, <A>val</A> )</C> 
 sets <A>val</A> as size of the

lib/sgpres.gi

@@ -1081,6 +1081,9 @@ InstallGlobalFunction( PresentationAugmentedCosetTable,
     # group generators.
     SetPrimaryGeneratorWords(T,aug.primaryGeneratorWords);
 
+    # Since T is mutable, we must set this attribite "manually"
+    SetTzOptions(T, TzOptions(T));
+
     # handle relators of length 1 or 2, but do not eliminate any primary
     # generators.
     TzOptions(T).protected := tree[TR_PRIMARY];

lib/tietze.gi

@@ -381,6 +381,9 @@ InstallGlobalFunction( PresentationFpGroup, function ( arg )
     SetOne(T,Identity( F ));
     T!.identity:=Identity( F );
 
+    # since T is mutable, we must set this attribute "manually"
+    SetTzOptions(T,TzOptions(T));    
+
     # initialize some Tietze options
     TzOptions(T).protected := 0;
     TzOptions(T).printLevel:=printlevel;

src/opers.c

@@ -2420,7 +2420,7 @@ Obj DoAttribute (
     val = CopyObj( val, 0 );
 
     /* set the value (but not for internal objects)                        */
-    if ( ENABLED_ATTR( self ) == 1 ) {
+    if ( ENABLED_ATTR( self ) == 1 && !IS_MUTABLE_OBJ( obj ) ) {
         switch ( TNUM_OBJ( obj ) ) {
         case T_COMOBJ:
         case T_POSOBJ:
@@ -2471,7 +2471,7 @@ static Obj DoVerboseAttribute(Obj self, Obj obj)
     val = CopyObj( val, 0 );
 
     /* set the value (but not for internal objects)                        */
-    if ( ENABLED_ATTR( self ) == 1 ) {
+    if ( ENABLED_ATTR( self ) == 1  && !IS_MUTABLE_OBJ( obj ) ) {
         switch ( TNUM_OBJ( obj ) ) {
         case T_COMOBJ:
         case T_POSOBJ:
@@ -2516,7 +2516,7 @@ static Obj DoMutableAttribute(Obj self, Obj obj)
     val = DoOperation1Args( self, obj );
 
     /* set the value (but not for internal objects)                        */
-    if ( ENABLED_ATTR( self ) == 1 ) {
+    if ( ENABLED_ATTR( self ) == 1  && !IS_MUTABLE_OBJ( obj ) ) {
         switch ( TNUM_OBJ( obj ) ) {
         case T_COMOBJ:
         case T_POSOBJ:
@@ -2561,7 +2561,7 @@ static Obj DoVerboseMutableAttribute(Obj self, Obj obj)
     val = DoVerboseOperation1Args( self, obj );
 
     /* set the value (but not for internal objects)                        */
-    if ( ENABLED_ATTR( self ) == 1 ) {
+    if ( ENABLED_ATTR( self ) == 1  && !IS_MUTABLE_OBJ( obj ) ) {
         switch ( TNUM_OBJ( obj ) ) {
         case T_COMOBJ:
         case T_POSOBJ:

tst/testinstall/attribute.tst

@@ -44,6 +44,31 @@ gap> NewAttribute("IsBanana", IsGroup, true, 15);
 <Attribute "IsBanana">
 gap> NewAttribute("IsBanana", IsGroup, "mutable", 15, "Hello, world");
 Error, Usage: NewAttribute( <name>, <filter>[, <mutable>][, <rank>] )
-
-#
+gap> DeclareAttribute("FavouriteFruit", IsObject);
+gap> foo := rec();;
+gap> fam := NewFamily("FruitFamily");
+<Family: "FruitFamily">
+gap> Objectify(NewType(fam, IsMutable and IsAttributeStoringRep), foo);
+<object>
+gap> InstallMethod(FavouriteFruit, [IsObject], x-> "apple");
+gap> FavouriteFruit(foo);
+"apple"
+gap> HasFavouriteFruit(foo);
+false
+gap> SetSize(foo, 17);
+gap> HasSize(foo);
+true
+gap> Size(foo);
+17
+gap> InstallMethod(FavouriteFruit, [HasSize], x-> "pear");
+gap> FavouriteFruit(foo);
+"pear"
+#@if not IsHPCGAP
+gap> MakeImmutable(foo);
+<object>
+gap> FavouriteFruit(foo);
+"pear"
+gap> HasFavouriteFruit(foo);
+true
+#@fi
 gap> STOP_TEST("attribute.tst", 1);