-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
pass macro call location to macro-function (rebase) #21746
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
Conversation
7aac52f to
64a60be
Compare
|
I can haz column info as well? |
|
The parser doesn't have any, afaik. I think getting line number ranges would be the next step. |
|
#9579 has code for column tracking in the parser. I don't really care about line ranges since that's easy to compute for string macros. |
|
Would you recommend calling this |
|
That sounds like a good idea. |
|
This breaks Documenter because it tries to pattern-match the macrocall expression to see if it has zero-arguments or no arguments specified, and will need the following patch: diff --git a/src/DocSystem.jl b/src/DocSystem.jl
index e3b89c3f..333feabe 100644
--- a/src/DocSystem.jl
+++ b/src/DocSystem.jl
@@ -88,7 +88,7 @@ binding(m::Module, λ::Any) = binding(λ)
function signature(x, str::AbstractString)
ts = Base.Docs.signature(x)
- (Meta.isexpr(x, :macrocall, 1) && !endswith(strip(str), "()")) ? :(Union{}) : ts
+ (Meta.isexpr(x, :macrocall, 2) && !endswith(strip(str), "()")) ? :(Union{}) : ts
end
if VERSION < v"0.5.0-dev"
Base.Docs.signature(::Any) = :(Union{})
diff --git a/src/Utilities/Utilities.jl b/src/Utilities/Utilities.jl
index c4ce0a28..6c477443 100644
--- a/src/Utilities/Utilities.jl
+++ b/src/Utilities/Utilities.jl
@@ -238,7 +238,7 @@ Returns a expression that, when evaluated, returns an [`Object`](@ref) represent
function object(ex::Union{Symbol, Expr}, str::AbstractString)
binding = Expr(:call, Binding, splitexpr(Docs.namify(ex))...)
signature = Base.Docs.signature(ex)
- isexpr(ex, :macrocall, 1) && !endswith(str, "()") && (signature = :(Union{}))
+ isexpr(ex, :macrocall, 2) && !endswith(str, "()") && (signature = :(Union{}))
Expr(:call, Object, binding, signature)
end
@@ -275,7 +275,7 @@ if VERSION < v"0.5-"
end
else
function docs(ex::Union{Symbol, Expr}, str::AbstractString)
- isexpr(ex, :macrocall, 1) && !endswith(rstrip(str), "()") && (ex = quot(ex))
+ isexpr(ex, :macrocall, 2) && !endswith(rstrip(str), "()") && (ex = quot(ex))
:(Base.Docs.@doc $ex)
end
end |
64a60be to
10afb8f
Compare
10afb8f to
1796a25
Compare
|
Does this need devdocs updates for AST changes? |
8c00bc2 to
1a42609
Compare
| # parser internal | ||
| @__FILE__, | ||
| @__DIR__, | ||
| @__LINE__, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs to be added to stdlib doc index to be included in the rendered docs
49686d3 to
eb75d1f
Compare
Compat flag for adding source location argument to macros (JuliaLang/julia#21746)
JuliaLang/julia#21746 (cherry picked from commit 27e8059)
eb75d1f to
93c1f33
Compare
eaeda8b to
4269a3e
Compare
doc/src/manual/metaprogramming.md
Outdated
|
|
||
| This kind of manipulation of variables should be used judiciously, but is occasionally quite handy. | ||
|
|
||
| Getting the hygiene rules correct can be a formidable challenge, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
end sentence with a period not a comma
4269a3e to
54a185b
Compare
|
For users needing compatible parsing, note that |
NEWS.md
Outdated
| rather than from the task-local `SOURCE_PATH` global when it was expanded. | ||
|
|
||
| * All macros receive an extra argument `__source__::LineNumberNode` which describes the | ||
| parser location in the source file for the `@` of the the macro call. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicated "the"
…ll macros also emit an explicit push_loc in @generated functions rather than depending on the existence of a LineNumberNode and other lowering heuristics to produce it
af3ab07 to
6b05e37
Compare
| (error "macros cannot accept keyword arguments")) | ||
| (expand-forms | ||
| `(function (call ,(symbol (string #\@ (cadr (cadr e)))) | ||
| (|::| __source__ (core LineNumberNode)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it's suddenly clear how this approach could work at all. Very clever!
| end | ||
| @test (@emit_LINE) == ((@__LINE__) - 3, @__LINE__) | ||
| @test @nested_LINE_expansion() == ((@__LINE__() - 4, @__LINE__() - 12), @__LINE__()) | ||
| @test @nested_LINE_expansion2() == ((@__LINE__() - 5, @__LINE__() - 9), @__LINE__()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vtjnash @ihnorton with this approach, it looks like there's no way to create a wrapper macro which should be transparent to @LINE, in the same way that non-block quote doesn't generate normal line number nodes.
In MicroLogging I've got @info, @warn implemented as trivial wrappers around an @logmsg macro which has all the guts of the logging implementation, and I was hoping to have @LINE expanded inside @logmsg. Of course, for the particular purposes of my package I can work around this limitation, but I'd rather not have to.
Do you see a viable way to allow such wrapper macros using the __source__ approach you've implemented here?
One probably ugly possibility would be to have access to a Expr(:macrocall_with_source, ...) (better name needed!) where the user can pass the __source__ argument explicitly, bypassing the expansion step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From inside one macro, I think both
return Expr(:macrocall, Symbol("@macroname"), __source__, args...)
or equivalently
m = :(@macroname args...); m.args[1] = __source__; return m
should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, of course. Thanks - this does make a lot of sense. (The reason I ask is that I'm having odd LLVM compile errors trying to build julia-0.7 right now, so I haven't been able to just try the code myself.)
This seems to be both the benefit and downside of the solution here: a benefit, because the __source__ should be clearly visible and non-magical in the AST. This is great. A downside, because it'll disturb any code which processes Expr(:macrocall, ...) (which quite honestly probably isn't a whole lot of user code).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Expr(:macrocall, ...) change broke multiple packages on nightly. A deprecation should really be put in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah, wouldn't have been a problem with #20895 :-P
How could a deprecation be made to work? I assume this broke packages which were doing moderately "interesting" AST manipulations. Those would just be manipulating the surface syntax via an Expr which happened to have head==:macrocall so there's nowhere to insert any kind of depwarn, is there?
I could imagine adding some kind of AST pattern matching tool inside Compat, and encouraging people to use that when interacting with ASTs containing macros? Could be a lot easier on users than the minimal flag in https://github.com/JuliaLang/Compat.jl/pull/355/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there've been a few similar deprecations put in at the lowering level for macro writers who construct obsolete AST forms, instead of hard-breaking them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so you're referring to packages which construct an Expr(:macrocall, args...) explicitly? In that case, would it be sufficient to detect when the first argument isn't a line number node, and add an extra dummy __source__ argument just before calling invoke-julia-macro?
There's another case which could cause breakage, as @a 1 now parses to include the value of the __source__ explicitly in the AST. Packages which attempt to process this AST into something else are presumably going to be surprised, when the Expr ends up with 1 in the args[3] slot rather than args[2]?
rebase of #13339
fixes #9577
replaces #9579
replaces #20895