Macroexpand1 #21662

jw3126 · 2017-05-01T11:53:16Z

This PR adds a non recursive variant of macroexpand, see #19365. What is the preferred API?

macroexpand1(ex), @macroexpand1(ex) ?
macroexpand(ex, recursive::Bool), @macroexpand(ex, recursive) ?

Both? Something else? For REPL usage I think @macroexpand1 is the most convenient, albeit a bit cryptic.

JeffBezanson · 2017-05-01T14:24:02Z

test/replutil.jl

+    @test macroexpand1(macroexpand1(ex)) == macroexpand(ex)
+    ex = :(@nest2b 42)
+    @test macroexpand1(ex) != macroexpand(ex)
+	@test macroexpand1(macroexpand1(ex)) == macroexpand(ex)


Should be 4 spaces.

JeffBezanson · 2017-05-01T14:25:29Z

src/macroexpand.scm

@@ -411,8 +412,10 @@
                 (m    (cdr form)))
             ;; m is the macro's def module
             (rename-symbolic-labels
-              (julia-expand-macros
+              ((lambda (ex) (julia-expand-macros-limited ex (- max-depth 1)))


This lambda is not necessary.

JeffBezanson · 2017-05-01T14:25:55Z

src/macroexpand.scm

-(define (julia-expand-macros e)
-  (cond ((not (pair? e))     e)
+(define (julia-expand-macros-limited e max-depth)
+  (cond ((eq? max-depth 0)   e)


Should use =.

JeffBezanson · 2017-05-01T14:33:05Z

Since we have both macro and function versions of macroexpand, if we add this we should probably have both here too.

StefanKarpinski · 2017-05-01T14:57:12Z

How about expressing this with a recur::Bool=true keyword instead?

jw3126 · 2017-05-01T16:22:32Z

@StefanKarpinski I though about this. Since recur is consistently spelled recursive in Base the choice is between @macroexpand1 ex and @macroexpand recursive=false ex.
But the latter is quite a mouthful, for quick REPL debugging. So I decided for the former.

alyst · 2017-05-01T17:39:36Z

One can also use depth::Bool = -1 (-1 for unlimited, 0 for no-op), depth=1 is shorter.

jw3126 · 2017-05-01T17:51:16Z

In principle one could have macroexpand(ex;depth::Int=-1) with depth=-1,2,3... all allowed. In femptolisp it is implemented like that. However I was not sure how to pass the depth parameter from julia to flisp.

vtjnash · 2017-05-01T18:38:12Z

Could we make macroexpand mean the "expand one macro" operations, and add a keyword all (fully?) to do the current behavior?

jw3126 · 2017-05-01T18:45:42Z

@vtjnash I like this idea, I guess the cases where you actually want to expand everything are library code, where typing some keyword does not hurt much. Of course this change is breaking in a subtle way.

jw3126 · 2017-05-05T22:25:17Z

So do you guys prefer @vtjnash idea of having only
macroexpand(code; recursive=false)
instead of
macroexpand, macroexpand1 ?
If so, how should the macroversion look like? Should it also take a keyword argument?

jw3126 · 2017-05-05T22:33:04Z

I think the usage pattern is as follows:

For REPL use, you want the macro and you want it non recursive.
For library use, you want the function and you want it recursive.

TotalVerb · 2017-05-06T01:18:14Z

What is the difference between this function and simply calling the macro itself (e.g. with getfield(current_module(), Symbol("@foo"))(...)?

vtjnash · 2017-05-06T03:24:44Z

In this case, both exist because a macro is a convenient way to quote an expression. In other cases, they differ in the post-processing done by a macro to apply hygiene. In fact, I was working on making an example of this today! The following diff should have no functional change (other than the catch_backtrace on error, given that I know @doc is docm): vtjnash@81a202c Enjoy! 😃

jw3126 · 2017-05-06T08:07:29Z

@TotalVerb I was not aware, that you could get a handle on a macro like that. Here is a concrete example of hygiene application:

julia> getfield(current_module(), Symbol("@show"))(:x)
quote 
    println("x = ", repr(begin  # show.jl, line 243:
                value = $(Expr(:escape, :x))
            end))
    value
end

julia> macroexpand1(:(@show x))
quote 
    (Base.println)("x = ", (Base.repr)(begin  # show.jl, line 243:
                #6#value = x
            end))
    #6#value
end

jw3126 · 2017-06-28T19:23:08Z

Sorry, something went terribly wrong during rebase.

tkelman · 2017-06-30T14:23:32Z

base/expr.jl


+* While [`macroexpand`](@ref) has an explicit `module` argument, `@macroexpand` always expands with respect to the module in which it is called. This is best seen in the following example:


wrap long lines

jw3126 · 2017-07-01T05:50:43Z

What are the CI fails about?

tkelman · 2017-07-01T10:09:00Z

doc/src/stdlib/base.md

@@ -275,6 +275,8 @@ Base.gc
 Base.gc_enable
 Base.macroexpand
 Base.@macroexpand
+Base.macroexpand1


the documentation fails to build because this doesn't exist any more

Thanks very much!

jw3126 · 2017-07-02T07:03:23Z

I think this PR is ready for merge.

KristofferC · 2017-07-12T17:56:05Z

Bump on review. @vtjnash?

vtjnash

Just some minor comments. This looks good – thanks for implementing it!

vtjnash · 2017-07-12T20:04:54Z

src/macroexpand.scm

-              (julia-expand-macros
-               (resolve-expansion-vars form m))))))
+              (julia-expand-macros-limited (resolve-expansion-vars form m) (- max-depth 1))
+               ))))


