Skip to content
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

RFC: Improvement to hygienic macros #6910

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

david-moon
Copy link

Make macro hygiene easier to use and less error-prone by eliminating the esc function and making hygiene automatic instead. Details are in the commit log. Comes from a discussion on julia-users.

Risk: this broke the @printf macro because of its complex implementation. It was easy to fix, but I do not know if there are other macros that will break that are in packages not tested by make testall.

Possible future work: eval does not respect hygiene so only non-hygienic quotes must be used; this does not require any changes to existing code. It would be nice to fix that but it would be a lot of work.

Dave Moon added 3 commits May 20, 2014 16:17
PURPOSE:

The problem with Julia's current implementation of hygienic macros is
that it depends on the author of a macro to put calls to esc() in all
the right places.  Mistakes are hard to detect by testing because no
failure will occur until there is an accidental name clash.  Macros
are already hard to think about, no need to add unnecessary complications.

STRATEGY:

Deprecate esc() and make it not do anything.  Instead, quote marks all
symbols that appear directly in the quote, rather than being interpolated,
as hygienic.  Post-processing of the macro expansion will replace these
with the appropriate gensym, symbol, or module reference.

Make a distinction between hygienic and non-hygienic quote, because
quote is not only used by macros.  Only hygienic quote does the above.
Quote is hygienic when it is in the body of a macro or inside an
@hygienic annotation.  The output of hygienic quote will only work as
part of a macro expansion, it will not work with eval.

Unary : always behaves the same as quote.

As a special exception, quoting a single symbol never makes it
hygienic.  This seems to be the best compromise to maximize usability
without adding a new construct to reflect the fact that quoting a
single symbol is sometimes used to construct executable code and other
times used just to quote data.

Fix the few macros in the base code that were broken by these changes.

In the interest of simplicity, there are NO changes to the C code.

TESTING:

A clean build and make testall works on 32-bit Linux.

RISKS:

Did not test any packages that are not tested by make testall.
There could be complex macros out there that will stop working
because of these changes, analogous to @printf or @uv_write.

The distinction between hygienic and non-hygienic quote could be
confusing to users, although not as confusing as esc() was.

esc() is deprecated but not marked as such.

Calls to esc() are not removed from all over the source code.  That
could be cleaned up later.

Another problem is that the macro system has its own separate code
walker to decide where to insert gensyms for hygiene.  This creates
complexity and risk.  These changes are not fixing that, because to do
so would require changing much more of the Julia implementation to
allow symbols marked as hygienic to be used directly without
replacement by gensyms.  However, if we did those changes, eval()
would respect hygiene too and it would no longer be necessary to have
distinct hygienic and non-hygienic versions of quote.

PERFORMANCE IMPACT:

Using assoc instead of assq is slower, but it only affects the macro
expander.  There is also slightly more consing than there was before.
I doubt that the difference will be large enough for anyone to notice.

I did not give scm_to_julia() a special node type for hygienic symbol
markers.  I doubt that not optimizing that costs much performance.

PORTABILITY IMPACT:

None expected.

DOCUMENTATION IMPACT:

Remove documentation of esc() and all mention of it.

Document the distinction between hygienic and non-hygienic quote.

Document @Hygiene.

Document that $:x is a convenient way to insert a non-hygienic x
into a hygienic quote.

I will see if I can provide a suggested rewrite of the metaprogramming
chapter to reflect these changes and make it easier to understand.

All macro examples should be checked to remove esc and see if anything
needs to be changed to keep them working.

FILES CHANGED:

src/julia-syntax.scm - New design for hygienic macros:
- (hygienic <symbol>) represents an occurrence of <symbol> in
  a hygienic quote.  It will be replaced with the appropriate
  gensym, symbol, or module reference when the macro is expanded.
- allow hygienic symbols as argument names, type parameter names,
  variables in pair-with-gensyms, and keyword argument names.  Keywords
  cannot be hygienic because the same symbol is both the name used
  by the caller and the variable bound in the callee (same as before).
