richpl / PyBasic

Simple interactive BASIC interpreter written in Python
GNU General Public License v3.0
170 stars 46 forks source link

IF...THEN does not work if the THEN clause is not a jump #51

Closed sblendorio closed 3 years ago

sblendorio commented 3 years ago

Example that does not work:

10 for i=1 to 10
20 if i=3 then print i
30 next

It should print "3", but it raises this error:

Expecting factor in numeric expression in line 20PRINT
RetiredWizard commented 3 years ago

This is actually a further generalization of issue #44. I believe @richpl based the original PyBasic syntax on TI Basic which only supported GOTO functionality on an IF statement.

richpl commented 3 years ago

It's a fair point. TI BASIC does only allow jumps from IF ... THEN statements, but I've seen plenty of dialects that allow arbitrary statements to be used instead. So I'll leave this issue open as a marker to look at this eventually.

RetiredWizard commented 3 years ago

@richpl , I know you had put this aside as a project you were going to work on but I've been turning some ideas over in my head. I think I could put an option together that might work although the idea isn't fully flushed out.

If you'd rather develop a solution without being influenced by my hack at it, I'll drop it, but if you'd like to see what I come up with. I could put my thoughts together as a PR in case it's of any use to a solution.

richpl commented 3 years ago

@RetiredWizard, I need to take a look, but I think that in principle it shouldn't be too difficult to implement, and it would give me GOSUB from an IF...THEN for free. I think I just need to jump back into statement processing after the THEN or ELSE if the next token is not GOTO or a number. By all means let me know your thinking though.

RetiredWizard commented 3 years ago

I actually did flush the rest of my thoughts out and tested them. The approach I took was to have the parse method check for an IF token and if it finds one, first call the stmt method as usual to allow if statement to determine the conditional results and then make a recursive call to parse to process the tokens after either the THEN or ELSE.

I'm a little disappointed by the fact that between the changes to support compound statements (the ":" separator) and my conditional generalization the parse method ended up so far from your original routine. The original parse method was very clear and easy to follow.

One thing I realized after coding the update was that you don't have to look for the GOTO anymore, like the GOSUB you get the GOTO for free :)

Let me now if you want me to post my changes as a PR.

RetiredWizard commented 3 years ago

Here's my updated version of parse:

def parse(self, tokenlist, line_number):
    """Must be initialised with the list of
    BTokens to be processed. These tokens
    represent a BASIC statement without
    its corresponding line number.

    :param tokenlist: The tokenized program statement
    :param line_number: The line number of the statement

    :return: The FlowSignal to indicate to the program
    how to branch if necessary, None otherwise

    """
    # Remember the line number to aid error reporting
    self.__line_number = line_number
    self.__tokenlist = []
    self.__tokenindex = 0
    linetokenindex = 0
    for token in tokenlist:
        # If statements will always be the last statment processed on a line so
        # any colons found after an IF are part of the condition execution statements
        # and will be processed in the recursive call to parse
        if token.category == token.IF:
            # process IF statement to move __tokenidex to the code block
            # of the THEN or ELSE and then call PARSE recursivly to process that code block
            # this will terminate the token loop by RETURNing to the calling module
            #
            # **Warning** if an IF stmt is used in the THEN code block or multiple IF statement are used
            # in a THEN or ELSE block the block grouping is ambiguious and logical processing may not
            # function as expected. There is no ambiguity when single IF statements are placed within ELSE blocks
            linetokenindex += self.__tokenindex
            self.__tokenindex = 0
            self.__tokenlist = tokenlist[linetokenindex:]

            # Assign the first token
            self.__token = self.__tokenlist[0]
            flow = self.__stmt() # process IF statement
            if flow and (flow.ftype == FlowSignal.EXECUTE):
                # recursive call to process THEN/ELSE block
                try:
                    return self.parse(tokenlist[linetokenindex+self.__tokenindex:],line_number)
                except RuntimeError as err:
                    raise RuntimeError(str(err)+' in line ' + str(self.__line_number))
            else:
                # branch on original syntax 'IF cond THEN lineno [ELSE lineno]'
                # in this syntax the then or else code block is not a legal basic statement
                # so recursive processing can't be used
                return flow
        elif token.category == token.COLON:
            # Found a COLON, process tokens found to this point
            linetokenindex += self.__tokenindex
            self.__tokenindex = 0

            # Assign the first token
            self.__token = self.__tokenlist[self.__tokenindex]

            flow = self.__stmt()
            if flow:
                return flow

            linetokenindex += 1
            self.__tokenlist = []
        elif token.category == token.ELSE:
            # if we find an ELSE we must be in a recursive call and be processing a THEN block
            # since we're processing the THEN block we are done if we hit an ELSE
            break
        else:
            self.__tokenlist.append(token)

    # reached end of statement, process tokens collected since last COLON (or from start if no COLONs)
    linetokenindex += self.__tokenindex
    self.__tokenindex = 0
    # Assign the first token
    self.__token = self.__tokenlist[self.__tokenindex]

    return self.__stmt()
richpl commented 3 years ago

Thanks @RetiredWizard, that looks good and I'm especially glad that you comment your code! I have to sprinkle my code with comments so that I can understand it myself, let alone anyone else! Please submit the PR and if you could update the ReadMe accordingly that would be great. Might be worth an explicit warning in the ReadMe of the perils of nested conditions - not something I thought about before but I guess a natural consequence of BASIC's lack of braces.

I assume the THEN and ELSE branches can contain colon separated statements too?

Very much appreciated. This project would not have received the attention it has without the work you and @brickbots put into it.

richpl commented 3 years ago

Fixed by @RetiredWizard