-
Notifications
You must be signed in to change notification settings - Fork 160
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
Dynamic arrays of non-managed struct and classic arrays of managed struct #2106
Dynamic arrays of non-managed struct and classic arrays of managed struct #2106
Conversation
But was not this already supported, since the beginning? Or did something change in their regard? |
@fernewelten I see that you drafted this PR as something that would solve two feature requests. Is one of them is covered by the changes already? Seeing this has stalled, could that make sense to make a PR only for the first feature for now? |
Thanks for the enquiry. I'm at it. I hope to be done in the next few days. |
Consistency, especially in the parser Also, denote unsigned literals as such
Also, some var renamings
It is very common that the compiler knows what symbol should be coming and then skips over it blindly, using 'GetNext()'. Modify these places to call the new function 'SkipNextSymbol()' instead. It takes a symbol that is expected and makes sure that it actually comes next.
…peWith()' Modify 'VartypeWithArray()' to make it possible to join adjacent classic array specifiers '[2, 3][5]' → '[2, 3, 5]' Replace 'VartypeWith()' by the functions - 'VartypeWithConst()' - 'VartypeWithDynarray()' - 'VartypeWithDynpointer()' The semantics of the respected cases has become that different that it doesn't make sense to have a common ‘umbrella’ function for them all.
Symboltable: – Modify 'VartypeWithArray()' to process arrays of arrays Parser: – Allow dynamic arrays of non-managed structs – Disallow dynamic arrays of classic arrays because that would make 'new' expressions ambiguous – Grok 'new[…][]' when initialising dynarrays of arrays – Make multiple indexes big-endian – In 'AccessData_ProcessCurrentArrayIndex()' → '…_ProcessArrayIndex()', grok a single index of a sequence of indexes. Clarify comments Googletests – Retracted the test that checks whether dynamic arrays of non-managed structs are disallowed (they are allowed now)
6c00f9e
to
3909560
Compare
So here's the code for the PR. Note that I've force-pushed. I've modified the very first comment to include all the functional changes. As for the changes in the code, I've done five commits: I've done some refactoring to the code, and it's clearer to keep the refactoring changes (e.g., renaming of variables) that don't modify any logic separate from the actual work that adds or changes logic. |
Hmm, could you clarify something, in the PR's description you say that this fixes #2080, which is "regular arrays of dynamic arrays". But further on you say "You can't define a classic array of a dynamic array". I cannot see any example for #2080 in the ticket's description either. So are these supported now, or not, and what is your opinion on the feature proposal itself? On another hand, in the PRs title and description you mention "Classic array of managed struct", but these were already supported since the beginning of managed pointers. Did anything change in their regard, why this is mentioned here? |
So this PR isn't about new concepts only. I've gone through the different cases of managed/non-managed entities and new/classic arrays that we offer so far and tried to describe them in a consistent system. Behind-the-scenes, I've refactored the relevant code, and these were the cases that I kept in mind that must all work or keep working. |
When we have a declaration such as The way I've implemented it, So
The reason why I chose this way over the other is, what happens with
I wanted to keep that rule in a world where we can have So generalising that, this means that in an array of an array, the outermost dimension is written first. |
You can have classic arrays where each component is a dynamic array, but you cannot have dynamic arrays where each component is a classic array. So currently, That's an implementation restriction that I might be able to release. |
@fernewelten Thank you for clarification!
This makes total sense to me indeed.
It's hard to tell how much of a priority that is. I've been trying to imagine a reason to do something like this, and the only case that comes to mind is having a dynamic array of fixed-size char arrays, to store "raw strings" of const size in them. |
So, I tested your PR for all 3 new features, and they seem to work! The script I used is under spoiler
This is going to be a good addition to the AGS script syntax. |
Found couple of errors.
You may then proceed accessing this second array, writing and reading elements in it. It works, but I suspect that memory contents become incorrect after this.
The compiler prints following error message:
(the garbage may vary of course) |
IMHO this is a type-check bug. I'm looking into it.
Darn, I've forgotten a |
Dynamic array types must match exactly. Also: fix small bug by adding '.c_str()' to a std::string expression
Adding a better typecheck and fixing the 'c_str()' bug. |
I confirm these issues were fixed. |
Fixes #1818 and #2080.
The new compiler can now generate code for dynamic arrays of non-managed
struct
s and for classic arrays of managedstruct
s.Also, you can define dynamic arrays of dynamic arrays and dynamic arrays of classic arrays. These can be nested at any depth.
When there are pointers of entities that have pointers, then RTTI must be enabled.
Dynamic array of non-managed struct
Typical code:
Lemmings
inroom_FirstLoad()
points to one chunk of memory where 10 structs are tucked side-by-side.Lemmings[0]
,Lemmings[1]
etc. will access the respective warriors, no need to initialise them individually withnew
. But the dynamic array itself must be initialised withnew[]
, as shown in the code above.If you go that way, then the static struct (in this case,
struct Warrior
) must not contain any dynamic variable components, not even indirectly – unless RTTI is enabled. For instance, the following code will fail to compile, unless RTTI is enabled:Classic array of managed struct
Typical code:
Warriors
inroom_FirstLoad()
is a chunk of 30 pointers tucked side-by-side. The classic array itself need not be initialised. But to get 30 usable components, you must initialise each of them withnew
, as shown in the code above.The managed struct may not contain dynamic variable components unless RTTI is enabled, but that's nothing new.
Dynamic arrays of dynamic arrays
This will only work when RTTI is enabled.
Here's code that defines and uses a dynamic array of a dynamic array of
short
s:Here's code to define and use a dynamic array of a dynamic array of a managed
struct
:You can nest the dimensions as deeply as you want. So
int Field[][][][][][];
is fine. You'll have to initialize the structure as follows:Field = new Field[7][][][][][];
Field[i] = new Field[7][][][][];
(for all non-negative values ofi
that are lower than 7)Field[i][j] = new Field[3][][][];
(for all the combinations ofi
andj
wherei
is lower than 7 andj
is lower than 3)Field[ì][j][k] = new Field[5][][];
…Field[i][j][k][l][m] = new Field[99];
Limitation:
You can't define a classic array of a dynamic array, e.g.,
int Field[][3, 5];
is forbidden. Once you've started with the dynamic markers[]
, you can only add further dynamic markers to the right.On the other hand, you can define a dynamic array of a classic array, e.g.,
int Field[3, 5][][];
.