Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite and officially document PlainListCopy #4332

Merged
merged 1 commit into from
Mar 25, 2021
Merged

Rewrite and officially document PlainListCopy #4332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/ref/lists.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,16 @@ handled more efficiently,
for example when one installs a method for a particular operation,
where an argument is required to be a list in a particular representation.

<ManSection>
<Func Name="PlainListCopy" Arg='list'/>
<Description>
This function returns a list equal to its argument, in a plain list
representation (see <Ref Filt="IsPlistRep"/>).
This is intended for use in certain rare situations,
such as before objectifying, or calling some kernel functions.
</Description>
</ManSection>

<#Include Label="IsPlistRep">

</Section>
Expand Down
13 changes: 0 additions & 13 deletions hpcgap/lib/vec8bit.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,19 +231,6 @@ InstallMethod( \+, "for an 8 bit vector of char 2 and a GF2 vector",
fi;
end);

#############################################################################
##
#M `PlainListCopyOp( <vec> )
##
## Make the vector into a plain list (in place)
##

InstallMethod( PlainListCopyOp, "for an 8 bit vector",
true, [IsSmallList and Is8BitVectorRep], 0,
function (v)
PLAIN_VEC8BIT(v);
return v;
end);

#############################################################################
##
Expand Down
26 changes: 0 additions & 26 deletions hpcgap/lib/vecmat.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1767,32 +1767,6 @@ function(f,v)
return Immutable(v);
end);


#############################################################################
##
#M PlainListCopyOp( <v> )
##

InstallMethod( PlainListCopyOp, "for a GF2 vector",
true, [IsGF2VectorRep and IsSmallList ],
0, function( v )
PLAIN_GF2VEC(v);
return v;
end);

#############################################################################
##
#M PlainListCopyOp( <m> )
##

InstallMethod( PlainListCopyOp, "for a GF2 matrix",
true, [IsSmallList and IsGF2MatrixRep ],
0, function( m )
PLAIN_GF2MAT(m);
return m;
end);


#############################################################################
##
#M MultVector( <vl>, <mul>)
Expand Down
35 changes: 0 additions & 35 deletions lib/list.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2371,41 +2371,6 @@ DeclareGlobalFunction( "IntersectionBlist" );
##
DeclareGlobalFunction( "ListWithIdenticalEntries" );


#############################################################################
##
#F PlainListCopy( <list> ) . . . . . . . . make a plain list copy of a list
##
## <ManSection>
## <Func Name="PlainListCopy" Arg='list'/>
##
## <Description>
## This is intended for use in certain rare situations,
## such as before objectifying.
## Normally, <C>ConstantAccessTimeList</C> should be enough.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This exists and was a typo for ConstantTimeAccessList

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, I don't think this sentence was helpful, so I am fine with removing it :-)

## </Description>
## </ManSection>
##
DeclareGlobalFunction("PlainListCopy");


#############################################################################
##
#O PlainListCopyOp( <list> ) . . . . . . . .return a plain version of a list
##
## <ManSection>
## <Oper Name="PlainListCopyOp" Arg='list'/>
##
## <Description>
## This operation returns a list equal to its argument, in a plain list
## representation. This may be the argument converted in place, or
## may be new. It is only intended to be called by <C>PlainListCopy</C>.
## </Description>
## </ManSection>
##
DeclareOperation("PlainListCopyOp", [IsSmallList]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we say here that it is a kernel function, in case people wonder where to find it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I am also fine with merging as-is, just wondering)

Copy link
Contributor Author

@ChrisJefferson ChrisJefferson Mar 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now moved the documentation inline in the doc XML, because it was weird here -- but maybe that's a bad idea too. We could start scanning C code for GAPDoc?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In doc/ref/makedocreldata.g we already list src/sysfiles.c and could add more. I would in fact suggest that we change that file to not hardcode the list of files to scan, but rather to scan all files src/*.{c,h}, lib/*.{gd,gi}, etc., but of course that goes way beyond the scope of this PR ;-)



#############################################################################
##
#O PositionNot( <list>, <val>[, <from>] ) . . . . . . . . . find not <val>
Expand Down
33 changes: 0 additions & 33 deletions lib/list.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3890,39 +3890,6 @@ InstallMethod( ViewObj,
Print( " ]" );
end );


#############################################################################
##
#F PlainListCopy( <list> ) . . . . . . . . . . make a plain list copy of
## a list
##
## This is intended for use in certain rare situations, such as before
## Objectifying. Normally, ConstantAccessTimeList should be enough
##
## This function guarantees that the result will be a plain list, distinct
## from the input object.
##
InstallGlobalFunction(PlainListCopy, function( list )
local tnum, copy;

if not IsSmallList( list ) then
Error("PlainListCopy: argument must be a small list");
fi;

# This is enough much of the time
copy := ShallowCopy(list);

# now do a cheap check on copy
tnum := TNUM_OBJ(copy);
if FIRST_LIST_TNUM > tnum or LAST_LIST_TNUM < tnum then
copy := PlainListCopyOp( copy );
fi;
Assert(2, not IsIdenticalObj(list,copy));
Assert(2, TNUM_OBJ(copy) >= FIRST_LIST_TNUM);
Assert(2, TNUM_OBJ(copy) <= LAST_LIST_TNUM);
return copy;
end);

#############################################################################
##
#M PositionNot( <list>, <obj>, <from-minus-one> ) . . . . . . default method
Expand Down
15 changes: 0 additions & 15 deletions lib/mat8bit.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,21 +241,6 @@ InstallMethod( \-, "for two 8 bit matrices in same characteristic",
);


