Gap syntaxtrees

[fingolfin]

@sebasguts is this up-to-date with your changes from yesterday?

I also had some more thoughts on the whole business last night we should discuss. One is that I think that we should never output "T_SEQ_STAT2", "T_SEQ_STAT3" etc., but only "T_SEQ_STAT"; then then when reassembling pieces, we should automatically use the appropriate variant. The same holds for the various 0ARGS, 1ARGS etc. calls, the loops and anything else involving these counts. Reason: While it of course makes it easier for us to produce and expect these tokens, it makes it much more complicated to manipulate these ASTs.

I've also been thinking about not produce T_SEQ_STAT at all, instead, it could just produce a list of statements. That would result in outputs that are easier to read for humans. And in a sense, T_SEQ_STAT is really an internal concept where I think it would make sense to abstract it away... I am not completely sure whether this is a good or bad idea, though... Anyway, it'd be easy to implement after applying the following patch:

@@ -99,9 +101,7 @@ static Obj SyntaxTreeCompiler(Expr expr)

     result = NewSyntaxTreeNode(comp.name);

-    comp.compile(result, expr);
-
-    return result;
+    return comp.compile(result, expr);
 }

 static Obj SyntaxTreeArgcompInt(UInt i)

With that, we can just throw away the record in the comp.compile function and instead return a list.

I also really want to come up with uniform terminology: As explained before, I dislike the current usage of "compile" for turning the built-in T_BODY (which also is a flattened AST) into a GAP manageable precord-of-precords-and-lists AST. If at all, I'd expect that "compile" would be the reverse direction. In this PR, you used "code" for the reverse direction, which suggest that we rename "compiler" to "decode". I could live with that, we should run it by @markuspf, too, though.

There were more things, but let's not take too many steps at once... :-)

[sebasguts]

@sebasguts is this up-to-date with your changes from yesterday?

Now it is

I also had some more thoughts on the whole business last night we should discuss. One is that I think that we should never output "T_SEQ_STAT2", "T_SEQ_STAT3" etc., but only "T_SEQ_STAT"; then then when reassembling pieces, we should automatically use the appropriate variant. The same holds for the various 0ARGS, 1ARGS etc. calls, the loops and anything else involving these counts. Reason: While it of course makes it easier for us to produce and expect these tokens, it makes it much more complicated to manipulate these ASTs.

Indeed, we can do that, with a fairly easy change I think. The interesting question is on how the read in function performs compared to the "regularly" typed in function, as it might be slower.

I also really want to come up with uniform terminology: As explained before, I dislike the current usage of "compile" for turning the built-in T_BODY (which also is a flattened AST) into a GAP manageable precord-of-precords-and-lists AST. If at all, I'd expect that "compile" would be the reverse direction. In this PR, you used "code" for the reverse direction, which suggest that we rename "compiler" to "decode". I could live with that, we should run it by @markuspf, too, though.

That is fine by me.

[sebasguts]

Should I make this a PR to gap-system, so we can continue discussing there? Or split this up?

[sebasguts]

I rebased the branch and added the float coders.

[codecov[bot]]

Codecov Report

Merging #114 into mh/syntaxtree will decrease coverage by 7.41%. The diff coverage is 15.89%.

@@                Coverage Diff                @@
##           mh/syntaxtree     #114      +/-   ##
=================================================
- Coverage          69.12%   61.71%   -7.42%     
=================================================
  Files                633      633              
  Lines             305868   306032     +164     
=================================================
- Hits              211443   188871   -22572     
- Misses             94425   117161   +22736

Impacted Files	Coverage Δ
src/code.h	86.11% <ø> (ø)	:arrow_up:
src/syntaxtree.c	4.94% <0%> (-6.53%)	:arrow_down:
src/code.c	90.66% <100%> (+0.04%)	:arrow_up:
src/intrprtr.c	70.72% <100%> (+1.14%)	:arrow_up:
lib/meatauto.gi	5.99% <0%> (-89.52%)	:arrow_down:
lib/nilpquot.gi	11.11% <0%> (-88.89%)	:arrow_down:
lib/grpcompl.gi	4.54% <0%> (-81.07%)	:arrow_down:
lib/reesmat.gi	27.14% <0%> (-72.04%)	:arrow_down:
lib/randiso2.gi	2.57% <0%> (-67.65%)	:arrow_down:
lib/filter.gi	33.33% <0%> (-66.67%)	:arrow_down:
... and 286 more

[codecov-io]

Codecov Report

Merging #114 into master will decrease coverage by 27.45%. The diff coverage is 10.17%.

@@             Coverage Diff             @@
##           master     #114       +/-   ##
===========================================
- Coverage   69.17%   41.72%   -27.46%     
===========================================
  Files         633       99      -534     
  Lines      305868    40161   -265707     
===========================================
- Hits       211593    16756   -194837     
+ Misses      94275    23405    -70870

Impacted Files	Coverage Δ
src/code.h	86.11% <ø> (ø)	:arrow_up:
src/code.c	90.65% <100%> (+0.03%)	:arrow_up:
src/intrprtr.c	69.57% <100%> (-1.15%)	:arrow_down:
src/syntaxtree.c	6.34% <2.59%> (-5.14%)	:arrow_down:
src/system.c	44.9% <0%> (-10.19%)	:arrow_down:
src/modules.c	49.77% <0%> (-5.48%)	:arrow_down:
src/io.c	41.58% <0%> (-5.03%)	:arrow_down:
src/vars.h	80% <0%> (-3.34%)	:arrow_down:
src/objects.c	50.99% <0%> (-3.16%)	:arrow_down:
src/gap.c	41.68% <0%> (-2.11%)	:arrow_down:
... and 542 more