- Add an argument to expand-backquote() to tell it whether the quote
  is hygienic or not.  If hygienic, add hygienic markers to all
  symbols that are variable names, but don't add them to Expr heads.
  Don't use copyast of inert in hygienic mode since we need to add
  hygienic markers to symbols.
- Add an argument to julia-expand-macros-() to tell it whether quotes
  are hygienic or not.  Support the @hygienic macro which uses the
  new with_hygiene Expr head.
- The hygienic renaming env has to be searched with assoc rather than
  assq because uniqueness of (hygienic <symbol>) objects cannot
  easily be preserved through julia_to_scm() and scm_to_julia().
- resolve-expansion-vars-() only replaces symbols marked as hygienic
  by wrapping in (hygienic <symbol>).  Plain symbols must have come
  from an interpolation of code supplied by a macro argument.

base/LineEdit.jl - The bindings introduced by @keymap are supposed
to be non-hygienic, make it so.

base/REPLCompletions.jl - There is no longer an escape Expr, remove
the check for it so the code will still work.

base/base.jl - Neuter esc() but for convenience don't remove it yet.
It's deprecated.

base/expr.jl - Add @hygienic macro which makes any quotes in its
body provide hygiene.  This means those quoted expressions will
only work as part of a macro expansion, they will not work with eval.

base/printf.jl - Add @hygienic to all subroutines of the @printf and
@sprintf macros so those macros will still be hygienic.  Fix code in
gen_d() to work around the fact that quote of just a symbol is no
longer hygienic; the replacement code is clearer to read anyway.
Fix @sprintf to use an explicit gensym to work around the same issue.

base/process.jl - @setup_stdio and @cleanup_stdio are supposed to
introduce non-hygienic bindings.

base/stream.jl - @uv_write is supposed to introduce a non-hygienic
binding of uvw.
…julia into moon/hygienic-macros

Merged with git pull because git push asked me to do so.
@kmsquire
Copy link
Member

This is probably one of the most professional pull requests Julia has seen (although judging from your background, David, that is not surprising). Thanks for making thing so clear.

@@ -157,7 +157,8 @@ function precompile(f, args::Tuple)
end
end

esc(e::ANY) = Expr(:escape, e)
# Neuter esc but for convenience don't remove it yet - it's deprecated
esc(e::ANY) = e
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the @deprecate macro to print a deprecation warning. The deprecation declarations are consolidated into the base/deprecated.jl file to make them easy to find.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On May 21, 2014, at 3:36 PM, pao wrote:

You can use the @deprecate macro to print a deprecation warning.
Probably better to use the terser, "Returns its argument."

Good suggestions. I will make these updates after a day or two to
let others weigh in.

On May 21, 2014, at 3:30 PM, Kevin Squire wrote:

Thanks for making thing so clear.

I don't see anything to be gained by obfuscation! I used
approximately the same form of commit log that we used to use at
Progress Software.

--Dave Moon

@kmsquire
Copy link
Member

I don't see anything to be gained by obfuscation! I used approximately the same form of commit log that we used to use at Progress Software.

:-) I was really just appreciating that you're taking a relatively deep and complicated change and making it accessible. I find it nice to see examples like this.

BTW, I don't want to imply that other PRs necessarily need to have this much detail. Most PRs are not this deep, and there's also something to be said for being agile, especially at this point in Julia's development.

(Sorry to distract from the more important discussion of the content of this PR. Cheers!)

@JeffBezanson
Copy link
Member

Very impressive. Great to have a PR from you, David!

A mundane matter: you seem to have one of the unlucky versions of sphinx that inserts an unwanted extra newline in every helpdb entry. You can roll back that part of the change and let somebody else update the helpdb.

@david-moon
Copy link
Author

On May 21, 2014, at 4:51 PM, Jeff Bezanson wrote:

A mundane matter: you seem to have one of the unlucky versions of
sphinx that inserts an unwanted extra newline in every helpdb
entry. You can roll back that part of the change and let somebody
else update the helpdb.

I wouldn't be pushing the updated helpdb into git, would I? So only
I would be affected by the Sphinx bug, I think.

@JeffBezanson
Copy link
Member

Yes, helpdb.jl is in the git repo as a normal source file.

@pao
Copy link
Member

pao commented May 21, 2014

I wouldn't be pushing the updated helpdb into git, would I?

To limit the number of dependencies required to build a complete Julia including the basic help() function, helpdb.jl is actually checked in. If you look at your pull request in the GitHub interface, you'll see that the file is changed, though due to the size of the diff the diff is suppressed.

To roll it back you can:

git checkout master doc/helpdb.jl
git add doc/helpdb.jl
git commit #etc...

@david-moon
Copy link
Author

On May 21, 2014, at 5:04 PM, pao wrote:

I wouldn't be pushing the updated helpdb into git, would I?

To limit the number of dependencies required to build a complete
Julia including the basic help() function, helpdb.jl is actually
checked in. If you look at your pull request in the GitHub
interface, you'll see that the file is changed, though due to the
size of the diff the diff is suppressed.

To roll it back you can:

git checkout master doc/helpdb.jl git add doc/helpdb.jl git commit
#etc...

Thanks, I get it now.

@mbauman
Copy link
Member

mbauman commented May 21, 2014

I think this could be wonderfully amazing. I've not tried it yet, but do I understand correctly that with this PR the only way to escape an expression is to interpolate it into a quote within a hygienic environment? Some of the syntaxes involving splatting (especially when defining function arguments) can be difficult to get correct in the context of interpolation and are sometimes much easier to manually build with Expr. Without the need for esc I think some of that could become much easier. I'll have to go back and look through the cases where I've manually constructed Exprs and see if it's no longer necessary with this change.

One set of untested macros (for which I'm partially responsible - I should have added tests!) which do this kind of Expr construction is @which and friends, defined at interactiveutil.jl:198-249.

@david-moon
Copy link
Author

On May 21, 2014, at 5:33 PM, Matt Bauman wrote:

I think this could be wonderfully amazing. I've not tried it yet,
but do I understand correctly that with this PR the only way to
escape an expression is to interpolate it into a quote within a
hygienic environment?

That's kind of backwards. There is no longer any need to escape
anything explicitly. Better you should say "The only way not to
escape an expression is to write it directly in a quote within a
hygienic environment."

Some of the syntaxes involving splatting (especially when defining
function arguments) can be difficult to get correct in the context
of interpolation and are sometimes much easier to manually build
with Expr. Without the need for esc I think some of that could
become much easier. I'll have to go back and look through the cases
where I've manually constructed Exprs and see if it's no longer
necessary with this change.

I don't think it makes a difference whether you use Expr or $ to
interpolate stuff. Either way it will be transparent.

You just need to use quote for anything that you want to be hygienic
(you could actually do it with Expr but I did not document how to do
that, and anyway it might change.)

One set of untested macros (for which I'm partially responsible - I
should have added tests!) which do this kind of Expr construction
is @which and friends, defined at interactiveutil.jl:198-249.

I looked at this. I guess you are referring to the code where you do:

for fname in
[:which, :less, :edit, :code_typed, :code_lowered, :code_llvm, :code_nat
ive]
@eval begin
macro ($fname)(ex0)
gen_call_with_extracted_types($(Expr(:quote,fname)), ex0)
end
end
end

I think you want fname to be hygienic here so its value gets
interpreted in the correct module. I think it is hygienic before my
change but not after my change. Of course you could just use Base.
$fname and not rely on macro hygiene. Or you could change it to:

     macro ($fname)(ex0)
         gen_call_with_extracted_types(quote $fname end, ex0)
     end

which should work both before and after my change. If this doesn't
work there is something wrong with the implementation of quote. Note
that the dollar signs belong to an outer @eval, so the first argument
to gen_call_with_extracted_types is actually something like quote
which end.

What you have here is essentially a macro-defining macro, although
you are using @eval instead of an outer macro. Macro-defining macros
are always a good test of the power and hygiene of macro facilities.
I haven't tested it but I think you could use something like:

macro defutils(macronames...)
quote begin
$(map(name->quote
macro $name(ex0)
gen_call_with_extracted_types(quote $name end, ex0)
end,
macronames))...
end # begin
end # quote
end # macro

@defutils(which, less, edit, code_typed, code_lowered, code_llvm,
code_native)

I do not claim this is better than the @eval approach you are using.
I am mostly just using it to show an example of a macro-defining macro.

gen_call_with_extracted_types also needs to make typesof and error
hygienic, or explicitly specify their module, to not get broken by my
change. For example, change

     return Expr(:call, fcn, esc(ex0.args[1]),
                 Expr(:call, :typesof, map(esc, ex0.args 

[2:end])...))

to

    @hygienic :($fcn($(ex0.args[1]), typesof($(ex0.args 

[2:end])...)))

Or you could change :typesof to @hygienic quote typesof end, but
using quote for the whole thing and abandoning the explicit Expr
calls looks easier.

I probably should have tested this before sending the email since it
is so easy to get confused by macros or splat or both.

Do I really have to search the code for every call to Expr and see if
it is affected by my change? That would be the safest way to
evaluate the risk.

Another good thing I should test is how a macro-defining macro can
generate a macro with quote interpolation in it. I think you need to
use something like $:$ to include a $ in the generated macro. This
depends on $ being a keyword so hygiene doesn't matter.

--Dave Moon

@simonster
Copy link
Member

(This is off-topic, but when you type macro names without escaping them in backticks, it pings the corresponding GitHub users on the discussion thread, although I suspect @eval and @which have already had enough of us and disabled notifications for this repository :-). You may have to post from the website for the backticks to have an effect; I tried to edit this post to add them and they didn't seem to parse.)

@david-moon
Copy link
Author

Github is a pain. I wish it had shown the special treatment of atsigns when I did Preview. Although my most recent posting was sent by email rather than from the github web site so I didn't preview it.

I'll try not to make the same mistake again.

--Dave Moon

@david-moon
Copy link
Author

More response to Matt Bauman:

I did some experimentation with macro-defining macros, but it's hopeless because as far as I can tell a quote cannot expand into a quote. I made some attempts at fixing this but they broke other things. It's clearly time to back off from trying to make Julia macros work well enough to support macro-defining macros.

I will write a test for hygiene in atsign-which and friends and update them to pass that test as part of my changes. I will update my pull request today or tomorrow with these changes and the other changes suggested earlier in this conversation.

Thanks for all the review, this is a very welcoming community.

@tknopp
Copy link
Contributor

tknopp commented May 22, 2014

Thanks for all the review, this is a very welcoming community.

Well thats the trick. Be nice to new contributors and you will gain wonderful pull requests :-)

@pao
Copy link
Member

pao commented May 22, 2014

atsign-which

Best workaround yet!

@timholy
Copy link
Member

timholy commented May 22, 2014

This is indeed an example of a great PR. I haven't reviewed it in detail yet, but I have to say that first impression suggests this will be easier to just "do the right thing" than sprinkling esc all over.

I have to confess a certain amount of shock that you didn't have to modify any of the macros or helper-functions in cartesian.jl to get make testall to pass.

@mbauman
Copy link
Member

mbauman commented May 22, 2014

Wow, thank you David. What a thorough and well-considered response. I understand much better now, and after playing with this I like it even more. Admittedly, the @which-and-friends macros were one of the first things I played with upon learning Julia. I think I'd write them differently today, and they can be so much simpler with this patch. Indeed, I think we can make this work with this PR without any changes to the hygienics.

Here's a basic proof-of-concept for getting this kind of thing to work with the PR as-is:

module M

@hygienic function gen_call_with_extracted_types(fcn, ex0)
    # don't worry about more complicated expressions for now, assume direct :call 
    return :(($(fcn))($(ex0.args[1]), Base.typesof($(ex0.args[2:end]...))))
end

for fname in [:which, :less, :edit, :code_typed, :code_lowered, :code_llvm, :code_native]
    @eval begin
        macro ($fname)(ex0)
            gen_call_with_extracted_types($fname, ex0)
        end
    end
end

end

And then the macro seems to be using the correct function and escaping the args automatically:

julia> which = +
       less  = 1
       M.@which which(less,2)
+(x::Int64,y::Int64) at int.jl:33

Since I'm passing the function itself, it gets resolved in the context of the @eval block. Am I missing anything here? I had also been worried about splatting, but it was the calls to esc that were making that difficult. It works wonderfully here.

@david-moon
Copy link
Author

To Matt Bauman: I think there are still some small problems in what you sent, based on what I have been doing today with writing tests for @which and fixing the problems uncovered. You sure opened up a can of worms, but better to discover that now than later!

When I update the pull request today or tomorrow you'll be able to see what I did.

--Dave Moon

@david-moon
Copy link
Author

Tim Holy writes:

I have to confess a certain amount of shock that you didn't have to modify any
of the macros or helper-functions in cartesian.jl to get make testall to pass.

Maybe because those macros do their best to turn off all hygiene? Throwing in esc() and Expr(:escape, ...) all over the place no longer does anything after my changes, but there probably are not any unit tests to verify that the hygiene does not work. I am not too surprised that cases that worked before still work. Those macros look like they could use some cleaning up, but I do not want to take that on.

--Dave Moon

Dave Moon added 5 commits May 22, 2014 15:29
Specifically:

gen_call_with_extracted_types() is declared hygienic, and uses quote
instead of manually constructing Expr's so the result will be hygienic
as it was before the new hygienic macros.

Fix it so "@which(a[i]=x)" will work.

Fix the too-complex error message not to assume @which is the caller.

Gratuitous rearrangement of code in the for fname loop.

Also tests have been added to make testall to verify all this.  The
tests failed with the new auto-hygienic macros before these fixes
were put in.
@david-moon
Copy link
Author

pao suggested using the atsign-deprecate macro with esc. After looking at what it does, I don't think we want to do that yet. A huge amount of warnings would be generated. I will leave it to someone with better Julia taste than me to decide when is the right time to do that.

@stevengj
Copy link
Member

Needs a rebase to be considered for merging? What is the @printf status?

@david-moon
Copy link
Author

On Dec 30, 2014, at 3:55 PM, Steven G. Johnson wrote:

Thanks for bumping this.

Needs a rebase to be considered for merging?

Surely. I have not worked on Julia since early last year.

Unfortunately, the hygienic macros work would probably benefit from
some attention from a very senior Julia developer. I don't think I
did the interface between the C and Lisp parts of the Julia parser in
the "right" way, although the way I did it does work. I would have
been happier if hygienic symbol objects maintained their identity
when transmitted across that interface and so could be compared with
eq rather than equal, but I could not figure out how to do that.
Maybe that's why it has not been put in.

What is the @printf status?

I don't know anything about that.

--Dave Moon

@vtjnash
Copy link
Member

vtjnash commented Apr 22, 2015

i've rebased this and got nearly everything working, except for one of the @goto tests:

julia/test/goto.jl

Lines 90 to 101 in 86ba78f

module GotoMacroTest
macro goto_test8_macro()
quote
function $(esc(:goto_test8))()
@label a
@goto a
end
end
end
end
GotoMacroTest.@goto_test8_macro

the problem there is that a is no longer just a symbol, but a variable name and will be "properly" sanitized before getting to macro goto. one option is to require the user to write @goto $:a. another option is to have @goto remove any Expr(:., :Main, QuoteNode(:a)) wrappers that it encounters (and accept that it may have been converted to a gensym if a was also a local variable)

thoughts?

@david-moon
Copy link
Author

david-moon commented Apr 22, 2015 via email

@vtjnash
Copy link
Member

vtjnash commented Apr 22, 2015

no, i left that as a possible future optimization

@vtjnash
Copy link
Member

vtjnash commented Apr 22, 2015

moved to #10940

@stevengj
Copy link
Member

@vtjnash says that resurrecting this for Julia 2.0 can just as easily start from this PR as from #10940.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.