Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New compiler: as VARTYPE clause in expressions #2693

Merged
merged 4 commits into from
Feb 26, 2025

Conversation

fernewelten
Copy link
Contributor

@fernewelten fernewelten commented Feb 24, 2025

Provide syntax to dynamically cast a pointer to a managed type to a pointer to another managed type.

The compiler will generate bytecode to check at runtime whether the object actually has the new type. (opcode 76). The engine must be able to interpret that bytecode, of course; work which is provided in #2665 .

Currently, the compiler assumes that opcode 76 expects a managed pointer in MAR. If this were changed to AX then the compiler could generate more compact code.

Typical code:

managed struct Me
{
    int Me;
};

managed struct Child extends Me
{
    int Child;
};

managed struct Grandchild extends Child
{
    int Grandchild;
};

int game_start()
{
    Me m = new Grandchild as Child;
    Grandchild *g = m as Grandchild *;
}

As concerns the third line from the bottom:

  • new Grandchild yields a Grandchild *, i.e., a dynpointer. It wouldn't make sense to convert a pointer to a non-pointer, so the compiler implies a * in the clause as Child.
  • m is a variable that accepts Me * where Me is an ancester of Child, so the compiler can prove statically that the expression can go into m. It does not generate a dynamic check.

As concerns the second line from the bottom:

  • Me is an ancester of Grandchild, so the compiler can't know at compile time whether m, which is a Me *, can be converted to a Grandchild *. So it does generate a dynamic check (opcode 76). If that check fails at runtime, then g will receive a null.

Googletests change that use symbol numbers (RTTI)
@ivan-mogilko ivan-mogilko added ags 4 related to the ags4 development context: script api labels Feb 24, 2025
@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Feb 25, 2025

I gave this a quick test, and this seem to work, if merged with #2665.
I only had to fix few things, one mistake in the generated compiler code (commented a commit above), and another change was to the interpreter, where it should treat register's value as a ready pointer rather than a handle.

Please tell, how should this instruction be modified using AX instead? is the logic same, just using AX instead of MAR, or there should be more arguments? I can do necessary changes to #2665 right away, then merge your PR first, and rebase 2665 after.

@fernewelten
Copy link
Contributor Author

Please tell, how should this instruction be modified using AX instead? is the logic same, just using AX instead of MAR, or there should be more arguments? I can do necessary changes to #2665 right away, then merge your PR first, and rebase 2665 after.

Let's go with SCMD_DYNAMICCAST works on the AX register instead of the MAR register, other logic unchanged.

@fernewelten
Copy link
Contributor Author

fernewelten commented Feb 25, 2025

Implemented the changes: Assume that SCMD_DYNAMICCAST works on AX; shut up the compiler warnings about std::copy by dropping the const keywords instead of dropping the std::copys

(Had to force-push)

@ivan-mogilko
Copy link
Contributor

Tested again, and the cast itself is working well (so long if I modify the engine correspondingly), which is great.

I found that there's a problem with chaining calls though. Consider following script:

GUIControl control = gGui1.Controls[1];
String text = (control as Label).Text;

I expect this to work, but compiler cannot handle this expression and throws error:

Expected an operator, found '.' instead

(is not '.' an operator?)

@fernewelten
Copy link
Contributor Author

fernewelten commented Feb 25, 2025

(is not '.' an operator?)

No, unfortunately it isn't. This is a left-over from the old compiler.

The compiler treats whole concatenations of literal, identifier, . and […] as atomic building blocks, or "extended variable names", as it were, and parses these building blocks separately. Only complete building blocks can be part of an expression and joined with operators such as +. The underlying, unspoken assumption seems to be that only numeric expressions can be expressions. (The old compiler has a similar approach and moreover an upper limit on how long the building blocks can be.)

This is not the usual way of handling . and […] AFAIK. So I've thought several times of changing the compiler and bring it in line with what is more common. I've even tried out some tentative code for this in GIT branches. But then I thought, well the code works even if it is quirky, so let's focus on other issues.

This might be the occasion to change this at long last. But l'd suggest making a separate PR for it.

@ivan-mogilko
Copy link
Contributor

Alright then.

I've been testing for syntax problems further, and found something else:
using a non-declared type in as expression leads to unexpected error, in the sense that the error message is not reporting an undefined type. For example:

Label label = lblGameName as UNDEFINED_TYPE;

Compiler reports:

Expected ',' or ';', found 'as' instead

I would expect it to say that UNDEFINED_TYPE is not a declared type.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Feb 26, 2025

I confirm that the last commit fixes the issue.

@ivan-mogilko ivan-mogilko merged commit 681d19c into adventuregamestudio:ags4 Feb 26, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ags 4 related to the ags4 development context: script api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants