Assertion failure when ternary operator is used inside an `#if defined` block

[Daniel-Cortez]

Issue description:

The following code:

#if defined main
Func()
{
	new a[4];
	new x = 0;
	a = x ? "ab" : "xyz";
	return a[0];
}
#else
Func(){}
#endif

main()
{
	Func();
}

causes an assertion failure when the compiler is built with assertions enabled:

Assertion failed: result, file source\compiler\sc3.c, line 1094

The line where the assertion fails is https://github.com/pawn-lang/compiler/blob/bab39b409144abbf435b39c530964724d68d8430/source/compiler/sc3.c#L1094

Minimal complete verifiable example (MCVE):

See above.

Workspace Information:

• Compiler version: 3.10.10, 3.2.3664
• Command line arguments provided (or sampctl version):
• Operating System:

[Daniel-Cortez]

In order to explain this bug, I'll use the following naming for the operands: expr_cond ? expr_true : expr_false

So, if expr_true and expr_false are arrays, one or both of them might be array-returning function calls, so before generating the code for those expressions the compiler needs to allocate heap space to temporarily store the returned array(s), but first it needs to know the sizes of returned arrays, in order to know how much space to allocate on the heap. Since the compiler works in a linear way, it can't know the array sizes in advance, so on the 1'st compilation pass, after parsing expr_true and expr_false it pushes the sizes of the resulting arrays into a queue ("push_heaplist(max-heap1,max-heap2)"), then on the 2'nd pass it pops the sizes from the queue ("popfront_heaplist(&heap1,&heap2)") and, if needed, allocates heap space before generating the code for expr_true and expr_false.

In other words, the compiler relies on the source code being the same on both the 1'st and the 2'nd passes - but this can't be guaranteed because of how conditional compilation of #if defined <function> blocks works! If ternary operator is used inside a conditionally compiled block, the compiler can make more pops on the 2'nd pass than pushes on the 1'st pass or vice versa, thus causing assertion failure (or allocating a garbage amount of bytes on the heap if the compiler is built without assertions enabled).

So I think the obvious solution is:

• we can't rely on the source code being the same on the 1'st and the 2'nd passes, because of conditional compilation, ...
• ... which means that there's no reliable way to know the amount of allocated heap space before generating the code for expr_true and expr_false, ...
• ... so we should probably try to allocate the heap space after generating the code for expressions, with a clever use of jump instructions.

It might be not the perfect solution, as it implies generating slightly more code, but for now this is the only solution I can think of.

Another problem is that when the array sizes in expr_true and expr_false differ, the compiler takes only the size of the first array into account when copying the array result of a ternary expression (see #49). To solve this, we may want to copy the data by a dynamic amount of bytes specified in PRI or ALT when the array sizes are different, but currently there's no way to do that, as instruction movs takes the data size (in bytes) as a constant argument, because PRI and ALT are already reserved for source and destination addresses. My proposition is to

• use a function stub (like one of those the compiler generates for state functions) to copy the data by a dynamic amount of bytes/cells;
• at the end of the code generated for both expr_true and expr_false store the array size in ALT, so it can be passed to the function stub later.

Again, this solution might be not perfect, as it means more generated code, and obviously copying the data directly via movs is faster than using a function stub, but at least it would help to overcome the limitation of instruction movs having a constant data size argument.

Would it be alright if I use the solutions described in this post to fix array copying for the ternary operator? Or maybe there are better ways to fix it? @Y-Less, @YashasSamaga?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity.