Add Simplicity

The Robot World Repo on GitHub
The Forth Repo on GitHub

#ElonaldDelendaEst! Now that CASE is stable, let’s use what we learned elsewhere. I think we’ll simplify things a bit.

If there is a broad learning¹ that comes from our work with CASE, it is that all the conditionals can be done with the same operations, zero branch and branch, 0BR and BR, managed using our new CompileInfo object on the compile stack. Currently, there are special operations compiled for some of those. We’ll fix that.

We will encounter an issue with the looping constructs, which I mention so that I won’t be surprised by it later. Looping constructs branch backward So far our branch updates have plugged the addresses of later locations into branches at earlier locations. The looping constructs branch backward, form high addresses to low. We’ll have to use their CompileInfo objects a bit differently.

I think it’s possible that a bit of polymorphism in CompileInfo instances might come out of this. We’ll see.

Along the way, you might notice some small changes from the code you’ve seen before. I’ve done a bit of tidying from time to time, that I didn’t write about.

Let’s get to it. We’ll look at IF first.

    def _define_if_else_then(self):
        def _compile_conditional(forth, word_to_compile, word_list):
            forth.compile_stack.push(len(word_list) + 1)
            forth.compile_word(word_to_compile)
            forth.append_word(0)

        def _patch_the_skip(forth, skip_adjustment, word_list):
            patch_loc = forth.compile_stack.pop()
            last_loc = len(word_list) + skip_adjustment
            word_list[patch_loc] = last_loc - patch_loc

        def _if(forth):
            _compile_conditional(forth,'*IF', forth.word_list)

        def _else(forth):
            _patch_the_skip(forth, 1, forth.word_list)
            _compile_conditional(forth, '*ELSE', forth.word_list)

        def _then(forth):
            _patch_the_skip(forth, -1, forth.word_list)

        self.pw('IF',   _if,   immediate=True)
        self.pw('ELSE', _else, immediate=True)
        self.pw('THEN', _then, immediate=True)

What we see above is that IF pushes an address onto the compile stack and then compiles *IF. The silly stuff with the +1 is a result of not defining the address after it is allocated, as we do in our newer code. We’ll see in a moment that *IF is really just a zero branch, except using skip logic instead of address (word index) logic. At the time it seemed to be the logical thing to do.²

If there is an ELSE, the code in the *IF or 0BR needs to jump there, so we do the patch. And, since there is an ELSE, we’ll need a branch over the ELSE code, to be used from inside the IF block. So else will consume the current compile stack info and provide new.

THEN does not know whether it is dealing with an ELSE or an IF. (Our current scheme will have it check to be sure that the compile stack is OK.) In either case, it patches the forward branch that is waiting for an address. Then compiles no code: it’s just a target.

The other two words, *IF and *ELSE, are in essence a zero branch and an unconditional branch, defined as skips rather than branches, like this:

    def define_skippers(self, forth):
        def _next_word(forth):
            return forth.next_word()

        def _zero_skip(forth):
            branch_distance = _next_word(forth)
            if forth.stack.pop() == 0:
                forth.active_word.skip(branch_distance)

        self.pw('*IF',    _zero_skip)
        self.pw('*ELSE',  lambda f: f.active_word.skip(f.next_word()))

Planning

We want to wind up compiling 0BR and BR here. Let’s see if we can think of a few ways to do it, looking for small steps.

We might be able to do the BR and 0BR separately, one at a time.

We might change *IF and *ELSE to look more like 0BR and BR and then just replace them in the compiled code. We could perhaps do that by substituting the CompileInfo object in the right places and adjusting the code in those two old words until it matches the new ones.

We might start by putting a CompileInfo on the compile stack instead of the simple values presently being stacked, make that work, and then see what’s next.

I think I like that last idea: it seems that we have to use CompileInfo in any case, and it’s mostly just an information holder, so that change might be fairly easy. We’ll try that.

The only push to the compile stack for IF ELSE THEN is here:

    def _define_if_else_then(self):
        def _compile_conditional(forth, word_to_compile, word_list):
            forth.compile_stack.push(len(word_list) + 1)
            forth.compile_word(word_to_compile)
            forth.append_word(0)

We’ll change that, see what breaks, and with luck, fix it quickly. If that holds water, we proceed. If not, revert and see if we can come up with a better idea.

I think we can reorder that function and remove the +1, like this:

    def _define_if_else_then(self):
        def _compile_conditional(forth, word_to_compile, word_list):
            forth.compile_word(word_to_compile)
            forth.compile_stack.push(len(word_list))
            forth.append_word(0)

Yes, Green. Commit: refactoring IF THEN.

    def _define_if_else_then(self):
        def _compile_conditional(forth, word_to_compile, word_list):
            forth.compile_word(word_to_compile)
            info = CompileInfo('IF', word_list, len(word_list))
            forth.compile_stack.push(len(word_list))
            forth.append_word(0)

That does nothing, but we are green so commit again. Inching forward, ideally on every green. Now we use it. This will break things.

    def _define_if_else_then(self):
        def _compile_conditional(forth, word_to_compile, word_list):
            forth.compile_word(word_to_compile)
            info = CompileInfo('IF', word_list, len(word_list))
            forth.compile_stack.push(info)
            forth.append_word(0)

Five tests break. Here’s where they break:

    def _patch_the_skip(forth, skip_adjustment, word_list):
        patch_loc = forth.compile_stack.pop()
        last_loc = len(word_list) + skip_adjustment
>       word_list[patch_loc] = last_loc - patch_loc
E       TypeError: unsupported operand type(s) for -: 'int' and 'CompileInfo'

Yes. Here we need to accommodate the fact that there is a CompileInfo on the stack, thus:

        def _patch_the_skip(forth, skip_adjustment, word_list):
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            last_loc = len(word_list) + skip_adjustment
            word_list[patch_loc] = last_loc - patch_loc

We grab the info, grab the location is holds, and use that. We are green. Commit.

OK. I think we have the actual locations to which the branches should occur, so therefore we should be able to use them.

What if we compile a 0BR for IF?

        def _if(forth):
            forth.compile_word('0BR')
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.compile_word('BR_TARGET')

That’s the right sequence, I’m sure. Compile the branch, save the location on the compile stack, compile the branch target cell into that location.

The tests break, surely because now patch_the_skip is wrong.

        def _patch_the_skip(forth, skip_adjustment, word_list):
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            word_list[patch_loc] = len(word_list)

One test is still broken. It’s the else test. I think we should change this:

        def _else(forth):
            _patch_the_skip(forth, 1, forth.word_list)
            _compile_conditional(forth, '*ELSE', forth.word_list)

To this:

        def _else(forth):
            _compile_conditional(forth, '*ELSE', forth.word_list)
            _patch_the_skip(forth, 1, forth.word_list)

That does not fix the problem. We might want to revert but let’s at least look at what seems to be happening. Here’s the test:

    def test_else(self):
        f = Forth()
        s = ': TEST IF 5 ELSE 50 THEN ;'
        f.process_line(s)
        test_word = f.find_word('TEST')
        f.stack.push(1)
        test_word(f)
        assert f.stack.pop() == 5
        f.stack.push(0)
        test_word(f)
        assert f.stack.pop() == 50

It’s failing with an empty stack looking for the 50.

Let’s see what the code looks like.

: TEST 0BR 6 *# 5 *ELSE 5 *# 50 ;
       0   1  2    3    4  5    6

That 6 should be branching to 4. We’re branching to the end. Let’s revert. With the reversion in place, we get this code:

: TEST *IF 3 *# 5 *ELSE 1 *# 50 ;
       0   1  2    3    4  5    6

But remember (I almost didn’t.) *IF is a skip, not a branch, so the location it hits is 4, 3 + location 1, so location 4 is the right answer. Similarly for the *ELSE, which will skip to location 5, i.e. 1 plus location 4.

Reflection

We need to do something different, obviously, since what we just did didn’t work. But I don’t see why: it made sense to me at the time. Let’s see if we can come at it from a different angle. Can we make *IF and *ELSE use location instead of skip, and then back once they look enough like 0BR and BR, replace them?

But don’t they really use absolute location now?

No, wait. I think I see the issue. Or part of it. Here is *ELSE:

        self.pw('*ELSE',  lambda f: f.active_word.skip(f.next_word()))

That issues a skip, it’s not just reading that word for its own benefit. We didn’t change that.

OK. Pick up al the balls and start juggling again. Where are we?

I think I want to inline the code. It might make it easier to think about. We’ll see:

        def _else(forth):
            info1 = forth.compile_stack.pop()
            patch_loc = info1.locations[0]
            last_loc = len(forth.word_list) + 1
            forth.word_list[patch_loc] = last_loc - patch_loc
            forth.compile_word('*ELSE')
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.append_word(0)

        def _then(forth):
            adjustment = -1
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            last_loc = len(forth.word_list) + adjustment
            forth.word_list[patch_loc] = last_loc - patch_loc

I think this may actually help. We’ll make these two things work, then make them alike enough to extract common aspects on the way to using our standard branch instructions. Inline adjustment, rename info1. Consider the else:

        def _else(forth):
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            last_loc = len(forth.word_list) + 1
            forth.word_list[patch_loc] = last_loc - patch_loc
            forth.compile_word('*ELSE')
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.append_word(0)

We can avoid the adjustment of last_loc by reordering:

        def _else(forth):
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            forth.compile_word('*ELSE')
            last_loc = len(forth.word_list)
            forth.word_list[patch_loc] = last_loc - patch_loc
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.append_word(0)

Green. I am afraid to commit, thinking we may need another revert.

The *ELSE is an unconditional skip. So I should be able to compile an unconditional branch instead:

I have a different error after a bit more changing than I would have preferred but I think we might be close. The else test is failing with different code this time:

: TEST 0BR 4 *# 5 BR 6 *# 50 ;
       0   1  2   3  4 5     6

The branch 4, patching the 0BR should be 5. I make a patch that I don’t like and we are green with this code:

        def _if(forth):
            forth.compile_word('0BR')
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.compile_word('BR_TARGET')

        def _else(forth):
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            forth.compile_word('BR')
            last_loc = len(forth.word_list)
            forth.word_list[patch_loc] = last_loc + 1
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.compile_word('BR_TARGET')

The “fix” was the + 1 on the location patched.

Ah. We can get rid of that by reordering. Carefully.

        def _if(forth):
            forth.compile_word('0BR')
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.compile_word('BR_TARGET')

        def _else(forth):
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            forth.compile_word('BR')
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_word('BR_TARGET')
            forth.word_list[patch_loc] = len(forth.word_list)
            forth.compile_stack.push(info)

        def _then(forth):
            info = forth.compile_stack.pop()
            patch_loc = info.locations[0]
            forth.word_list[patch_loc] = len(forth.word_list)

I back away slowly, and commit: IF-ELSE-THEN uses 0BR and BR.

It should follow that we can remove *IF and *ELSE, and any tests that reference them directly. There are no such tests. Commit: remove unused *IF and *ELSE.

OK, not too bad. Couple of hours, couple of tries. I kind of lost the thread for a moment, and that final set of changes was larger than I’d have liked, changing all of IF ELSE and THEN at once, but since they interact, perhaps there was no shorter way. More likely, I just didn’t see it.

I think there is help on the CompileInfo for doing the patching, though its checking might cause us some trouble. Probably not, we’re using IF in both cases. We can do this:

        def _if(forth):
            forth.compile_word('0BR')
            info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(info)
            forth.compile_word('BR_TARGET')

        def _else(forth):
            old_info = forth.compile_stack.pop()
            forth.compile_word('BR')
            new_info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(new_info)
            forth.compile_word('BR_TARGET')
            old_info.patch('IF')

        def _then(forth):
            forth.compile_stack.pop().patch('IF')

The else is a bit nasty because we have to cache the old info for a while, until we get to the location we want it to branch to. Let’s do this instead:

        def _else(forth):
            forth.compile_word('BR')
            new_info = CompileInfo('IF', forth.word_list, len(forth.word_list))
            forth.compile_stack.push(new_info)
            forth.compile_word('BR_TARGET')
            forth.compile_stack.swap_pop().patch('IF')

We push the new one down when the location is right, then pull the old one out from under it with swap_pop. Works as advertised. Commit: refactoring _else.

I think we’ve done enough damage for today. Let’s sum up.

Summary

We now have IF ELSE and THEN all using 0BR and BR, removing words *IF and *ELSE entirely. We have the new implementation using CompileInfo to store the necessary information and do the patching.

The code, I think, is pretty decent, especially given that ELSE kind of does need to juggle the old and new locations, if we are to use our current notion of setting up and patching CompileInfo at the moment when the word we’re about to compile is the one we want patched or branched to.

I think we’ll find that a method on Forth will make this a bit simpler, and we’ll look for other commonalities next time. For this time, we have a better feature with less code, and that is what we set out to do.

A good morning!

#ElonaldDelendaEst! See you next time!