Immutable Object Flag (WIP) #1563
Conversation
markuspf
added
the
do not merge
markuspf
force-pushed
the
immutable-obj-flag
branch
from
c9e06fb
to
2cfa03f
Compare
Even if the names are now more verbose (in particular in the context they are used in) they are also more descriptive.
markuspf
force-pushed
the
immutable-obj-flag
branch
from
2cfa03f
to
b1fc7ad
Compare
Codecov Report
@@ Coverage Diff @@
## master #1563 +/- ##
==========================================
+ Coverage 63.97% 63.98% +<.01%
==========================================
Files 993 993
Lines 325125 325147 +22
Branches 12986 12993 +7
==========================================
+ Hits 208011 208032 +21
+ Misses 114324 114322 -2
- Partials 2790 2793 +3
|
src/objects.c
Outdated
| @@ -809,10 +809,13 @@ void (*MakeImmutableObjFuncs[LAST_REAL_TNUM+1])( Obj ); | |||
| void MakeImmutable(Obj obj) | |||
| { | |||
| if (IS_MUTABLE_OBJ(obj)) { | |||
| SET_OBJ_FLAG(obj, OBJ_FLAG_IMMUTABLE); | |||
| if (!IS_INTOBJ(obj) && !IS_FFE(obj)) { | |||
There was a problem hiding this comment.
Neither INTOBJ not FFE can ever be mutable, so I'm not sure what this check is for.
There was a problem hiding this comment.
I think this is a consequence of me introducing a bug in IS_MUTABLE_OBJ which meant that SET_OBJ_FLAG would be called on FFEs or INTOBJs. This is something I fixed on the train yesterday (but didn't push yet, since I break HPC-GAP in interesting ways which I want to understand first).
markuspf
force-pushed
the
immutable-obj-flag
branch
from
21bbd5e
to
669e5fd
Compare
|
As also pointed out by @fingolfin on the slack channel there might be a point in introducing a mutability flag. This would mean more changes initially, but it is probably more sane to have immutable objects as a default and having to make things mutable explicitly. I'll have a look at the required changes. |
| { | ||
| if(IS_INTOBJ(obj) || IS_FFE(obj)) { | ||
| return 0; | ||
| } else if (TEST_OBJ_FLAG(obj, OBJ_FLAG_IMMUTABLE) == OBJ_FLAG_IMMUTABLE) { |
There was a problem hiding this comment.
Shouldn't this be more like
} else if (FIRST_IMM_MUT_TNUM <= type && type <= LAST_IMM_MUT_TNUM) {
return TEST_OBJ_FLAG(obj, OBJ_FLAG_IMMUTABLE) != OBJ_FLAG_IMMUTABLE;
} else {There was a problem hiding this comment.
That depends on whether you want the immutable flag to only be valid for those TNUMs I suppose?
Admittedly it makes little sense to start allowing making permutations mutable...
There was a problem hiding this comment.
As they are set up OBJ_FLAGS are universal -- the meaning or value of a flag is not bound to specific TNUMs for which it makes sense.
So it would seem logical to make sure each flag is set appropriately even for those TNUMs where there is only one appropriate setting. So all permutations would have the immutable flag, for instance (or not have the mutable flag). Unless this creates some run-time overhead I haven't thought of, then it seems simpler. The only problem would be if we ran out of flag bits.
That leaves T_INTOBJ and T_FFE. I think the nicest way to handle those (and the most consistent with what is done for those objects elsewhere) is to distinguish BAG_FLAGS from OBJ_FLAGS and deal with the two immediate types in the OBJ_FLAGS code. So Gasman never has to worry about those types and higher level code sees flags consistently on every object.
|
Another idea: how about adjusting |
|
Never going to be finished, anyway. |
This PR a work-in-progress to introduce an object flag
OBJ_FLAG_IMMUTABLEinstead of usingTNUMs to determine mutability of objects.This will make #1520 obsolete.