Parser cleanup

Started by fernewelten, Mon 14/01/2019 14:17:04

Previous topic - Next topic

fernewelten

So, I am back.  :)

I've done my best to touch up the parser code. I'm providing googletests that cover 75 % of the code lines and prove that the rewritten parser generates exactly the same bytecode as the pre-rewrite parser, byte for byte. Nearly all the work is in solution Compiler.Lib, file script/cs_parser.cpp. Details in the note to the change request I posted (to the AGS4 branch).

Tell me what you think.

Now I am unsure how to proceed.

Up to now, I had a "closed, self-contained system" consisting of just the parser files and googletests.  I've pushed that as far as it would go. But now integration comes into play. For instance, when I write a program in the AGS studio, it has predefined macros:

Code: ags

for (function Loop = 0; Loop < 10; Loop ++) {
    break;
}

This is legal AGS code, and the only way this makes sense is assuming that "function" has been #defined as "int" somewhere. I don't know where the macros and stuff are set up, or more generally, just what happens at all before cc_parse() is called.

Also, as far as I know, you can call predefined properties in AGS code, but you can't define your own properties in AGS code -- so I can't write self-contained googletests that check property parsing code.

I also don't know what happens after cc_parse() has done its thing. There's bound to be a linking step somewhere?

So how do I widen the horizon (efficiently)?



Crimson Wizard

#1
Hello!

There was little "hiccup" with Alan deleting "ags4" branch in our repository :), and your pull request was automatically closed.
In our repository "ags4" is merged into "master" branch now, which is the branch for AGS4 development, although it's not really being developed much yet. We were mostly working in "ags3" branch for the last 6 months, with what could be the last big 3.* release. But all of its contents are merged to master too.

I asked following on the github as well: please create a new feature branch based on top of our latest master and put only your related commits there. This way there will be proper pull request without unnecessary commits from the older history.

Quote from: fernewelten on Mon 14/01/2019 14:17:04
Now I am unsure how to proceed.

Well, depends on what are your intentions. Usually we recommend having one PR per task. If your first aim was to refactor and enhance the code (not adding/removing any functionality) that would be first pull request that needs to be checked and fixed if necessary to keep everything in working state when new code is merged.

Meanwhile you may ofcourse proceed with other changes, creating another feature branch (this is also why making separate branches per task is beneficial).


Quote from: fernewelten on Mon 14/01/2019 14:17:04
Also, as far as I know, you can call predefined properties in AGS code, but you can't define your own properties in AGS code -- so I can't write self-contained googletests that check property parsing code.

If by "properties" you mean ones declared with attribute keyword (e.g. Object.X), user may create these. Unfortunately they were never properly documented in the manual, but there's this wiki topic for the reference: https://www.adventuregamestudio.co.uk/wiki/Keyword:_attribute


I cannot answer your other questions yet, but I'll try to find time and investigate.

fernewelten

#2
Quote from: Crimson Wizard on Mon 14/01/2019 20:56:00
If by "properties" you mean ones declared with attribute keyword (e.g. Object.X), user may create these. Unfortunately they were never properly documented in the manual, but there's this wiki topic for the reference: https://www.adventuregamestudio.co.uk/wiki/Keyword:_attribute

I've just gone hunting through the code, and you had the right hunch; thanks for the tip. All the original parsing code and all the relevant comments in the parsing code seem to talk about “properties”, but the word that is actually expected in AGS code for that concept is “attribute”. So you have code, e.g., in Compiler/script/cc_symboltable.cpp, that says:
Code: c++

add_ex("export", SYM_EXPORT, 0);
add_ex("return", SYM_RETURN, 0);
add_ex("readonly", SYM_READONLY, 0);
    ...
add_ex("attribute", SYM_PROPERTY, 0);
add_ex("enum", SYM_ENUM, 0);
add_ex("managed", SYM_MANAGED, 0);


One line in the code above is the odd man out: guess which one it is?  :) Oh well.


Crimson Wizard

Yes it is one.

By the way, I don't know if you found that earlier, but you may see how built-in API is declared in this file:
https://github.com/adventuregamestudio/ags/blob/master/Editor/AGS.Editor/Resources/agsdefns.sh

This script is embedded as resource into Editor's binary and prepended to user scripts before compilation.

fernewelten

Great! That's exactly what I was looking for. And it contains the line that #defines "function" to mean "int", too.  ;-D

SMF spam blocked by CleanTalk