added `@macroexpand` by jw3126 · Pull Request #18660 · JuliaLang/julia

jw3126 · 2016-09-24T10:58:32Z

tkelman · 2016-09-24T11:19:32Z

base/expr.jl

 macroexpand(x::ANY) = ccall(:jl_macroexpand, Any, (Any,), x)

+"""
+@macroexpand


missing indent, and would have to be added to rst if this gets merged before #18588

where do I have to add it to rst?

simonbyrne · 2016-09-24T20:36:47Z

test/expr.jl

+    @test (@macroexpand @doc "" f() = @x) == Expr(:error, UndefVarError(Symbol("@x")))
+    @test (@macroexpand @seven_dollar $bar) == 7
+    x = 2
+    @test (@macroexpand @seven_dollar 1+$x) == :(1 + 2)


Is this actually the correct behaviour? Shouldn't it throw an error?

You are right!

macro macroexpand(code) code_expanded = macroexpand(code) Expr(:quote, code_expanded) end

code_expanded looks good the problem is the eval call on Expr(:quote, code_expanded) which does $ interpolation.

So would the solution be to search and quote the dollars expressions in code_expanded?

I'm honestly not sure. This stuff always makes my head hurt.

I think correct behaviour would be not to throw an error but return something like
1 + $(Expr(:$, :x))

Yeah my head hurts, too.

Maybe it would be better to print the result, rather than return it, i.e. do:

macro macroprint(code) print(macroexpand(code)) end

After all, that's probably what you want in 99% of cases.

👎 to print it.

macro macroexpand(code) QuoteNode(macroexpand(code)) end

Is eval(QuoteNode(ex)) == ex for all expressions ex?

It should be.

simonbyrne · 2016-09-25T09:25:08Z

LGTM

yuyichao · 2016-09-25T13:35:13Z

test/expr.jl

@@ -0,0 +1,18 @@
+


Why is this in a new file? This can probably go to parse or replutil or reflection.

yuyichao · 2016-09-25T13:41:16Z

base/expr.jl

+"""
+    @macroexpand
+
+Return equivalent expression with all macros removed (expanded).


It should be documented that the expansion happens in the module where the @macroexpand macro is expanded (i.e. the same module if the expression is used directly) and not the current module when the returned code runs (which is the module if you call macroexpand) The difference is demonstrated below

julia> module M macro macroexpand(code) QuoteNode(macroexpand(code)) end macro m() 1 end function f() (@macroexpand(@m), macroexpand(:(@m))) end end M julia> macro m() 2 end @m (macro with 1 method) julia> M.f() (1,2)

This makes no difference when used in the REPL and as I said the current @macroexpand might be slightly better but this should still be documented.

Can you give a usecase this would actually make a difference? And why is @macroexpand potentially better for interactive use?

Can you give a usecase this would actually make a difference?

That's exactly what the code above is showing.

And why is @macroexpand potentially better for interactive use?

I said

This makes no difference when used in the REPL

yuyichao · 2016-09-25T13:47:17Z

LGTM other than the comments above.

tkelman · 2016-09-26T02:16:16Z

commit messages could also be a bit more descriptive

yuyichao · 2016-09-26T02:30:02Z

The PR title and the first commit message looks OK (more detail is fine but it's not bad). I assume the fix up commits will be squashed when merged.

kshyatt · 2016-09-26T20:36:33Z

So this needs a squash and then can be merged? Is that correct, @yuyichao?

yuyichao · 2016-09-26T20:38:20Z

Also #18660 (review)

yuyichao · 2016-09-29T19:02:15Z

base/expr.jl

+julia> M.f()
+(1,2)
+```
+With @macroexpand the expression expands where @macroexpand appears in the code (module M).


@macroexpand, macroexpand and M should be quoted. Otherwise 👍

yuyichao

LGTM. Will merge later after the CI finishes. Thx for the contribution.

jw3126 · 2016-09-29T19:22:05Z

Cool!

tkelman · 2016-10-03T04:51:27Z

base/expr.jl

-With macroexpand the expressions expands in the current module where the code was finally called.
-Note that when calling macroexpand or @macroexpand directly from the REPL, both of these contexts coincide, hence there is no difference.
+With `@macroexpand` the expression expands where `@macroexpand` appears in the code (module `M`).
+With `macroexpand` the expressions expands in the current module where the code was finally called (REPL).


this isn't the repl if the function is run in a script, is it?

Let me explain it differently. If you call macroexpand, the expansion takes place at runtime. If you call @macroexpand the expansion takes place at compile time (when @macroexpand is expanded to be more precise.). So the context of @macroexpand is where it appears in the code, while the context of macroexpand is the current environment. So if you run M.f() from a script, the answer will be
(1, whatever @m() currently means in your script)
Does this answer the question?

That makes sense. I'm referring to the "where the code was finally called (REPL)." here. I don't see why "(REPL)" is accurate.

But in the example it is the REPL?

You're making a general statement here "with macroexpand the expression expands` ... followed by a very not-general parenthetical that doesn't serve much purpose

The julia> in the example implies that it runs in the REPL, doesn't it? And the "(module M)" in the sentence before also makes sense only in context of the example. So I'd say either leave as is or rephrase as "(module M in the example)" and "(REPL in the example)" (which I could well do in #18784).

@martinholters I am happy with either formulation, so rephrase it as you see fit.

a few words saying "in the example" sounds like a good clarification, thanks

added @macroexpand

988f7a1

tkelman reviewed Sep 24, 2016

View reviewed changes

jw3126 added 2 commits September 24, 2016 13:57

Update expr.jl

bff375b

added @macroexpand to rst

0b2c3ca

simonbyrne reviewed Sep 24, 2016

View reviewed changes

fix

2dfe407

yuyichao reviewed Sep 25, 2016

View reviewed changes

yuyichao requested changes Sep 25, 2016

View reviewed changes

kshyatt added the needs docs Documentation for this change is required label Sep 26, 2016

jw3126 added 2 commits September 29, 2016 20:49

fixup: improving docu and removing test/expr.jl

691052d

fixup

ebe9b53

yuyichao reviewed Sep 29, 2016

View reviewed changes

Update expr.jl

c66c096

yuyichao approved these changes Sep 29, 2016

View reviewed changes

Update choosetests.jl

f8157dc

yuyichao removed the needs docs Documentation for this change is required label Sep 30, 2016

yuyichao merged commit 759ac10 into JuliaLang:master Sep 30, 2016

tkelman reviewed Oct 3, 2016

View reviewed changes

tkelman mentioned this pull request Oct 5, 2016

Couple of small docs fixes #18784

Merged

Uh oh!

Conversation

jw3126 commented Sep 24, 2016 • edited by yuyichao Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuyichao Sep 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonbyrne commented Sep 25, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jw3126 Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuyichao commented Sep 25, 2016

Uh oh!

tkelman commented Sep 26, 2016

Uh oh!

yuyichao commented Sep 26, 2016

Uh oh!

kshyatt commented Sep 26, 2016

Uh oh!

yuyichao commented Sep 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuyichao left a comment

Choose a reason for hiding this comment

Uh oh!

jw3126 commented Sep 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

jw3126 commented Sep 24, 2016 •

edited by yuyichao

Loading

yuyichao Sep 24, 2016 •

edited

Loading

jw3126 Sep 26, 2016 •

edited

Loading