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

/STRING seems to have problems with negative values #46

Closed
scotws opened this issue Jun 7, 2018 · 8 comments
Closed

/STRING seems to have problems with negative values #46

scotws opened this issue Jun 7, 2018 · 8 comments
Assignees
Labels
bug Flat out, plain broken, seriously, dude, fix this
Projects
Milestone

Comments

@scotws
Copy link
Owner

scotws commented Jun 7, 2018

... at least it's failing the test suite there.

@scotws scotws added the bug Flat out, plain broken, seriously, dude, fix this label Jun 7, 2018
@scotws scotws added this to the BETA milestone Jun 7, 2018
@scotws scotws self-assigned this Jun 7, 2018
@scotws scotws added this to To do in REACH BETA Jun 7, 2018
@SamCoVT
Copy link
Contributor

SamCoVT commented Jun 7, 2018

What tests are you running?
I tried the ones at:
http://forth-standard.org/standard/string/DivSTRING
and they initially failed because I was still in hex mode, but worked fine once I switched to decimal mode:

: s1 s" ABCDEFG" ;  ok
\ Put in test words here, which switches base to hex.
{ s1 5 /string -> s1 swap 5 + swap 5 - }  ok
{ s1 10 /string -4 /string -> s1 6 /string } INCORRECT RESULT: { s1 10 /string -4 /string -> s1 6 /string } ACTUAL RESULT: { 17A4 -5 } ok
\ Figured out base was still hex at this point.
decimal  ok
{ s1 5 /string -> s1 swap 5 + swap 5 - }  ok
{ s1 10 /string -4 /string -> s1 6 /string }  ok
{ s1 0 /string -> s1 }  ok 

@scotws
Copy link
Owner Author

scotws commented Jun 8, 2018

I pushed an update to master where I am now, and the string stuff works after I got rid of the tabs (which is worrying in itself, we might have a problem with PARSE and whitespace?). At the moment, it crashes at

{ output-test -> } you should see the standard graphic characters:
!"#$%&'()*+,-./0123456789:;<=>?@
ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`
abcdefghijklmnopqrstuvwxyz{|}~
you should see 0-9 separated by a space:
0 1 2 3 4 5 6
7 8 9

    PC  AC XR YR SP NV-BDIZC

65C02: 6873 0a 78 00 f7 00110100

Py65 Monitor

   PC  AC XR YR SP NV-BDIZC

6502: 0000 00 00 00 ff 00110000
.

Py65 Monitor

   PC  AC XR YR SP NV-BDIZC

6502: 0000 00 00 00 ff 00110000

which all worked fine before I included the string tests. I'm going to be busy with the Real World (c) for the next couple of days, so that's where I have to leave it for now.

@scotws
Copy link
Owner Author

scotws commented Jun 8, 2018

I pushed an update to master where I am now, and the string stuff works after I got rid of the tabs (which is worrying in itself, we might have a problem with PARSE and whitespace?). At the moment, it crashes at

{ output-test -> } you should see the standard graphic characters: !"#$%&'()*+,-./0123456789:;<=>?@ ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_
abcdefghijklmnopqrstuvwxyz{|}~
you should see 0-9 separated by a space:
0 1 2 3 4 5 6
7 8 9

    PC  AC XR YR SP NV-BDIZC

65C02: 6873 0a 78 00 f7 00110100

Py65 Monitor

   PC  AC XR YR SP NV-BDIZC

6502: 0000 00 00 00 ff 00110000
.

Py65 Monitor

   PC  AC XR YR SP NV-BDIZC

6502: 0000 00 00 00 ff 00110000
` which all worked fine before I included the string tests. I'm going to be busy with the Real World (c) for the next couple of days, so that's where I have to leave it for now.

@SamCoVT
Copy link
Contributor

SamCoVT commented Jun 8, 2018

I did have to replace tabs with spaces in John Hayes' code to get it to work with Tali. If both tabs and spaces are supposed to be equivalent as a separator, that is not the case at the moment.
As a side note, my copy of gforth doesn't accept tabs as input because it has tab completion.

@scotws
Copy link
Owner Author

scotws commented Jun 8, 2018

So I checked and the standard says clearly that PARSE-NAME (which EVALUATE uses) skips leading spaces (https://forth-standard.org/standard/core/PARSE-NAME). However, if you trick Gforth into accepting tabs with s\" \t words" evaluate, it will actually print out the list of words it knows. I think for the moment I'll just add a suggestion to the docs that EVALUATE doesn't work with tabs and we can figure out what to do at some later stage.

@SamCoVT
Copy link
Contributor

SamCoVT commented Jun 9, 2018

The usage page for the standard http://forth-standard.org/standard/usage#subsubsection.3.4.1.1
says in section 3.4.1.1 that you may treat control characters (which I guess TAB falls into due to it's low ASCII value) as delimiters, but you certainly don't have to. I have no issues with space (BLANK) being the only delimiter.

I noticed you commented out the test case with the negative value passed to /string. I believe you will find it works if you change the base to decimal before running that code. It currently sees 10 as $10 and moves 16 characters forwards in the first step. I got it to pass a test run by adding decimal before the test:

testing string words: /string -trailing sliteral  ok
  ok
decimal  ok
  ok
{ : s1 s" abcdefghijklmnopqrstuvwxyz" ; -> }  ok
   ok
{ s1  5 /string -> s1 swap 5 + swap 5 - }  ok
{ s1 10 /string -4 /string -> s1 6 /string }  ok
{ s1  0 /string -> s1 }  ok

I think I want to break the tests up into separate files and have a way to run only one group of tests as well as all of the tests. I'm thinking of a makefile where you might type make all to run the entire test suite or perhaps make strings to load up the test words and then run only the string tests. I'll send you a pull request if it bothers me enough to get around to it.

I've also found a bug in the python tester program that may be hiding the exact location of the crash back to the simulator monitor (#49). I'll keep you updated as I dig deeper.

@scotws
Copy link
Owner Author

scotws commented Jun 9, 2018

I think breaking up the test into multiple parts is a great idea, simply because of the time now involved in testing (I now finally know why all those people in the movies always have code running through on some terminal window -- they're testing their 8-bit Forths!).

The first step would be to break up talitests.fs into various files based on their word set in the standard (core, core_ext, ...) with one separate one for Tali/Gforth sourced stuff. The file was originally named core.fs, so I probably shouldn't have renamed it.

I'm wondering if a make file would be the best solution, or if this should be made part of the Python testing script instead. I'm never sure if the Windows people can run make out of the box, for one. We'd have to include argparse for command line arguments, but I had been wondering if we wanted to do that anyway for some other options: silent (only print the errors), beep (make a noise when it's done), and chose the output file (instead of always overwriting results.txt). Then we could add an option to run one or more specifics tests, or "all", which would be a default.

I've enough stuff with argparse that I should be able to add the basic structures pretty quickly, once I can sit down to do it, that is.

@SamCoVT
Copy link
Contributor

SamCoVT commented Jun 9, 2018

I think you can close this bug as it seems to be working properly. To get your test case working, you can either switch to decimal (be sure to switch back to hex afterwards, as following tests expect it) or adjust the numbers in the test to be be hex or to be 9 or less so it doesn't matter. Do you want me to issue a pull request with one of those, or do you want to just fix it up yourself?

@scotws scotws closed this as completed in 5d5de3f Jun 10, 2018
@scotws scotws moved this from To do to Done in REACH BETA Jun 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Flat out, plain broken, seriously, dude, fix this
Projects
No open projects
REACH BETA
  
Done
Development

No branches or pull requests

2 participants