search

adventuregamestudio / ags

AGS editor and engine source code
Other
707 stars 159 forks source link

New compiler: Only imply `*` in import object declarations when a compiler flag is unset #2513

Closed fernewelten closed 2 months ago

fernewelten commented 3 months ago

In legacy AGS, the engine autogenerates code such as import Object object[20]; where object is meant to be an array of Object and not an array of Object *. I'm told ( but didn't check this myself ) that this is no longer true in recent AGS 4.0. This PR makes the compiler cope with this situation.

Introduce the compiler flag SCOPT_NOAUTOPTRIMPORT which is false by default. Normally, whenever AGS code declares an object variable then always imply that it is pointered. However, in import declarations of non-autopointered varables only do so when the flag SCOPT_NOAUTOPTRIMPORT is false (which is the default).

In the legacy AGS situation, the flag SCOPT_NOAUTOPTRIMPORT can be set to true so that the compiler generates the correct (legacy) code. This might become interesting, e.g., if the compiler is backported to AGS 3 versions.

ivan-mogilko commented 3 months ago

@fernewelten this pr contains a copy of a commit from a previous pr, could you rebase on top of latest ags4 branch?

fernewelten commented 3 months ago

Okay (had to force-push)

ivan-mogilko commented 3 months ago

I am confused by a test Bytecode1.Attributes07b. https://github.com/adventuregamestudio/ags/pull/2513/commits/7a09e0c73f238a931265078b989fb131692f824c#diff-44471be089f78e9894080155761ea454320db2e28ebe3ee05c3e3556a3560f82R1467

It sais in comments:

// Assignment to attribute -- should not generate null dereference error // Assume that builtin managed objects are not autopointered.

Reading this comment I assume this means that the compiler will work in "legacy" mode, and generate an imported object Label lbl, not an automatic pointer.

However in the following test code the flag is set as ccSetOption(SCOPT_NOAUTOPTRIMPORT, false);, meaning it's compiled with autopointering?

Either the comment is not correct, or the test's logic is inverted?

ivan-mogilko commented 2 months ago

Ok, apparently the comment is incorrect, but the test is right. I might fix the comments myself after merging this.

Also tested this flag with the following script:

// in header
import Character PLAYERCHAR;
// in script
Character PLAYERCHAR;

This compiles well with SCOPT_NOAUTOPTRIMPORT unset (default), but fails with SCOPT_NOAUTOPTRIMPORT set (legacy mode), where it reports that PLAYERCHAR is declared as Character* in the header and Character in script.

ericoporto commented 2 months ago

Please add label :"context: script compiler" here.