RFC: rewrite_sourceloc! function #22623

c42f · 2017-06-29T23:25:37Z

A tool to propagate the implicit __source__ variable from a macro to any
macro calls in the generated AST.

This automates @vtjnash's trick from #21746 (comment) and makes it accessible to people who want to do the common task of making uses of @__LINE__ inside a macro expand to the location of the macro caller, rather than a location inside the macro. (The trick is simple to use directly in some cases, but this tool makes it a lot easier to use if @__LINE__ is deeply embeeded in the AST.)

An obvious question is whether this should propagate into nested macro calls when processing the AST. For now, I've not done this but perhaps I need to think a bit more about that.

c42f · 2017-06-30T01:01:36Z

base/loading.jl

+function propagate_sourceloc(sourceloc, ex)
+    if ex isa Expr
+        if ex.head == :macrocall
+            ex.args[2] = sourceloc


Thinking about it, I'd better recurse into this as well.

vtjnash · 2017-06-30T02:17:29Z

base/loading.jl

+"""
+macro propagate_sourceloc(ex)
+    :(propagate_sourceloc($(esc(:__source__)), $(esc(ex))))
+end


This macro feels unnecessary to me - using the function form alone seems cleaner.

Sure. How will we deal with 0.6 compatibility?

For context, I ported this utility from here: https://github.com/c42f/MicroLogging.jl/blob/e37dffcb7aeb9c581a27b897fa9d4fe3467d8b85/src/util.jl#L34 where I'm trying to support both 0.6 and 0.7; in 0.6 the __source__ variable isn't present which is the reason for the macro.

What's the sanest way to deal with getting a nop version of this into Compat? I could add something to make the following work in 0.6, by just removing the function call. A bit messy!

@compat propagate_sourceloc(__source__, quote @__LINE__ end)

Alternatively I could just move the @propagate_sourceloc macro to Compat...

how about:

code = quote end Compat.macros_have_sourceloc && propagate_sourceloc!(__source__, code) return code ```

Ok then. The few people who care about this (myself and two others?) can delete the small amount of mess once 0.7 is out - no need to make it super complicated.

c42f · 2017-06-30T22:34:41Z

I changed some things as suggested and added some documentation.

tkelman · 2017-07-01T00:06:55Z

doc/src/stdlib/file.md

@@ -50,6 +50,7 @@ Base.Filesystem.basename
 Base.@__FILE__
 Base.@__DIR__
 Base.@__LINE__
+Base.propagate_sourceloc!


doesn't seem like the right manual section for this?

Correct, however this is most related to @__LINE__, @__FILE__ and @__DIR__. Perhaps all these should be moved to another location. The location in the manual ("metaprogramming") seems much more sensible for these things, as they're all about introspection of the macro expansion process.

How about moving all four of these to "doc/src/stdlib/base.md"?

I've moved these around - hopefully into a more coherent location in the stdlib docs.

tkelman · 2017-07-01T00:07:49Z

base/exports.jl

@@ -1214,6 +1214,7 @@ export
    @__FILE__,
    @__DIR__,
    @__LINE__,
+    propagate_sourceloc!,


does this need to be exported?

Good question. That depends on whether there's a distinction between "public API" and "exported function". I want this as a tool which can be used by packages, but it's also relatively obscure. So if there's a well defined category "public but not exported" this would fit in there, and we could remove the export.

There are some things that are public but not exported, usually it's whether they are documented in a public way as if to be used qualified. Most of those are under more specific modules than Base though, like Profile. Could maybe move this to the Meta module?

Ok, let's try this and see how it looks.

cool, so if it's in Meta it probably shouldn't be re-exported from Base too

Ah yes. I could have sworn I fixed that.

tkelman · 2017-07-01T10:13:11Z

doc/src/manual/metaprogramming.md

+julia> dump(@__LOCATION__())
+Tuple{Int64,String}
+  1: Int64 1
+  2: String "REPL[2]"


will this pass when you run the doctests?

It will now ;-)

c42f · 2017-07-01T11:48:09Z

Now moved to Base.Meta and exported from there. There seemed to be a couple of useful exported but non documented functions in Base.Meta so I had a go at documenting them while I was at it.

c42f · 2017-07-02T02:26:41Z

Looks like the travis failure is a timeout.

ihnorton · 2017-07-02T03:49:45Z

Why base?

ihnorton · 2017-07-02T04:11:49Z

base/meta.jl

+macro example()
+    propagate_sourceloc!(__source__,
+        quote
+            @__LINE__


I think I'm not understanding the purpose of this example, so I'm wary of, erhm, propagating it... If it's trying to solve c42f/MicroLogging.jl@f6db371#commitcomment-20846418 then why not interpolate __source__.line? @__LINE__ is documented and implemented as an almost-literal (that's why we need the parser to insert it the value in the macrocall), so I think having this would be confusing.

This is somewhat related to that old comment, but addresses the more general problem of wrapping macros which make use of __source__, for example here:

https://github.com/c42f/MicroLogging.jl/blob/e37dffcb7aeb9c581a27b897fa9d4fe3467d8b85/src/MicroLogging.jl#L205

If it were just @__LINE__ I was worried about, interpolating __source__.line would be a better solution.

That is, if package A provides the macro

macro a() quote some_complicated_expression($(__source__.line)) end end

And package B wishes to wrap it -

macro b() quote # ... @a end end

while preserving reporting of the source location, you either need a tool a bit like this, or to manually dig into the AST generated in @b and rewrite the source location of the macro invocations contained within.

c42f · 2017-07-02T04:15:34Z

Why base?

Because without this small tool, using @__LINE__ and @__FILE__ inside a macro expands to the location inside the macro, which is almost never what you want. So in this sense, it belongs in the standard library, to make some other core features of Base usable.

ihnorton · 2017-07-05T14:55:36Z

Someone else should probably comment/approve. My thoughts are:

not clear on why this needs to be recursive vs. a single-shot forwarding call like in the discussion thread with Jameson.
I would prefer not exporting, and removing the example, at least from the narrative Metaprogramming section. Macros-calling-macros is fairly advanced and I don't think that such an example should be in the intro docs (at some point we may need to split some content off to an "Advanced Metaprogramming" section). Many issues such as this can be avoided by making the code generator a function.

ihnorton · 2017-07-05T14:55:58Z

It would be great to find a more general way to handle this issue, e.g. during expansion, but given that AFAICT Clojure hasn't solved it either, I suspect it will be challenging.

Also closely related: http://tratt.net/laurie/blog/entries/debugging_layers

c42f · 2017-11-28T06:20:33Z

I've just wanted this again, so I've had occasion to come back to this and clean it up as suggested (the function is exported by Meta, but not by Base, so I've left that alone).

As to whether this needs to be recursive - well obviously it doesn't, but it's a lot nicer for it to be recursive when the macro call or calls are nested deeply in the AST.

I fully agree that having shared functions which generate Exprs for use in multiple macros is generally nicer. This is a good idea if you have control over both macros. But when the called macro is from a separate module this isn't so easy.

StefanKarpinski · 2017-11-28T15:42:42Z

The functionality 👍, that name though... Of course, I don't have any better ideas.

c42f · 2017-11-29T01:16:25Z

Yes, the name is now quite a lot worse than it was when this PR was first submitted. I think I'll change the name to replace_sourceloc! instead which is a lot more descriptive.

(Regarding length, both names are long but I think that's desirable for obscure and little used functionality.)

c42f · 2017-11-29T01:41:17Z

Renamed. I had a think, but I'm all out of names which describe this operation clearly but succinctly. replace_sourceloc is at least consistent with Compat.macros_have_sourceloc.

c42f · 2017-11-29T11:10:18Z

I assume the OSX Travis failures are spurious. It's hard to imagine how they could be related to the changes here.

KristofferC · 2017-11-29T11:22:04Z

Could this be used to fix the following:

julia> @test_nowarn warn("fds")
Test Failed at /Users/kristoffer/julia/usr/share/julia/site/v0.7/Test/src/Test.jl:521

Here the source location info from the @test that is wrapped inside the @test_nowarn macro and we thus print the "wrong" location. We really want the location where @test_nowarn was expanded.

c42f · 2017-11-29T11:55:04Z

Yes, this is exactly what this function is for.

(Though I'm just about to make @test_warn obsolete, I hope!)

vtjnash · 2020-10-27T16:37:51Z

Anyone remember: why didn't we ever merge this?

A tool to propagate the __source__ variable from a macro to any macro calls in a given AST such that the inner macro calls are attributed the same line and file name.

Also find an improved location for __FILE__, __DIR__ and __LINE__ in the manual.

c42f · 2020-10-30T06:47:20Z

I don't know why this wasn't merged; I think mainly the discussion went on long enough that I eventually forgot about it.

Thanks for rebasing. I still use this trick occasionally, I just tend to write it out by hand.

c42f mentioned this pull request Jun 29, 2017

Line number logging with __LINE__ doesn't work c42f/MicroLogging.jl#1

Closed

ararslan requested review from vtjnash and yuyichao June 30, 2017 00:57

ararslan added the macros @macros label Jun 30, 2017

c42f commented Jun 30, 2017

View reviewed changes

vtjnash reviewed Jun 30, 2017

View reviewed changes

c42f force-pushed the propagate-sourceloc branch from 838a519 to 213ab34 Compare June 30, 2017 21:38

tkelman reviewed Jul 1, 2017

View reviewed changes

c42f force-pushed the propagate-sourceloc branch from a1abd4b to bd28452 Compare July 1, 2017 12:19

ihnorton reviewed Jul 2, 2017

View reviewed changes

c42f force-pushed the propagate-sourceloc branch from bd28452 to 78d1870 Compare November 28, 2017 06:06

c42f changed the title ~~RFC: propagate_sourceloc macro to expand __LINE__ in caller context~~ RFC: propagate_sourceloc function Nov 28, 2017

c42f force-pushed the propagate-sourceloc branch from 78d1870 to d97df51 Compare November 29, 2017 01:36

c42f changed the title ~~RFC: propagate_sourceloc function~~ RFC: rewrite_sourceloc function Nov 29, 2017

c42f changed the title ~~RFC: rewrite_sourceloc function~~ RFC: rewrite_sourceloc! function Nov 29, 2017

vtjnash added the forget me not PRs that one wants to make sure aren't forgotten label Oct 27, 2020

c42f added 2 commits October 29, 2020 14:02

Add replace_sourceloc!() to rewrite macro __source__

7123e7b

A tool to propagate the __source__ variable from a macro to any macro calls in a given AST such that the inner macro calls are attributed the same line and file name.

Documentation for quot() and isexpr()

7a6495c

Also find an improved location for __FILE__, __DIR__ and __LINE__ in the manual.

vtjnash force-pushed the propagate-sourceloc branch from d97df51 to 7a6495c Compare October 29, 2020 18:14

vtjnash merged commit 10b0a01 into JuliaLang:master Oct 30, 2020

vtjnash mentioned this pull request May 29, 2021

Document renumbering lines in macros #40955

Closed

simeonschaub removed the forget me not PRs that one wants to make sure aren't forgotten label May 29, 2021

RFC: rewrite_sourceloc! function #22623

RFC: rewrite_sourceloc! function #22623

Conversation

c42f commented Jun 29, 2017

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

c42f commented Jun 30, 2017

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

c42f Jul 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

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

c42f commented Jul 1, 2017

c42f commented Jul 2, 2017

ihnorton commented Jul 2, 2017

ihnorton Jul 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c42f commented Jul 2, 2017

ihnorton commented Jul 5, 2017

ihnorton commented Jul 5, 2017 • edited Loading

c42f commented Nov 28, 2017

StefanKarpinski commented Nov 28, 2017

c42f commented Nov 29, 2017

c42f commented Nov 29, 2017

c42f commented Nov 29, 2017

KristofferC commented Nov 29, 2017 • edited Loading

c42f commented Nov 29, 2017

vtjnash commented Oct 27, 2020

c42f commented Oct 30, 2020

c42f Jul 1, 2017 •

edited

Loading

ihnorton Jul 2, 2017 •

edited

Loading

ihnorton commented Jul 5, 2017 •

edited

Loading

KristofferC commented Nov 29, 2017 •

edited

Loading