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

Filtering out generated symbols #7

Merged
merged 1 commit into from Feb 18, 2015
Merged

Conversation

zmaril
Copy link
Contributor

@zmaril zmaril commented Feb 17, 2015

When trying to generate code to destructure the results of generators
with for-all, the macro was getting caught up by gen-syms. By filtering
out symbols with the prefixes "map__" and "vec__", the macro no longer
tries to generate code that references the generated symbols and works
as expected (or at least how I expected it to work). A simple test was
added as well.

When trying to generate code to destructure the results of generators
with for-all, the macro was getting caught up by gen-syms. By filtering
out symbols with the prefixes "map__" and "vec__", the macro no longer
tries to generate code that references the generated symbols and works
as expected (or at least how I expected it to work). A simple test was
added as well.
@gfredericks
Copy link
Owner

Oh interesting -- the problem isn't that gensyms are inherently bad to capture this way, but since the test.chuck code calls clojure.core/destructure itself it just ends up with different gensyms than the ones that eventually get used in the course of normal destructuring.

@gfredericks
Copy link
Owner

I'm going to ponder whether there's an easy enough clean way to do this without guessing based on symbol names; thanks for the PR!

@zmaril
Copy link
Contributor Author

zmaril commented Feb 17, 2015

No problem! I'm using this fix locally for now but I have no idea if it will come back and break something else later on. All the other tests run just fine, but I don't know enough about how for works to say if there is a cleaner way to do this.

@gfredericks
Copy link
Owner

Okay I think I figured out a different approach. I'll merge this to get the test, add my changes on top, then make a release.

Thanks again.

gfredericks added a commit that referenced this pull request Feb 18, 2015
Filtering out generated symbols
@gfredericks gfredericks merged commit 6cd2864 into gfredericks:master Feb 18, 2015
@gfredericks
Copy link
Owner

Version "0.1.15" is out

@zmaril
Copy link
Contributor Author

zmaril commented Feb 18, 2015

This seems to work for me! Thanks for the quick release.

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