[addder]

What was wrong

A script with two or more function prototypes compiles to code whose calls never return. For example:

int A(int n);
int B(int n);
int A(int n) { return n + 1; }
int B(int n) { return n + 2; }
int StartingConditional() { return A(1) + B(1) * 10; }

should return 32. The compiled script instead runs into the wrong bodies and, in the VM I tested it on, aborts with a call-depth overflow (the same shape with recursion, Fact/Fib, loops until the call stack is exhausted). A file with a single prototype compiles correctly, so the bug shows up only when a file has at least two prototypes. That is common in game scripts: 21 of the 1,774 vanilla K1 .nss sources declare at least two prototypes in the file itself.

Cause

FunctionForwardDeclaration.compile emits a NOP as a placeholder (the stub) for each prototype. When the matching FunctionDefinition is compiled later, _compile_function finds the stub with

stub_index = ncs.instructions.index(root.function_map[name].instruction)

and inserts the body after it. list.index compares with ==, and NCSInstruction.__eq__ compares type, arguments and jump target by value. All stubs are NOPs with no arguments and no jump, so they are all equal, and index returns the first stub in the file. The second prototype's body is therefore inserted after the first prototype's stub, and the calls to the second function jump to its own (empty) stub, which falls through into whatever follows.

The fix

Look the stub up by identity:

stub = root.function_map[name].instruction
stub_index = next(i for i, instruction in enumerate(ncs.instructions) if instruction is stub)

One site, one statement. With one prototype the result is unchanged; with several, each body lands at its own stub, in prototype order, whatever order the definitions appear in.

Tests

Five byte-level regression tests in TestNCSCompiler, in a new "Multiple Forward Declaration Tests" region after the other function tests. Each compares bytes_ncs(compile_nss(...)) with an NCS file assembled in the test from the instruction layout (a small label-resolving assembler in the test class), so they do not use Interpreter and do not depend on the compiler's own output:

• test_two_prototypes_defined_in_declaration_order
• test_two_prototypes_defined_in_reverse_order
• test_two_prototypes_interleaved_with_definitions (prototype, definition, prototype, definition)
• test_two_prototypes_with_unprototyped_function_between (a third function with no prototype defined between the other two definitions)
• test_recursion_through_two_prototypes (two functions calling each other)

All five fail on master and pass with this change.

I also compiled seven small scripts and ran them on an independent NCS VM: Fact/Fib with both prototypes first (calling Fact), the same with each prototype directly before its definition, one prototype with two definitions, both prototypes first (calling Fib), mutual recursion with one prototype, and the A/B example above in both definition orders. With this change they return 120, 55120, 2120, 55, 11, 32 and 32, as expected. On master five of them abort with a call-depth overflow; the two scripts with a single prototype already gave the right value.

Test results

Scoped command from CONTRIBUTING.md (Windows, Python 3.12, Libraries/PyKotor/tests, K1_PATH unset), compared test by test from the JUnit XML:

	passed	failed	skipped
master (ca61ce8)	1575	5	143
this branch	1580	5	143

The only difference is the five new tests. The same five tests in tests/common/test_decode_fallbacks.py fail on both (test_8bit_encoding_only, test_bom, test_no_valid_encoding_found_strict_errors, test_strict_error_handling_decoding_failure, test_with_BOM_included); they are unrelated to this change. (The skip count is higher than on a full checkout because the native _gl_accel extension was not built in my worktree.) ruff format --check is clean for both touched files, and ruff check reports no finding on the changed lines.

Other notes

• bleeding-edge has the same line (classes.py, ncs.instructions.index(root.function_map[name].instruction)), so it needs the same one-line change there.
• I found this while testing the compiler against the VM; it is independent of the engine-call argument order issue (NSS compiler (since 2.0.0) pushes engine-function arguments in source order; NCS expects last argument first #351) and reproduces on master with scripts that make no engine calls.

[ProjectSynchro]

Closing this for now, Please see: #354