closing parens should be all on the same (previous) line

vtjnash · 2017-07-12T20:05:11Z

src/macroexpand.scm

        ((eq? (car e) 'module) e)
        (else
-         (map julia-expand-macros e))))
+         (map (lambda (ex) (julia-expand-macros-limited ex max-depth) ) e))))


no space before closing parens

vtjnash · 2017-07-12T20:11:10Z

src/macroexpand.scm

@@ -466,11 +466,12 @@

 ;; macro expander entry point

-(define (julia-expand-macros e)
-  (cond ((not (pair? e))     e)
+(define (julia-expand-macros-limited e max-depth)


I think it might be more straightforward to define this as a boolean with a default value:
(define (julia-expand-macros e (recursive #t))
I'm not sure there's going to be a use case for arbitrary depths, so it's nice just to keep the implementation simple.

I find the max depth implementation actually slightly simpler.

But I will try a default value (define (julia-expand-macros e (max-depth -1)). Did not expect this feature.

vtjnash · 2017-07-12T20:12:29Z

base/expr.jl


 Takes the expression `x` and returns an equivalent expression with all macros removed (expanded)
 for executing in module `m`.
+The `recursive` keyword controls whether deeper levels of nested macros are also expanded.
+This is demonstrated in the example below:
+```julia


I think this can be jldoctest, but would be good for someone else to confirm

It contains stuff like

:(#= REPL[16]:6 =# M.@m1)

that probably makes the doctest very fragile?

Should be julia-repl if it uses repl format but is not a doctest.

Thanks was not aware of julia-repl.

jw3126 · 2017-07-13T19:18:05Z

@vtjnash LGTY?

tkelman · 2017-07-14T01:48:32Z

does this really need to be exported?

vtjnash · 2017-07-14T02:08:25Z

We could move them to Base.Meta or interactiveutils.jl, but it would be really annoying for these not to be easily available at the REPL.

tkelman · 2017-07-14T04:33:06Z

how often is the non recursive version needed?

KristofferC · 2017-07-14T04:36:57Z

More often than the recursive?

`@macroexpand1` is a non recursive variant of `@macroexpand` The `macroexpand` function now also takes an keyword argument `recursive` to indicate whether to expand one level of macros or all macros recursively.

JeffBezanson reviewed May 1, 2017

View reviewed changes

jw3126 force-pushed the macroexpand1 branch from 9eceb96 to a8f43f8 Compare May 1, 2017 16:21

jw3126 closed this May 1, 2017

jw3126 reopened this May 1, 2017

jw3126 closed this May 5, 2017

jw3126 reopened this May 5, 2017

jw3126 force-pushed the macroexpand1 branch 3 times, most recently from c65d9fb to 7d99913 Compare June 30, 2017 05:15

tkelman reviewed Jun 30, 2017

View reviewed changes

jw3126 force-pushed the macroexpand1 branch from 9d1081c to 97a4968 Compare June 30, 2017 17:06

tkelman reviewed Jul 1, 2017

View reviewed changes

jw3126 added 4 commits July 12, 2017 21:09

add macroexpand1 function and macro

62ba9d0

fix

647d6d8

fix

7f0c59d

fix

b3ee2cb

jw3126 force-pushed the macroexpand1 branch from a450368 to b3ee2cb Compare July 12, 2017 19:10

vtjnash reviewed Jul 12, 2017

View reviewed changes

jw3126 added 2 commits July 12, 2017 22:29

fix

8bf9dfc

fix

c2c516d

vtjnash merged commit a7f24c4 into JuliaLang:master Jul 13, 2017

KristofferC mentioned this pull request Sep 10, 2017

non-recursive macroexpand #19365

Closed


		* While [`macroexpand`](@ref) has an explicit `module` argument, `@macroexpand` always expands with respect to the module in which it is called. This is best seen in the following example:

Macroexpand1 #21662

Macroexpand1 #21662

Conversation

jw3126 commented May 1, 2017

JeffBezanson May 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented May 1, 2017

StefanKarpinski commented May 1, 2017

jw3126 commented May 1, 2017

alyst commented May 1, 2017 • edited Loading

jw3126 commented May 1, 2017

vtjnash commented May 1, 2017

jw3126 commented May 1, 2017

jw3126 commented May 5, 2017 • edited Loading

jw3126 commented May 5, 2017

TotalVerb commented May 6, 2017

vtjnash commented May 6, 2017

jw3126 commented May 6, 2017

jw3126 commented Jun 28, 2017

Choose a reason for hiding this comment

jw3126 commented Jul 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jw3126 commented Jul 2, 2017

KristofferC commented Jul 12, 2017

vtjnash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jw3126 commented Jul 13, 2017

tkelman commented Jul 14, 2017

vtjnash commented Jul 14, 2017

tkelman commented Jul 14, 2017

KristofferC commented Jul 14, 2017

JeffBezanson May 1, 2017 •

edited

Loading

alyst commented May 1, 2017 •

edited

Loading

jw3126 commented May 5, 2017 •

edited

Loading