Support `CONTINUE` Statement

engineerjoe440 commented 1 year ago

Hello! It's been a while! I'm poking around at the "linter" project I've been working on, again, and I stumbled across a bit of missing grammar. I think that we're missing "CONTINUE" from the grammar. Looks like it's an extension of the 61131 standard, but both CODESYS and Schneider Electric seem to support it:

A simple example might be like the following:

WHILE condition DO
    IF another_condition THEN
        CONTINUE;
    END_IF
END_WHILE

I'll see if I can't get a PR up to add the syntax.

klauer commented 1 year ago

Yet another good catch 👍 Happy to take a look into this if you ran into issues while trying to make a fix.

Looks like it parses as valid grammar as-is but as a "no op" statement. Why this is technically valid grammar isn't clear to me:

engineerjoe440 commented 1 year ago

Thanks, @klauer!

I'm a bit confused by the screenshot you posted here. Could you elaborate on what the bValue "magic FB_Init Variable" might be in reference to?

I forget, now, what the status of my working branch was. I think I had a rudimentary implementation in place, but I think I was running into an alarming number of failures, even when I looked at the master branch. I wonder if there's something else that changed that's causing some of those tests to fail? Taking this in a tangent... but at least it was found during this exploration! 😝

Thank you, again!

klauer commented 1 year ago

Ah, sorry - my reply was a bit terse. Fleshing it out a bit more:

Take, for example, something basic like:

FUNCTION_BLOCK test
VAR
  bValue : BOOL;
END_VAR

  bValue;

END_FUNCTION_BLOCK

The bValue; in the implementation section is technically valid code that TwinCAT will compile and only warn about with: The code 'bValue;' has no effect. Is this the intent?. That's what the above screenshot was showing.

Now that's relevant here because my assumption - which I just confirmed with a quick blark parse - is that CONTINUE; is parsed into the same node type. After all, an undefined variable name is still a variable name, as far as the grammar is concerned:

$ blark parse -i abc.st
In [1]: print(result)
FUNCTION_BLOCK abc
    WHILE TRUE
    DO
        CONTINUE;
    END_WHILE
END_FUNCTION_BLOCK

In [2]: result
Out[2]: SourceCode(items=[FunctionBlock(name=Token('IDENTIFIER', 'abc'), access=None, extends=None, implements=None, declarations=[], body=StatementList(statements=[WhileStatement(expression=SimpleVariable(name=Token('IDENTIFIER', 'TRUE'), dereferenced=False), statements=StatementList(statements=[NoOpStatement(variable=SimpleVariable(name=Token('IDENTIFIER', 'CONTINUE'), dereferenced=False))]))]))], filename=PosixPath('test.st'), raw_source='FUNCTION_BLOCK abc\n  WHILE TRUE DO\n        CONTINUE;\n  END_WHILE\nEND_FUNCTION_BLOCK\n')

Specifically: NoOpStatement(variable=SimpleVariable(name=Token('IDENTIFIER', 'CONTINUE'), dereferenced=False)

I forget, now, what the status of my working branch was. I think I had a rudimentary implementation in place, but I think I was running into an alarming number of failures, even when I looked at the master branch. I wonder if there's something else that changed that's causing some of those tests to fail? Taking this in a tangent... but at least it was found during this exploration! 😝

Ah, there may be some big changes upstream in lark. I haven't run the test suite recently to see what the state is...

engineerjoe440 commented 1 year ago

Aha!!! Ok... I see the connection now.

I guess I was overthinking... (really? an engineer? overthink? no...... 😆 ) and I had thought you were implying something about variables in relation to something like an FB_Init method call. I see your point about recognizing CONTINUE as a variable. And that makes perfect sense. It's pretty much exactly what I'd observed!

Thank you, again! :tada:

engineerjoe440 commented 1 year ago

Ooops... Missed that we didn't close this one when it was merged. 🤦

klauer / blark

Support `CONTINUE` Statement #47