From 85b558483cec0bd507465b82b2dcd2e802d7f22f Mon Sep 17 00:00:00 2001
From: Max Horn <max@quendi.de>
Date: Wed, 21 Apr 2021 16:50:33 +0200
Subject: [PATCH] Change QuaternionAlgebra to use GET_FROM_SORTED_CACHE

Also ensure that the exact same code is used to produce the
resulting algebra regardless of whether it is created freshly
or from the cache. This revealed a bug: `A.1` only worked
in the former case, not the latter, which could lead to subtly
broken code. Before this patch, the following happened:

    gap> A:= QuaternionAlgebra( [1], 2, 5 );
    <algebra-with-one of dimension 4 over Rationals>
    gap> A.1;
    e
    gap> B:= QuaternionAlgebra( [1], 2, 5 );
    <algebra-with-one of dimension 4 over Rationals>
    gap> B.1;
    Error, illegal access to record component `obj.1'

This is now fixed.

Resolves #2940
---
 lib/algsc.gi | 44 ++++++++------------------------------------
 1 file changed, 8 insertions(+), 36 deletions(-)

diff --git a/lib/algsc.gi b/lib/algsc.gi
index dbab32f80d..5f01eea2f4 100644
--- a/lib/algsc.gi
+++ b/lib/algsc.gi
@@ -689,26 +689,9 @@ InstallAccessToGenerators( IsSCAlgebraObjCollection and IsFullSCAlgebra,
 ##
 #V  QuaternionAlgebraData
 ##
-InstallFlushableValue( QuaternionAlgebraData, [] );
+InstallFlushableValue( QuaternionAlgebraData, [ [], [] ] );
 if IsHPCGAP then
     ShareSpecialObj( QuaternionAlgebraData );
-
-    BindGlobal("StoreQuaternionAlgebraData", function( a, b, F, A )
-      local stored;
-      atomic readwrite QuaternionAlgebraData do
-        stored:= First( QuaternionAlgebraData,
-                        t -> t[1] = a and t[2] = b and
-                             IsIdenticalObj( t[3], FamilyObj( F ) ) );
-        if stored = fail then
-          # Store the data for the next call.
-          Add( QuaternionAlgebraData, MakeImmutable([ a, b, FamilyObj( F ), A ]) );
-        else
-          A:= AlgebraWithOne( F, GeneratorsOfAlgebra( stored[4] ), "basis" );
-          SetGeneratorsOfAlgebra( A, GeneratorsOfAlgebraWithOne( A ) );
-        fi;
-      od;
-      return A;
-    end);
 fi;
 
 
@@ -744,16 +727,8 @@ InstallGlobalFunction( QuaternionAlgebra, function( arg )
     fi;
 
     # Generators in the right family may be already available.
-    atomic readonly QuaternionAlgebraData do
-      stored:= First( QuaternionAlgebraData,
-                      t ->     t[1] = a and t[2] = b
-                           and IsIdenticalObj( t[3], FamilyObj( F ) ) );
-    od;
-    if stored <> fail then
-      A:= AlgebraWithOne( F, GeneratorsOfAlgebra( stored[4] ), "basis" );
-      SetGeneratorsOfAlgebra( A, GeneratorsOfAlgebraWithOne( A ) );
-    else
-
+    stored := GET_FROM_SORTED_CACHE( QuaternionAlgebraData, [ a, b, FamilyObj( F ) ],
+    function()
       # Construct a filter describing element properties,
       # which will be stored in the family.
       filter:= IsSCAlgebraObj and IsQuaternion;
@@ -780,15 +755,12 @@ InstallGlobalFunction( QuaternionAlgebra, function( arg )
               filter );
       SetFilterObj( A, IsAlgebraWithOne );
 #T better introduce AlgebraWithOneByStructureConstants?
+      return A;
+    end );
 
-      # Store the data for the next call.
-      if IsHPCGAP then
-        A := StoreQuaternionAlgebraData( a, b, F, A );
-      else
-        Add( QuaternionAlgebraData, [ a, b, FamilyObj( F ), A ] );
-      fi;
-
-    fi;
+    A:= AlgebraWithOne( F, GeneratorsOfAlgebra( stored ), "basis" );
+    SetGeneratorsOfAlgebra( A, GeneratorsOfAlgebraWithOne( A ) );
+    SetIsFullSCAlgebra( A, true );
 
     # A quaternion algebra with negative parameters over a real field
     # is a division ring.