(stop) within (collect words) crashes dgdebug

[susan-davis]

Test code in question:

#dont-go-north
(test *)
(run *)
	(current player $Player)
	($Player is #in #foyer)
	(collect words) (perform [go #north]) (into $) %% throw away output
	($Player is #in #foyer)

...which results in

Attempting 10 tests.
Assertion failed: (w->flags & WORDF_DICT), function eval_run, file eval.c, line 2533.
Testing #dont-go-north:make: *** [cloak] Abort trap: 6

[dstelzer]

Fascinating! Let me see if I can reduce this down to an example that doesn't require the standard library.

[dstelzer]

Very interesting…it looks like what happens is that a (stop) gets called inside a (collect words), which leaves the program collecting words indefinitely, and thus some of the words it collects aren't in the dictionary (which should never happen).

[dstelzer]

Minimal example:

(program entry point)
	(stoppable) {
		(collect words)
			(inside collect)
			(stop)
		(into $)
	}
	(outside collect)

(inside collect)
	This will be collected into the dictionary

(outside collect)
	These words are not collected and should not be in the dictionary

The short-term fix is to make sure you never call something that can (stop) inside a (collect words). The long-term fix is to make sure the CWL register is reset at any (stop). The question is, how to make sure that happens?

(If the goal is just to keep output from being redirected to the screen, put it inside a (status bar $). In dgdebug, that ensures that the code inside will be run and its output discarded, whether or not it (stop)s. But if this is a frequent need, we should think about adding a proper way to do this to the language, if the (stop) issue can't be fixed quickly.)

[dstelzer]

This problem doesn't occur on Z-machine or Å-machine, thankfully—those would be much harder to fix! It looks like it's only the debugger that doesn't reset CWL properly. That's going to make this much easier.

[dstelzer]

There's code in compile.c (look for AN_OUTPUTBOX) that ensures the div is closed properly even if a (stop) happens inside it. That could easily be copied to AN_COLLECT_WORDS. But it doesn't seem to be necessary on Z-machine or Å-machine. Why not? What's happening differently there?

[dstelzer]

Here's the IR from that minimal example:

R0: (group leader) (1 incoming) clause 65535
	JUMP                      R1           -            -           
R1: (part of group R0) (1 incoming) clause 65535
	ALLOCATE                  0            0            -           
	PUSH_STOP                 R2           -            -           
	COLLECT_BEGIN 0           -            -            -           
	FOR_WORDS 1               -            -            -           
	PUSH_CHOICE               0            R3           -           
	SET_CONT                  R4           -            -           
	INVOKE_ONCE               (inside collect)
R2: (group leader) (1 incoming) clause 65535
	POP_CHOICE                0            -            -           
	POP_STOP                  -            -            -           
	DEALLOCATE 0              -            -            -           
	INVOKE_TAIL_ONCE 0        (outside collect)
R3: (group leader) (1 incoming) clause 65535
	POP_CHOICE                0            -            -           
	FOR_WORDS 0               -            -            -           
	MAKE_VAR                  X0           -            -           
	COLLECT_END_V 0           X0           -            -           
	STOP                      -            -            -           
R4: (group leader) (1 incoming) clause 65535
	STOP                      -            -            -           
R5: (unreachable) (0 incoming) clause 65535

I honestly can't figure out why this works on Z-machine and Å-machine. Clearly CWL is getting cleared again before invoking (outside collect), but how? The only way that can happen is FOR_WORDS 0…

[dstelzer]

Aha! The debugger is producing slightly different IR than the compiler! But why??

Compiler:

R0: (group leader) (1 incoming) clause 65535
	JUMP                      R1           -            -           
R1: (part of group R0) (1 incoming) clause 65535
	ALLOCATE                  0            0            -           
	TRACEPOINT 0              failed_assert.dg:1
	PUSH_STOP                 R2           -            -           
	COLLECT_BEGIN 0           -            -            -           
	FOR_WORDS 1               -            -            -           
	PUSH_CHOICE               0            R3           -           
	TRACEPOINT 1              failed_assert.dg:4
	SET_CONT                  R4           -            -           
	INVOKE_ONCE               (inside collect)
R2: (group leader) (1 incoming) clause 65535
	POP_CHOICE                0            -            -           
	POP_STOP                  -            -            -           
	TRACEPOINT 1              failed_assert.dg:8
	DEALLOCATE 0              -            -            -           
	INVOKE_TAIL_ONCE 0        (outside collect)
R3: (group leader) (1 incoming) clause 65535
	POP_CHOICE                0            -            -           
	FOR_WORDS 0               -            -            -           
	MAKE_VAR                  X0           -            -           
	COLLECT_END_V 0           X0           -            -           
	STOP                      -            -            -           
R4: (group leader) (1 incoming) clause 65535
	TRACEPOINT 3              failed_assert.dg:4
	TRACEPOINT 1              failed_assert.dg:5
	STOP                      -            -            -           
R5: (unreachable) (0 incoming) clause 65535

Debugger:

R0: (group leader) (1 incoming) clause 65535
	JUMP                      R1           -            -           
R1: (part of group R0) (1 incoming) clause 65535
	ALLOCATE                  0            0            -           
	TRACEPOINT 0              failed_assert.dg:1
	PUSH_STOP                 R2           -            -           
	COLLECT_BEGIN 0           -            -            -           
	FOR_WORDS 1               -            -            -           
	PUSH_CHOICE               0            R3           -           
	TRACEPOINT 1              failed_assert.dg:4
	SET_CONT                  R4           -            -           
	INVOKE_ONCE               (inside collect)
R2: (group leader) (1 incoming) clause 65535
	POP_CHOICE                0            -            -           
	POP_STOP                  -            -            -           
	TRACEPOINT 1              failed_assert.dg:8
	SET_CONT                  R6           -            -           
	INVOKE_ONCE               (outside collect)
R3: (group leader) (1 incoming) clause 65535
	POP_CHOICE                0            -            -           
	FOR_WORDS 0               -            -            -           
	MAKE_VAR                  X0           -            -           
	COLLECT_END_V 0           X0           -            -           
	STOP                      -            -            -           
R4: (group leader) (1 incoming) clause 65535
	TRACEPOINT 3              failed_assert.dg:4
	TRACEPOINT 1              failed_assert.dg:5
	STOP                      -            -            -           
R5: (unreachable) (0 incoming) clause 65535
R6: (group leader) (1 incoming) clause 65535
	TRACEPOINT 3              failed_assert.dg:8
	DEALLOCATE 1              -            -            -           
	PROCEED 0                 -            -            -           

Where is R6 coming into this…

[dstelzer]

In the meantime, putting an extra (stoppable) inside the (collect words) should also fix the problem. The issue only arises when a (stop) breaks out of the (collect words) too early.