kernel: replace COPYING tnums by single T_COPYING

[fingolfin]

This removes a lot of duplicate code, and makes it simpler to add further copying functions in the future.

Point in case: the semigroups package (the only to implement a non-trivial CopyObjFuncs entry) gets it wrong (paging @james-d-mitchell ), as it does not use a forwarding pointer approach at al. That is, it violates this invariant:

gap> x := [];; # imagine a T_SEMI here, I have no idea how to create one
gap> y := [ x, x ];;
gap> IsIdenticalObj(y[1], y[2]);
true
gap> z := StructuralCopy(y);
[ [  ], [  ] ]
gap> IsIdenticalObj(y[1], z[1]);
false
gap> IsIdenticalObj(z[1], z[2]);  # <- I predict this will return false if x is a T_SEMI
true

Now, I am not blaming anybody here: the copying logic in the GAP kernel simply is not that easy to understand. In fact, we made the same mistake in the SingularInterface package. With the changes in this PR, getting it right will be much simpler

I have incremented GAP_KERNEL_MAJOR_VERSION to detect the new code in the semigroups code. Among the few other packages implementing CopyObjFuncs, only SingularInterface needs a fix.

In addition, this is a first small step towards reducing the differences between the "traditional" copying code, and the HPC-GAP copying code; my hope is that we can eventually share a lot of the code to implement them.

This should not be merged before we branch stable-4.10, it's too risky.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@5c02f3c). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master    #2777   +/-   ##
=========================================
  Coverage          ?   75.87%           
=========================================
  Files             ?      481           
  Lines             ?   241174           
  Branches          ?        0           
=========================================
  Hits              ?   182982           
  Misses            ?    58192           
  Partials          ?        0

Impacted Files	Coverage Δ
src/objects.h	100% <ø> (ø)
src/gap.c	85.11% <ø> (ø)
src/precord.h	96.66% <ø> (ø)
src/modules.h	100% <ø> (ø)
src/blister.c	94.89% <100%> (ø)
src/stringobj.c	93.81% <100%> (ø)
src/weakptr.c	99.2% <100%> (ø)
src/objects.c	80.47% <100%> (ø)
src/plist.c	93.84% <100%> (ø)
src/range.c	94.62% <100%> (ø)
... and 1 more

[ChrisJefferson]

Just out of interest, what will happen when / if T_COPYING get out into general GAP sessions? (I imagine that could happen if the memory limit is reached during copying)

[fingolfin]

Good question, I'll try to answer this on multiple levels:

First off, I don't think the "memory limit" being reached during copying is an issue: either we double the heap, and resume; or else GAP aborts. End of story. In GAP, NewBag always must succeed, no GAP code is prepared for it to fail, ever.

Next, the effect of a T_COPYING escaping will be equivalent to the effect of, say, a T_PLIST+COPYING object escaping right now. I was quite careful about that, e.g. I kept T_COPYING after LAST_REAL_TNUM, just like the +COPYING tnums right now are. So, in this regard, this PR will not make things worse than they are -- IMHO, to the contrary, because it reduces the numbers of copying TNUMs from many dozens to a single.

But now suppose it does escape. We are not really prepared to that (just like we are not prepared for a T_BODY escaping. The main problem will be function tables, like TypeObjFuncs, which actually only provide space up to LAST_REAL_TNUM -- so an out-of-bounds access would happen if somebody tries to get the type of a T_COPYING (right now, it'd be silent, but we could add a range assertion to TYPE_OBJ etc.. Probably a good idea anyway).

Of course we could address this: e.g. by moving T_COPYING a few lines up, to be inside the list of "real" tnums. Then it'd be handled by the default handlers we set in function tables; to stay with the TYPE_OBJ example, trying to get the type would end up calling TypeObjError. So, all in all, that would probably be the easiest "fix", and I see nothing speaking against it. The main reason I didn't do it in this PR is that I felt it already contained a rather big change, and (as described above) by leaving T_COPYING outside the range of "real" TNUMs, I minimized the semantical/behavioural changes in this PR. We can change that in a future PR, though.

[ChrisJefferson]

Thanks for the extensive feedback.

I forgot GAP's "memory is full, type return to continue" messages aren't generated until you next execute a GAP bytecode instruction, so they won't interrupt copying.