-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
pass macro call location to macro-function (rebase) #21746
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
@@ -1247,6 +1247,7 @@ export | |||
# 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
@@ -708,6 +729,32 @@ julia> foo() | |||
|
|||
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
@@ -1162,6 +1162,7 @@ | |||
(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