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

Macro expansion bug? #378

Closed
yrashk opened this Issue Jul 4, 2012 · 9 comments

Comments

Projects
None yet
4 participants
@yrashk
Contributor

yrashk commented Jul 4, 2012

iex> expr = quote do
...> if !a, do: oneway, else: anotherway
...> end
{:if,0,[{:"!",0,[{:a,0,:quoted}]},[{:do,{:oneway,0,:quoted}},{:else,{:anotherway,0,:quoted}}]]}
iex> IO.puts Macro.to_binary(Macro.expand(expr, __ENV__))  
case(!!a) do
  false ->
    oneway
  true ->
    anotherway
end
:ok
iex> Macro.expand(expr, __ENV__)
{:case,0,[{:"!",0,[{:"!",0,[{:a,0,:quoted}]}]},[{:do,{:"->",0,[{[false],{:oneway,0,:quoted}},{[true],{:anotherway,0,:quoted}}]}}]]}
@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Jul 4, 2012

Member

What is supposed to be the bug here? Notice that Macro.expand does not expand recursively.

Member

josevalim commented Jul 4, 2012

What is supposed to be the bug here? Notice that Macro.expand does not expand recursively.

@yrashk

This comment has been minimized.

Show comment
Hide comment
@yrashk

yrashk Jul 4, 2012

Contributor

I guess what I was looking for is recursive macro expansion... anyway, why does it expand ! to !! ?

Contributor

yrashk commented Jul 4, 2012

I guess what I was looking for is recursive macro expansion... anyway, why does it expand ! to !! ?

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Jul 4, 2012

Member

if actually expands to case(!...), that's why you get two !!. Btw, if you can probably loop through the expressions if you want to expand it recursively. I have updated the docs to be clear about it.

Member

josevalim commented Jul 4, 2012

if actually expands to case(!...), that's why you get two !!. Btw, if you can probably loop through the expressions if you want to expand it recursively. I have updated the docs to be clear about it.

@josevalim josevalim closed this Jul 4, 2012

@yrashk

This comment has been minimized.

Show comment
Hide comment
@yrashk

yrashk Jul 4, 2012

Contributor

No way we can support optional recursive expansion? Doesn't make much sense?

Contributor

yrashk commented Jul 4, 2012

No way we can support optional recursive expansion? Doesn't make much sense?

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Jul 4, 2012

Member

Maybe with another function that does it explicitly, like expand_all. But I
haven't needed it myself, so I don't have plans to work on it.

Member

josevalim commented Jul 4, 2012

Maybe with another function that does it explicitly, like expand_all. But I
haven't needed it myself, so I don't have plans to work on it.

@jechol

This comment has been minimized.

Show comment
Hide comment
@jechol

jechol Oct 12, 2017

I expected Macro.expand to expand recursively, so that I had to do googling a bit of time to find this issue.
Seems a little bit confusing because there's Macro.expand_once.

jechol commented Oct 12, 2017

I expected Macro.expand to expand recursively, so that I had to do googling a bit of time to find this issue.
Seems a little bit confusing because there's Macro.expand_once.

@jechol

This comment has been minimized.

Show comment
Hide comment
@jechol

jechol Oct 12, 2017

In Macro.expand_once doc,

Notice that expand_once/2 performs the expansion just once and it is not recursive.
Check expand/2 for expansion until the node can no longer be expanded.

It says Macro.expand_once is not recursive and recommend to check Macro.expand for such a case. It sounds like Macro.expand would expand recursively...

jechol commented Oct 12, 2017

In Macro.expand_once doc,

Notice that expand_once/2 performs the expansion just once and it is not recursive.
Check expand/2 for expansion until the node can no longer be expanded.

It says Macro.expand_once is not recursive and recommend to check Macro.expand for such a case. It sounds like Macro.expand would expand recursively...

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Oct 12, 2017

Member

Macro.expand recurses but it does not traverse.

Member

josevalim commented Oct 12, 2017

Macro.expand recurses but it does not traverse.

@OvermindDL1

This comment has been minimized.

Show comment
Hide comment
@OvermindDL1

OvermindDL1 Oct 12, 2017

For anyone that needs to recursively expand, I use this all over the place in my macro libraries (it is surprising just how often I need it actually...):

expanded_ast = Macro.prewalk(ast, &Macro.expand(&1, env))

But yeah, as seen it is easy enough to write it that it is not really needed to add it to the Macro module.

OvermindDL1 commented Oct 12, 2017

For anyone that needs to recursively expand, I use this all over the place in my macro libraries (it is surprising just how often I need it actually...):

expanded_ast = Macro.prewalk(ast, &Macro.expand(&1, env))

But yeah, as seen it is easy enough to write it that it is not really needed to add it to the Macro module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment