Skip to content

Commit

Permalink
MakeIsPlistVectorRep: remove argument 'mutable'
Browse files Browse the repository at this point in the history 
It is redundant, and incorrect use could lead to
inconsistencies, which we now avoid
  • Loading branch information
@fingolfin
fingolfin committed Jan 20, 2023
1 parent d93569a commit 18e734c
Showing 1 changed file with 23 additions and 33 deletions.
56 changes: 23 additions & 33 deletions lib/matobjplist.gi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
# Constructors:
############################################################################

# TODO: experiment with making `MakeIsPlistVectorRep` an operation???
BindGlobal( "MakeIsPlistVectorRep",
function(basedomain, mutable, list )
function(basedomain, list )
local fam, types, typ;
fam := FamilyObj(basedomain);
#types := _PlistVectorRepTypeCache(basedomain);

# special case: integers
if IsIntegers(basedomain) then
Expand Down Expand Up @@ -65,7 +65,7 @@ BindGlobal( "MakeIsPlistVectorRep",
types := fam!.PlistVectorRepTypes;
fi;
fi;
if mutable then
if IsMutable(list) then
typ := types[2];
else
typ := types[1];
Expand All @@ -80,15 +80,15 @@ InstallMethod( NewVector, "for IsPlistVectorRep, a ring, and a list",
if ValueOption( "check" ) <> false and not IsSubset( basedomain, l ) then
Error( "the elements in <l> must lie in <basedomain>" );
fi;
return MakeIsPlistVectorRep(basedomain, true, ShallowCopy(l));
return MakeIsPlistVectorRep(basedomain, ShallowCopy(l));
end );

InstallMethod( NewZeroVector, "for IsPlistVectorRep, a ring, and an int",
[ IsPlistVectorRep, IsRing, IsInt ],
function( filter, basedomain, len )
local list;
list := ListWithIdenticalEntries(len, Zero(basedomain));
return MakeIsPlistVectorRep(basedomain, true, list);
return MakeIsPlistVectorRep(basedomain, list);
end );

InstallMethod( NewMatrix,
Expand Down Expand Up @@ -256,7 +256,7 @@ InstallMethod( Vector, "for a plain list and a plist vector",
[ IsList and IsPlistRep, IsPlistVectorRep ],
function( l, v )
# wrap the given list without copying it (this is documented behavior)
return MakeIsPlistVectorRep(v![BDPOS], IsMutable(l), l);
return MakeIsPlistVectorRep(v![BDPOS], l);
# TODO: optionally check that l is consistent with t
end );

Expand All @@ -266,7 +266,10 @@ InstallMethod( Vector, "for a list and a plist vector",
local m;
m := IsMutable(l);
l := PlainListCopy(l);
return MakeIsPlistVectorRep(v![BDPOS], m, l);
if not m then
MakeImmutable(l);
fi;
return MakeIsPlistVectorRep(v![BDPOS], l);
end );

# compatibility method for older representations as list of elements
Expand Down Expand Up @@ -296,7 +299,7 @@ InstallMethod( \[\]\:\=, "for a plist vector, a positive integer, and an obj",
InstallMethod( \{\}, "for a plist vector and a list",
[ IsPlistVectorRep, IsList ],
function( v, l )
return MakeIsPlistVectorRep(v![BDPOS], true, v![ELSPOS]{l});
return MakeIsPlistVectorRep(v![BDPOS], v![ELSPOS]{l});
end );

InstallMethod( PositionNonZero, "for a plist vector", [ IsPlistVectorRep ],
Expand Down Expand Up @@ -343,7 +346,7 @@ InstallMethod( Unpack, "for a plist vector",

InstallMethod( ShallowCopy, "for a plist vector", [ IsPlistVectorRep ],
function( v )
return MakeIsPlistVectorRep(v![BDPOS], true, ShallowCopy(v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], ShallowCopy(v![ELSPOS]));
end );

# StructuralCopy works automatically
Expand All @@ -361,19 +364,15 @@ InstallMethod( PostMakeImmutable, "for a plist vector", [ IsPlistVectorRep ],
InstallMethod( \+, "for two plist vectors",
[ IsPlistVectorRep, IsPlistVectorRep ],
function( a, b )
local mutable;
# TODO: check that basedomains are equal or at least "match"?
mutable := IsMutable(a) or IsMutable(b);
return MakeIsPlistVectorRep(a![BDPOS], mutable, SUM_LIST_LIST_DEFAULT(a![ELSPOS],b![ELSPOS]));
return MakeIsPlistVectorRep(a![BDPOS], SUM_LIST_LIST_DEFAULT(a![ELSPOS],b![ELSPOS]));
end );

InstallMethod( \-, "for two plist vectors",
[ IsPlistVectorRep, IsPlistVectorRep ],
function( a, b )
local mutable;
# TODO: check that basedomains are equal or at least "match"?
mutable := IsMutable(a) or IsMutable(b);
return MakeIsPlistVectorRep(a![BDPOS], mutable, DIFF_LIST_LIST_DEFAULT(a![ELSPOS],b![ELSPOS]));
return MakeIsPlistVectorRep(a![BDPOS], DIFF_LIST_LIST_DEFAULT(a![ELSPOS],b![ELSPOS]));
end );

InstallMethod( \=, "for two plist vectors",
Expand Down Expand Up @@ -467,61 +466,52 @@ InstallOtherMethod( MultVectorLeft, "for an integer vector, and a small integer"
InstallMethod( \*, "for a plist vector and a scalar",
[ IsPlistVectorRep, IsScalar ],
function( v, s )
return MakeIsPlistVectorRep(v![BDPOS], IsMutable(v),
PROD_LIST_SCL_DEFAULT(v![ELSPOS],s));
return MakeIsPlistVectorRep(v![BDPOS], PROD_LIST_SCL_DEFAULT(v![ELSPOS],s));
end );

InstallMethod( \*, "for a scalar and a plist vector",
[ IsScalar, IsPlistVectorRep ],
function( s, v )
return MakeIsPlistVectorRep(v![BDPOS], IsMutable(v),
PROD_SCL_LIST_DEFAULT(s,v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], PROD_SCL_LIST_DEFAULT(s,v![ELSPOS]));
end );

InstallMethod( \/, "for a plist vector and a scalar",
[ IsPlistVectorRep, IsScalar ],
function( v, s )
return MakeIsPlistVectorRep(v![BDPOS], IsMutable(v),
PROD_LIST_SCL_DEFAULT(v![ELSPOS],s^-1));
return MakeIsPlistVectorRep(v![BDPOS], PROD_LIST_SCL_DEFAULT(v![ELSPOS],s^-1));
end );

InstallMethod( AdditiveInverseSameMutability, "for a plist vector",
[ IsPlistVectorRep ],
function( v )
return MakeIsPlistVectorRep(v![BDPOS], IsMutable(v),
AdditiveInverseSameMutability(v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], AdditiveInverseSameMutability(v![ELSPOS]));
end );

InstallMethod( AdditiveInverseImmutable, "for a plist vector",
[ IsPlistVectorRep ],
function( v )
return MakeIsPlistVectorRep(v![BDPOS], false,
AdditiveInverseImmutable(v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], AdditiveInverseImmutable(v![ELSPOS]));
end );

InstallMethod( AdditiveInverseMutable, "for a plist vector",
[ IsPlistVectorRep ],
function( v )
return MakeIsPlistVectorRep(v![BDPOS], true,
AdditiveInverseMutable(v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], AdditiveInverseMutable(v![ELSPOS]));
end );

InstallMethod( ZeroSameMutability, "for a plist vector", [ IsPlistVectorRep ],
function( v )
return MakeIsPlistVectorRep(v![BDPOS], IsMutable(v),
ZeroSameMutability(v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], ZeroSameMutability(v![ELSPOS]));
end );

InstallMethod( ZeroImmutable, "for a plist vector", [ IsPlistVectorRep ],
function( v )
return MakeIsPlistVectorRep(v![BDPOS], false,
ZeroImmutable(v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], ZeroImmutable(v![ELSPOS]));
end );

InstallMethod( ZeroMutable, "for a plist vector", [ IsPlistVectorRep ],
function( v )
return MakeIsPlistVectorRep(v![BDPOS], true,
ZeroMutable(v![ELSPOS]));
return MakeIsPlistVectorRep(v![BDPOS], ZeroMutable(v![ELSPOS]));
end );

InstallMethod( IsZero, "for a plist vector", [ IsPlistVectorRep ],
Expand Down

0 comments on commit 18e734c

Please sign in to comment.