-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Quote captures result
of containing macro
#7323
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
Comments
Yeah this is a really strange and annoying bug. One way I've found to still be able to use result in quote do is to do this: import macros
macro mac(): untyped =
let res = newIdentNode("result")
quote do:
proc test(): int =
(proc(): int = `res` = 123)()
mac()
echo test() # Works fine, outputs 123 |
I have figured out a nice simple fix for this now: PMunch@6089654. It's still not completely done but I hope to have it done by tomorrow and create a PR for this. |
A simple test case based on GitHub issue nim-lang#7323 on how you can't put result in a quote do block. This test verifies that it actually works correctly now.
Done, here is the PR for my fix: #7343 |
maybe this problem is resolved when the quote do macro is limited to bind proc- macro- and template-symbols. To my experience binding var-, let-, param, etc symbols always goes wrong. |
A simple test case based on GitHub issue nim-lang#7323 on how you can't put result in a quote do block. This test verifies that it actually works correctly now.
* Fix result not being able to use in quote do This fixes the annoying issue of not be able to use result inside a quote do block. It works by a simple trick. The quote do mechanic is based on dynamically creating a template and immediately calling it with the arguments found within the quote do block. Since this is called in the scope of the macro the result variable is shadowed. This trick works by changing all occurences of result (which shouldn't cause any issues as result isn't used for anything else for the same reason) to another name and then passing in an IdentNode with result as a named parameter with that name. Note that currently this just replaces it with a fixed named variable "res" which should be changed to a non-colliding, dynamically created name. * Fix hard coded parameter "res" to anonymous symbol This fixes the hard coded parameter "res" to be an anonymous symbol instead so it won't collide with other parts of the argument list. * Add test case for result in quote do block A simple test case based on GitHub issue #7323 on how you can't put result in a quote do block. This test verifies that it actually works correctly now. * Add test for explicit capturing of result * Rebased against devel
@krux02 maybe close this issue as well? |
I can confirm, this works now. |
* Fix result not being able to use in quote do This fixes the annoying issue of not be able to use result inside a quote do block. It works by a simple trick. The quote do mechanic is based on dynamically creating a template and immediately calling it with the arguments found within the quote do block. Since this is called in the scope of the macro the result variable is shadowed. This trick works by changing all occurences of result (which shouldn't cause any issues as result isn't used for anything else for the same reason) to another name and then passing in an IdentNode with result as a named parameter with that name. Note that currently this just replaces it with a fixed named variable "res" which should be changed to a non-colliding, dynamically created name. * Fix hard coded parameter "res" to anonymous symbol This fixes the hard coded parameter "res" to be an anonymous symbol instead so it won't collide with other parts of the argument list. * Add test case for result in quote do block A simple test case based on GitHub issue nim-lang#7323 on how you can't put result in a quote do block. This test verifies that it actually works correctly now. * Add test for explicit capturing of result * Rebased against devel
I've just copied the code from the top of the report and run it with Nim 0.19.2. It produced the same error message. |
works on devel |
Hello! Here's a contrived reproduction:
Using
return
instead circumvents the issue.The text was updated successfully, but these errors were encountered: