Skip to content

Commit

Permalink
kernel: add PLAIN_LIST_COPY helper
Browse files Browse the repository at this point in the history
The function PLAIN_LIST converts any other type of internal lists (blists,
strings, ranges) into plain lists. This is a powerful function that can lead
to very surprising and problematic results, hence it would be good to minimize
or even remove its usage.

It turns out that in quite some places, we first make a shallow copy of a list
and then apply PLAIN_LIST to that. So here we introduce a new helper which
combines the two tasks into one and avoids the in-place conversion altogether.
This let's the remaining uses of PLAIN_LIST in the kernel stand out more.
  • Loading branch information
fingolfin committed Oct 7, 2019
1 parent cd9d598 commit 7a7f72b
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 28 deletions.
3 changes: 1 addition & 2 deletions src/error.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,7 @@ Obj CALL_WITH_CATCH(Obj func, volatile Obj args)
RequireArgument("CALL_WITH_CATCH", args, "must be a list");
#ifdef HPCGAP
if (!IS_PLIST(args)) {
args = SHALLOW_COPY_OBJ(args);
PLAIN_LIST(args);
args = PLAIN_LIST_COPY(args);
}
#endif

Expand Down
17 changes: 17 additions & 0 deletions src/lists.c
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,23 @@ static void PlainListError(Obj list)
(Int)TNAM_OBJ(list), 0L );
}

Obj PLAIN_LIST_COPY(Obj list)
{
if (IS_PLIST(list)) {
return SHALLOW_COPY_OBJ(list);
}
const Int len = LEN_LIST(list);
if (len == 0)
return NewEmptyPlist();
Obj res = NEW_PLIST(T_PLIST, len);
SET_LEN_PLIST(res, len);
for (Int i = 1; i <= len; i++) {
SET_ELM_PLIST(res, i, ELMV0_LIST(list, i));
CHANGED_BAG(res);
}
return res;
}


/****************************************************************************
**
Expand Down
7 changes: 7 additions & 0 deletions src/lists.h
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,13 @@ EXPORT_INLINE void PLAIN_LIST(Obj list)
}


/****************************************************************************
**
*F PLAIN_LIST_COPY(<list>) . . . . . . . copy a list to a mutable plain list
*/
Obj PLAIN_LIST_COPY(Obj list);


/****************************************************************************
**
*F TYPES_LIST_FAM(<fam>) . . . . . . . list of types of lists over a family
Expand Down
48 changes: 22 additions & 26 deletions src/trans.cc
Original file line number Diff line number Diff line change
Expand Up @@ -513,15 +513,8 @@ static Obj FuncTRANS_IMG_KER_NC(Obj self, Obj img, Obj ker)
UInt4 * ptf4;
UInt i, pos, deg;

copy_img = SHALLOW_COPY_OBJ(img);
copy_ker = SHALLOW_COPY_OBJ(ker);

if (!IS_PLIST(copy_img)) {
PLAIN_LIST(copy_img);
}
if (!IS_PLIST(copy_ker)) {
PLAIN_LIST(copy_ker);
}
copy_img = PLAIN_LIST_COPY(img);
copy_ker = PLAIN_LIST_COPY(ker);
MakeImmutableNoRecurse(copy_img);
MakeImmutableNoRecurse(copy_ker);

Expand Down Expand Up @@ -567,15 +560,8 @@ static Obj FuncIDEM_IMG_KER_NC(Obj self, Obj img, Obj ker)
UInt4 * ptf4, *pttmp;
UInt i, j, deg, rank;

copy_img = SHALLOW_COPY_OBJ(img);
copy_ker = SHALLOW_COPY_OBJ(ker);

if (!IS_PLIST(copy_img)) {
PLAIN_LIST(copy_img);
}
if (!IS_PLIST(copy_ker)) {
PLAIN_LIST(copy_ker);
}
copy_img = PLAIN_LIST_COPY(img);
copy_ker = PLAIN_LIST_COPY(ker);
MakeImmutableNoRecurse(copy_img);
MakeImmutableNoRecurse(copy_ker);

Expand Down Expand Up @@ -3619,20 +3605,28 @@ static Obj FuncOnPosIntSetsTrans(Obj self, Obj set, Obj f, Obj n)
Obj * ptres, res;
UInt i, k;

if (LEN_LIST(set) == 0) {
const UInt len = LEN_LIST(set);

if (len == 0) {
return set;
}

if (LEN_LIST(set) == 1 && INT_INTOBJ(ELM_LIST(set, 1)) == 0) {
if (len == 1 && INT_INTOBJ(ELM_LIST(set, 1)) == 0) {
return FuncIMAGE_SET_TRANS_INT(self, f, n);
}

PLAIN_LIST(set);

const UInt len = LEN_PLIST(set);

res = NEW_PLIST_WITH_MUTABILITY(IS_PLIST_MUTABLE(set), T_PLIST_CYC_SSORT, len);
SET_LEN_PLIST(res, len);
if (IS_PLIST(set)) {
res = NEW_PLIST_WITH_MUTABILITY(IS_PLIST_MUTABLE(set), T_PLIST_CYC_SSORT, len);
SET_LEN_PLIST(res, len);
}
else {
// input is not a plain list, so we make a copy of it, and then also reuse
// that copy for our output
res = PLAIN_LIST_COPY(set);
if (!IS_MUTABLE_OBJ(set))
MakeImmutableNoRecurse(res);
set = res;
}

ptset = CONST_ADDR_OBJ(set) + len;
ptres = ADDR_OBJ(res) + len;
Expand All @@ -3649,6 +3643,7 @@ static Obj FuncOnPosIntSetsTrans(Obj self, Obj set, Obj f, Obj n)
}
SortPlistByRawObj(res);
REMOVE_DUPS_PLIST_INTOBJ(res);
RetypeBagSM(res, T_PLIST_CYC_SSORT);
return res;
}
else if (TNUM_OBJ(f) == T_TRANS4) {
Expand All @@ -3663,6 +3658,7 @@ static Obj FuncOnPosIntSetsTrans(Obj self, Obj set, Obj f, Obj n)
}
SortPlistByRawObj(res);
REMOVE_DUPS_PLIST_INTOBJ(res);
RetypeBagSM(res, T_PLIST_CYC_SSORT);
return res;
}
RequireTransformation("OnPosIntSetsTrans", f);
Expand Down

0 comments on commit 7a7f72b

Please sign in to comment.