Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix https://github.com/vlachoudis/brexx/issues/8 #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RossPatterson
Copy link

This PR implements the WORDPARSE operation from section 8.3.1.7 (pp. 86-90) of the ANSI X3J18-199X REXX Standard. This changes bREXX's PARSE instruction to correctly parse blank-separated targets between triggers, including the implied start-of-string and end-of-string triggers.

Because bREXX splits the FINDNEXTBREAK operation from the standard between the compiler and the interpreter, and out of a desire to make the fix as non-invasive as reasonably possible, I chose to leave most of the processing intact, with several significant changes:

  1. The compiled code for PARSE no longer proceeds in a straight path from OP_PARSE to OP_TR_END and OP_POP. Instead, I inserted OP_JMPs such that parsing of word-separated targets does not begin until after the trigger to their right has been evaluated. This results in all blank-separated targets are now correctly parsing only the data between the triggers to their immediate left and right.
  2. I introduced two more parsing-position variables in the main interpreter loop: word_start and word_end. These are used by the blank-separated-target parsing code to walk through the words in the input substring that was isolated by the triggers.

bRexx has no test suite, but I created a set of tests for PARSE that verify correct operation. To do so, I pulled many examples from my collection of Rexx code, and I typed in all the examples from Mike Cowlishaw's The Rexx Language, 2nd edition. I have attached the file parse_.txt, which demonstrates the failure described in Issue #8 (test 2 in the file), as well as several others I discovered while investigating this bug (tests 3, 8, and 20). It also shows one questionable behavior, that I need to confirm against the original VM/CMS Rexx implementation (test 16), and one separate bug that I'll open up another issue for (test 19). This PR fixes all but the last two, which will be addressed separately (if at all). Run the tests by simply running brexx parse_.txt.

parse_.txt

@mgrossmann
Copy link
Contributor

Hello @RossPatterson

will try to integrate this patch into BREXX/370 as well.

Thank you

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.

2 participants