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

Update step5_tco.mal #93

Closed
wants to merge 1 commit into from
Closed

Update step5_tco.mal #93

wants to merge 1 commit into from

Conversation

PhilipCollison
Copy link

test should succeed on correct implementation

test should succeed on correct implementation
@PhilipCollison
Copy link
Author

Hmm... it appears that this change should also modify the EXCLUDE_TESTS in MAKFILE to be exactly those tests that fail test5 (although I'm a bit curious about why some of those languages would fail...).

At this point I could either make the required changes in MAKEFILE and submit another pull request or you could close this request and I could drop the matter. What are you thoughts?

@PhilipCollison
Copy link
Author

Ah-ha!

After looking through the ruby example (and from my own experiences resolving this issue) it would appear that the TCO issues come from not evaluating argument before they're passed into the closure.

As an example from my own code:

} else if(auto closure = cast(MalClosure)list[0]) {
    MalVal[] exprs = [];
    ast = closure.ast;
    env = new Env(closure.env, closure.binds, exprs);
    ...

gave me the same error that ruby currently has while

} else if(auto closure = cast(MalClosure)list[0]) {
    MalVal[] exprs = [];
    foreach(v; list[1..$]) {
        exprs ~= EVAL(v, env);
    }
    ast = closure.ast;
    env = new Env(closure.env, closure.binds, exprs);
    ...

allowed me to evaluate 50005000 successfully.

Conceptually the argument would be that arguments to functions should be values as opposed to asts.

Unfortunately this procedure is just creating a bit-more-explicit call-stack which means that you'll still stack-overflow eventually.

If you want to ensure a stack-overflow or equivalent then I suggest a limit higher than 1000 as apparently I've misunderstood the intent of this test.

@kanaka
Copy link
Owner

kanaka commented Oct 25, 2015

The main purpose of the second section of the step5 tests is to be a control for the first part. In other words, target languages without automatic TCO should still succeed for the first part but fail for the second part. A second purpose of the second section of step5 is to make sure that implementations without automatic TCO in the target language throw a stack overflow without crashing.

I added an issue to split the non-TCO test case into implementation specific tests directories: #94

@kanaka kanaka closed this Oct 25, 2015
@PhilipCollison PhilipCollison deleted the patch-1 branch August 28, 2017 01:46
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.

None yet

2 participants