#############################################################################
##
#M `PlainListCopyOp( <mat> )
##
## Make the matrix into a plain list
##

InstallMethod( PlainListCopyOp, "for an 8 bit vector",
true, [IsSmallList and Is8BitMatrixRep], 0,
function (m)
PLAIN_MAT8BIT(m);
return m;
end);


#############################################################################
##
#M ConvertToMatrixRepNC( <list>, <fieldsize )
Expand Down
24 changes: 0 additions & 24 deletions lib/sparselistgen.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,30 +94,6 @@ InstallMethod(ViewObj, "sparse lists", [IsSparseList and IsDenseList],
Print(" >>");
end);

#############################################################################
##
#M PlainListCopyOp( <sl> ) make a copy which will really be a plain list
##

InstallMethod(PlainListCopyOp, "sparse list", [IsSparseList and IsSmallList],
function(sl)
local l, i, ss;
ss := SparseStructureOfList(sl);
if IsBound(ss[1]) then
l := ListWithIdenticalEntries(Length(sl), ss[1]);
else
l := [];
fi;
for i in [1..Length(ss[2])] do
if IsBound(ss[3][i]) then
l[ss[2][i]] := ss[3][i];
else
Unbind(l[ss[2][i]]);
fi;
od;
return l;
end);

#############################################################################
##
#F PLAIN_SL ( <sl> ) convert a sparse list in place to a plain list
Expand Down
14 changes: 0 additions & 14 deletions lib/vec8bit.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,20 +220,6 @@ InstallMethod( \+, "for an 8 bit vector of char 2 and a GF2 vector",
fi;
end);

#############################################################################
##
#M `PlainListCopyOp( <vec> )
##
## Make the vector into a plain list (in place)
##

InstallMethod( PlainListCopyOp, "for an 8 bit vector",
true, [IsSmallList and Is8BitVectorRep], 0,
function (v)
PLAIN_VEC8BIT(v);
return v;
end);

#############################################################################
##
#M DegreeFFE( <vector> )
Expand Down
25 changes: 0 additions & 25 deletions lib/vecmat.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,31 +1579,6 @@ function(f,v)
end);


#############################################################################
##
#M PlainListCopyOp( <v> )
##

InstallMethod( PlainListCopyOp, "for a GF2 vector",
true, [IsGF2VectorRep and IsSmallList ],
0, function( v )
PLAIN_GF2VEC(v);
return v;
end);

#############################################################################
##
#M PlainListCopyOp( <m> )
##

InstallMethod( PlainListCopyOp, "for a GF2 matrix",
true, [IsSmallList and IsGF2MatrixRep ],
0, function( m )
PLAIN_GF2MAT(m);
return m;
end);


#############################################################################
##
#M MultVector( <vl>, <mul>)
Expand Down
7 changes: 7 additions & 0 deletions src/lists.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1590,6 +1590,12 @@ Obj PLAIN_LIST_COPY(Obj list)
return res;
}

Obj FuncPlainListCopy(Obj self, Obj list)
{
RequireSmallList(SELF_NAME, list);
return PLAIN_LIST_COPY(list);
}


/****************************************************************************
**
Expand Down Expand Up @@ -1856,6 +1862,7 @@ static StructGVarFunc GVarFuncs [] = {
GVAR_FUNC_1ARGS(IS_SSORT_LIST_DEFAULT, list),
GVAR_FUNC_1ARGS(IS_POSS_LIST_DEFAULT, list),
GVAR_FUNC_3ARGS(POS_LIST_DEFAULT, list, obj, start),
GVAR_FUNC_1ARGS(PlainListCopy, list),
{ 0, 0, 0, 0, 0 }

};
Expand Down
38 changes: 38 additions & 0 deletions tst/testinstall/list.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,44 @@ gap> l;
gap> TNAM_OBJ(l);
"plain list (rectangular table)"

# Check PlainListCopy
gap> checkPlainListCopy := function(l)
> local copy, tnum;
> tnum := TNUM_OBJ(l);
> copy := PlainListCopy(l);
> return IsPlistRep(copy) and l = copy and
> not IsIdenticalObj(l,copy) and TNUM_OBJ(l) = tnum;
wilfwilson marked this conversation as resolved.
Show resolved Hide resolved
> end;;
gap> checkPlainListCopy([]);
true
gap> checkPlainListCopy([1, ,()]);
true
gap> checkPlainListCopy([1..5]);
true
gap> checkPlainListCopy([10,8..-4]);
true
gap> checkPlainListCopy("");
true
gap> checkPlainListCopy("abc");
true
gap> checkPlainListCopy(ListWithIdenticalEntries(3, false));
ChrisJefferson marked this conversation as resolved.
Show resolved Hide resolved
true
ChrisJefferson marked this conversation as resolved.
Show resolved Hide resolved
gap> checkPlainListCopy(NewZeroVector(IsGF2VectorRep, GF(2), 10));
true
gap> checkPlainListCopy(NewZeroVector(Is8BitVectorRep, GF(3), 10));
true
gap> PlainListCopy(6);
Error, PlainListCopy: <list> must be a small list (not the integer 6)
gap> PlainListCopy((1,2,3));
Error, PlainListCopy: <list> must be a small list (not a permutation (small))
#@if IsHPCGAP
gap> PlainListCopy(Group((1,2)));
Error, PlainListCopy: <list> must be a small list (not an atomic component object)
#@else
gap> PlainListCopy(Group((1,2)));
Error, PlainListCopy: <list> must be a small list (not a component object)
#@fi

# Check TNUM behaviours
gap> x := [1,,"cheese"];;
gap> x[2] := 2;;
Expand Down