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

Bug in countdown DO +LOOP #12

Closed
hexagon5un opened this issue Jan 24, 2017 · 5 comments
Closed

Bug in countdown DO +LOOP #12

hexagon5un opened this issue Jan 24, 2017 · 5 comments
Assignees
Labels

Comments

@hexagon5un
Copy link
Collaborator

: foo 1 4 do i . -1 +loop ;

should print 4 3 2 1. Instead, it just prints 4.

@TG9541
Copy link
Owner

TG9541 commented Jan 24, 2017

Before implementing +LOOP I couldn't find any clear requirements. I then compared different implementations and found one the addressed the risk of very long loops due to erroneous upper limit and starting point. That's the one I implemented, since it doesn't require looking at the sign of the increment.
I now understand this is a valid use case, and I'll change the code :-)

@TG9541 TG9541 self-assigned this Jan 24, 2017
@TG9541
Copy link
Owner

TG9541 commented Jan 24, 2017

How should the following code behave?
: foo 1 4 do i . -2 +loop ;

@hexagon5un
Copy link
Collaborator Author

gforth and mecrisp-stellaris say: 4 2.

My understanding is that the TOS index is increasing the test is run against <, and when it is decreasing, it's run against <=. (https://www.forth.com/starting-forth/6-forth-do-loops/)

(Equivalently, I think it's begin >= until and begin > until loops under the hood.)

This is crappy, b/c 4 1 do i . 1 +loop -> 1 2 3 (doesn't 4 b/c 4<4 is false) and 1 4 do i . -1 +loop -> 4 3 2 1 (1 b/c 1 <= 1 is true), so the loops have different lenghts, but that's the way Forth is defined.

https://github.com/gerryjackson/forth2012-test-suite is written for ANS Forth. This is in core.fs:

T{ : GD2 DO I -1 +LOOP ; -> }T
T{ 1 4 GD2 -> 4 3 2 1 }T
T{ -1 2 GD2 -> 2 1 0 -1 }T

TG9541 added a commit that referenced this issue Jan 24, 2017
@TG9541
Copy link
Owner

TG9541 commented Jan 24, 2017

Thanks! Good links, too!

I fixed it. If you need binaries right away, I updated the 2.2.6.snapshot.

@TG9541 TG9541 removed the question label Jan 24, 2017
@TG9541 TG9541 closed this as completed Jan 24, 2017
@hexagon5un
Copy link
Collaborator Author

hexagon5un commented Jan 25, 2017

Binaries not necessary, and I can confirm that at least the obvious test cases are working as they should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants