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

(index.lisp) add-indexes: Ensure (safety 3) #9

Merged
merged 1 commit into from
Jun 17, 2016

Conversation

PuercoPop
Copy link
Contributor

Although modifying &REST lambda variables is not strictly forbidden by
the CLHS, it is discouraged[1]. When compiling under safety values less
than 3 SBCL assumes the &REST parameter to be a list, so calls to elt are
specialized to the list variation.

[1]: "There is a particular problem with &REST lambda variables, which
are always bound to a value of type LIST."
cf. http://www.lispworks.com/documentation/HyperSpec/Issues/iss178_w.htm

A better solution in the long term would be to rewrite add-indexes

@luismbo
Copy link
Member

luismbo commented Jun 17, 2016

How about wrapping the body with (let ((indexes indexes)) ...)? Does that work?

@PuercoPop
Copy link
Contributor Author

It does, I'll update the PR

Although modifying &REST lambda variables is not strictly forbidden by
the CLHS, it is discouraged[1]. When compiling under safety values less
than 3 SBCL assumes the &REST parameter to be a list, so calls to elt are
specialized to the list variation. Rebind the rest parameter to itself
as a workaround.

[1]: "There is a particular problem with &REST lambda variables, which
are always bound to a value of type LIST."
cf. http://www.lispworks.com/documentation/HyperSpec/Issues/iss178_w.htm
@PuercoPop
Copy link
Contributor Author

With this PR, #7 and #8 the test suite passes on SBCL. It still fails on CCL and on ABCL though. the ABCL failures seem related to this commit

@luismbo luismbo merged commit 039a619 into sharplispers:master Jun 17, 2016
@luismbo
Copy link
Member

luismbo commented Jun 17, 2016

Cool, thanks. Merged the three PRs.

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