-
Notifications
You must be signed in to change notification settings - Fork 30
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 macro expander - arrays ignored? #24
Comments
In generator.go |
Yeah, you're right. For arrays and hashes, it just puts the literal array in without checking recursively for unquotes. We should recurse for both arrays and hashes. I don't have much spare time these days since I'm busy with Ph.D. research. Do you think you can take care of it? You'll have to add new instruction types to the VM. They should be something like the "squash" instruction I use for syntax quoting lists, except they should generate arrays or hashes instead. |
Yeah, I can probably do it, but I really appreciate the confirmation of what's needed. I was thinking that new instruction(s) might be needed, but thought there might be an easier way. One question is -- does it make sense to spill everything in an array to the stack and then pack it back into the array? Maybe we could get away with expanding and squashing each element in turn in isolation....? |
I think spilling things out onto the stack and then squashing them all in one go is the right way to go. It isn't very efficient to build up an array in bits and pieces, since that will involve a lot of copying. And since we're only doing this for macro expansions, it's unlikely the array will be very large anyway. |
I added you as a collaborator to this repo. You can make your changes in a new branch. |
Thanks. I'll give it go. |
This is fixed in GoDiesel; commit https://github.com/glycerine/godiesel/commit/33f37342f63655b3688fbe61f286d38b6dd23617 includes the work and some additions (to support for loops, range loops and upscope-setting of variables by set). It will need minor adjustments for Glisp, but that should be straightforward. I may not get to backporting it to Glisp myself in the near term; anyone with a few minutes and inclination should find it straightforward. Ping me with any questions. See in particular generate.go:908-1048 and the two new instructions Vectorize and Hashize in vm.go. |
As I've renamed my dialect Zygomys, the above mentioned links need to be replaced by these: |
It appears there's some kind of bug in the macro expander. Example below. This might have to do with doing expansions within an array, as in the let-array below.
The text was updated successfully, but these errors were encountered: