-
Notifications
You must be signed in to change notification settings - Fork 3
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
add support for unary not #29
Conversation
} | ||
} | ||
stmts | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this works? Tracing through the logic, I think you're missing a .collect()
or something to actually pull all the values together to have a complete vec of statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm safe because I pushed them one by one here:
https://github.com/ejrgilbert/whamm/pull/29/files/6a05d42c900ac243fb6973aa1e4be50c66f7207b#diff-665de66fa07a44b55ab96803488e9d1f829f7fe1179c3e1aca83e81b999923f3R172
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, it's mapping to the unwrapped Some
contents and then iterating in the for
loop. That makes sense.
Also, looks like merging your ast print visitor PR resulted in some merge conflicts here. |
Looks good! Merging now |
Add support for unary operator not.
Also applied some cargo clippy changes.