curiousdannii-testing / inform7-imported-bugs

0 stars 0 forks source link

[I7-1173] [Mantis 1208] Repeat with precomputes the next iteration's value and is sometimes unsafe #911

Closed curiousdannii-testing closed 2 years ago

curiousdannii-testing commented 2 years ago

Reported by : curiousdannii

Description :

This is filed as a documentation issue because we figure you have good reasons for implementing it as you have, though if that's not the case please consider changing the implementation too!

So in Kerkerkruip we've run into a nasty bug which was very hard to diagnose: https://github.com/i7/kerkerkruip/issues/258#issuecomment-38198215

The core of the problem is that repeat with loops precompute the next iteration's value. This causes problems if the code for the current iteration alters the next object so that it no longer qualifies.

The docs should be updated to make authors aware of this potential problem, and to suggest how to safely perform these loops. "while there is (description) (called H):" is a safe alternative.

Steps to reproduce :


Additional information :

imported from: [Mantis 1208] Repeat with precomputes the next iteration's value and is sometimes unsafe
  • status: Closed
  • resolution: Resolved
  • resolved: 2022-04-07T04:56:17+10:00
  • imported: 2022/01/10
curiousdannii-testing commented 2 years ago

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by graham :
I've added a note to the documentation.

curiousdannii-testing commented 2 years ago

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by zarf :
The precomputation supports the (presumed common) case:

repeat with T running through things in the pot: now T is off-stage;

...which uses a linked-list and would therefore choke without precomputation.

(Sure, I would use a "now all things in the pot..." statement here, but not everyone would.)

I feel like precomputation is going to give the desired result in "most" cases. I haven't done any systematic study, but I figure that the ideal behavior is to decide the entire iterator set in advance and then loop through it. This is impractical for memory reasons, but precompute-one is a step closer to it than naive iteration.

I took a look at the Kerkerkruip bug but I'm not sure what the loop structure is.

curiousdannii-testing commented 2 years ago

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by curiousdannii :
Hmm I guess the underlying issue is that the iteration functions require the object to still match the description in order to find the next one. I don't really know how those functions work, but I wonder if they couldn't just store an index instead?

Kerkerkruip loops through windows opening them one at a time. But some of the windows then open windows themselves. When it gets back to the original loop it already has the next iteration's window and opens it even though it's already open.