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 macro expander - arrays ignored? #24

Closed
glycerine opened this issue Jan 7, 2016 · 8 comments
Closed

bug in macro expander - arrays ignored? #24

glycerine opened this issue Jan 7, 2016 · 8 comments

Comments

@glycerine
Copy link
Collaborator

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.

> (def h (hash 'a 1 'b 2))
> h
{a 1 b 2}
> (defmac sizer [my-hash]                                                                                                                                                                          
  `(let [n (len ~my-hash)] (println n)))                                                                                                                                                           
> (sizer h)
(sizer h)
error in __main:1: symbol {my-hash 117} not found
> 
@glycerine
Copy link
Collaborator Author

In generator.go
func (gen *Generator) GenerateSyntaxQuote(args []Sexp)
does not recurse into arrays.

@zhemao
Copy link
Owner

zhemao commented Jan 8, 2016

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.

@glycerine
Copy link
Collaborator Author

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....?

@zhemao
Copy link
Owner

zhemao commented Jan 8, 2016

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.

@zhemao
Copy link
Owner

zhemao commented Jan 8, 2016

I added you as a collaborator to this repo. You can make your changes in a new branch.

@glycerine
Copy link
Collaborator Author

Thanks. I'll give it go.

@glycerine
Copy link
Collaborator Author

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.

@glycerine
Copy link
Collaborator Author

As I've renamed my dialect Zygomys, the above mentioned links need to be replaced by these:
https://github.com/glycerine/zygomys/blob/master/repl/generator.go
https://github.com/glycerine/zygomys/blob/master/repl/vm.go

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

No branches or pull requests

2 participants