Add functions to convert GAP functions into syntax trees

[markuspf]

Note the incomplete code for compiling syntax trees has been removed, and will be added (completed) in a later PR.

This PR adds a function to convert a GAP function into a GAP-level syntax tree, it also contains some (untested, possibly still incomplete) code to compile such a syntax tree into a "cleaned up" syntax tree, and to pretty print functions.

Once this is merged the next step is to add a function that compiles a syntax tree back into GAP's internal binary representation. I have prototype code for such a function but it doesn't work reliably yet.

Note that this code predates quite a few changes made to the internal representation of GAP code. This PR contains a test that compiles all globally findable functions into a syntax tree, but this test passing of course only means that the syntax tree code does not crash: As soon as I have the coding function working, we can test rountrips (and compare the binary representations of functions).

TODO:

• Check for completeness (Floats are not supported at the moment)
• Check that code for LVars, HVars is still valid after @fingolfin's changes
• Possibly other things.

[fingolfin]

Huh, what LVars/HVars changes did I make that could affect this? I am not aware of having changed anything related to that, other than inside the reader (which does not affect the internal representation). Am I forgetting something?

[markuspf]

I think I was thinking of the interpreter code, which is of course not relevant for this PR.

I am getting a bit irritated by the ward failure now...

[fingolfin]

Now that ward has been updated, I restarted the HPC-GAP Travis job for this PR, let's see if it passes now.

[codecov]

Codecov Report

Merging #2628 into master will increase coverage by 0.35%.
The diff coverage is 97.76%.

@@            Coverage Diff             @@
##           master    #2628      +/-   ##
==========================================
+ Coverage   83.81%   84.17%   +0.35%     
==========================================
  Files         685      635      -50     
  Lines      343206   317687   -25519     
==========================================
- Hits       287663   267413   -20250     
+ Misses      55543    50274    -5269

Impacted Files	Coverage Δ
lib/syntaxtree.gd	100% <100%> (ø)
lib/read8.g	100% <100%> (ø)	⬆️
lib/syntaxtree.gi	100% <100%> (ø)
src/syntaxtree.c	97.66% <97.66%> (ø)
lib/random.g	66.66% <0%> (-30.56%)	⬇️
lib/read2.g	85.71% <0%> (-14.29%)	⬇️
lib/read1.g	85.36% <0%> (-13.42%)	⬇️
lib/random.gd	91.3% <0%> (-8.7%)	⬇️
lib/methsel2.g	42.66% <0%> (-8%)	⬇️
lib/package.gd	93.47% <0%> (-6.53%)	⬇️
... and 163 more

[ChrisJefferson]

I know in the past you mentioned translating all functions (was that global functions?). While the result wouldn't be useful, perhaps add a test which tries to translate all functions, just so we know the code at least doesn't crash / get broken to where it crashes?

[fingolfin]

Some remarks on this PR. These do not constitute change requests, nor do they block merging; I simply went through the code to understand it, and have a couple questions on it. I am interested in helping with the work on this code, but before I make changes resp. PRs for it, I'd like to understand it better. So I'd appreciate any (partial) answers :-).

[fingolfin]

My suggestion (and one on which I'd be happy to work on, if people agree with the general idea) would be to not use ADDR_STAT nor ADDR_EXPR here. Instead, pass the T_BODY object as an additional argument body to each syntax tree callback; then replace ADDR_STAT(stat)[i] by ADDR_OBJ(body)[stat+i] -- and of course one can use helper functions or macros to make this nicer, and e.g. replace ADDR_STAT(stat)[i] by, say ReadStat(body, stat, i) with

static inline Stat ReadStat(Obj body, Stat stat, UInt i)
{
   return ADDR_OBJ(body)[stat+i];
}

Some more work is needed (e.g. for ARGI_INFO), so it's not a quick change, but if one introduces the above ReadStat helper, it could easily be done incrementally.

Added bonus: an error during syntax tree compilation does not risk leaving STATE(PtrBody) in a bad state.

[markuspf]

I rebased and cut down the amount of code added (removing incomplete example compilers and other fancy technology).

I did add floats.

Once this is merged I will first make a PR that adds a function that takes a syntax tree that is produced by SYNTAX_TREE and turns it back into a callable GAP function.

I have some example code that shows how I imagine writing "compilers" for GAP code, for example a "Pretty Printer" which I will complete and submit as a PR.
We could do fairly simple constant folding, certain amounts of inlining (possibly with the help of #1811), and other fun stuff with this technology.

I'd like to incrementally work on this now, for example one can also consider moving from that slightly horrible macro trap to C++ templates (though I have not carefully thought about this).

[fingolfin]

While I have a few remaining nitpicks, overall this is in good shape, and could be merged now and improved later.

[fingolfin]

I was about to comment that this will loose the sign of the integer (in my "suggested changes", I had a check if ( T_INTNEG == *(UInt *)ADDR_EXPR(expr)) size = -size; for that).

But I then tried to repro this, and it turns out that integer can never be negative right now:

gap> 2^100;
1267650600228229401496703205376
gap> st:=SyntaxTree(x -> 1267650600228229401496703205376);
<syntax tree>
gap> st!.tree;
rec( argnams := [ "x" ], locnams := [  ], narg := 1, nloc := 0, stats := rec( statements := [ rec( obj := rec( type := "T_INT_EXPR", value := 1267650600228229401496703205376 ), type := "T_RETURN_OBJ" ) ], type := "T_SEQ_STAT" ),
  type := "T_FUNC_EXPR", variadic := false )
gap> st:=SyntaxTree(x -> -1267650600228229401496703205376);
<syntax tree>
gap> st!.tree;
rec( argnams := [ "x" ], locnams := [  ], narg := 1, nloc := 0, stats := rec( statements := [ rec( obj := rec( op := rec( type := "T_INT_EXPR", value := 1267650600228229401496703205376 ), type := "T_AINV" ), type := "T_RETURN_OBJ" ) ]
        , type := "T_SEQ_STAT" ), type := "T_FUNC_EXPR", variadic := false )

However, in the future, we may want negative entries, e.g. as result of optimization passes, so perhaps still fix this?

Of course, in the future, we might also want to ditch this IntExpr expression, and merge it with the float expressions: I'd envision that every T_BODY would also carry an (optional) pointer to a plist containing a list of (immutable) constants. Then, a new T_CONST_EXPR could be added which consists of just an index into that list of constants (it could even be an "immediate" like T_INTEXPR and T_REFLVAR). Then we can replace T_INT_EXPR and T_FLOAT_EXPR_EAGER by this, and perhaps also T_CHAR_EXPR (I would keep T_TRUE_EXPR and T_FALSE_EXPR, as we check those in various places; but you may disagree). Anyway, in the long run, this could then also be used to transform arbitrary expressions like MakeImmutable("abc") into a constant, as an optimization... This would then also go a good way towards resolving issue #1993.

[fingolfin]

Suggested change

	{ 0 } };
	{ 0, 0, 0, 0, 0 } };

Otherwise I get a compiler warning with clang.