Skip to content

Fix strange issue with reassigned argument values#205

Merged
dstelzer merged 3 commits intomainfrom
misbehaving-closures
Mar 23, 2026
Merged

Fix strange issue with reassigned argument values#205
dstelzer merged 3 commits intomainfrom
misbehaving-closures

Conversation

@dstelzer
Copy link
Copy Markdown
Contributor

Fixes #204

See there for explanation

@dstelzer dstelzer marked this pull request as ready for review March 16, 2026 02:56
@bkirwi
Copy link
Copy Markdown
Contributor

bkirwi commented Mar 16, 2026

The linked issue does a great job of describing the issue, but the fix seems to me just as obscure; any brief description of why this fixes that?

@dstelzer
Copy link
Copy Markdown
Contributor Author

It's a weird one! Basically, when compiling IR, Dialog tries to use as few temporary registers as possible. As part of this, it keeps an array called known_args that holds the known values of all the argument registers. If it needs to load a constant into a register, and one of the known_args already holds that constant, it uses that argument register instead of a new temporary.

Except in a certain edge case, the value of the argument register could change, but the slot in known_args would not be updated. For example:

(test 1)
    (show [[1] [2]])

When (test $) is called, its argument 1 is put into argument register A0, and it needs to put the list [[1] [2]] into A0 for the (show $) call. As it's building that list, known_args for A0 still has the value 1, so it tries to use that to construct the list. But A0 has already been overwritten by [2], so it ends up constructing [[[2]] [2]] instead.

This PR fixes the problem by updating the known_args in that edge case: putting in a new value if it's a compile-time constant, or just setting it to NULL if it's not.

@dstelzer dstelzer merged commit ce43ebf into main Mar 23, 2026
4 checks passed
@dstelzer dstelzer deleted the misbehaving-closures branch March 23, 2026 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strange bug with closures

2 participants