lowering: track location of macros better for stackwalk

[vtjnash]

Noticed since scrub_backtrace is looking for a macro expansion when
printing an exception thrown in @test, but that was missing for simple
expressions, particularly those at the top-level or spliced in.

Fix #44757

[KristofferC]

Does this have any relation to #44551?

[vtjnash]

No, I would need to change this to (push_loc f "macro expansion"), like we do for generated functions, to handle that case

[JeffBezanson]

I would rather try to handle this elsewhere. e.g. wrapping top-level expressions in blocks with line numbers in the parser, instead of putting it inside the macro expansion code.

[timholy]

Given that this came up in discussions of #40537, what's the status here? Was #44995 (comment) addressed and it's just waiting for more review?

[vtjnash]

I didn't think that would be possible to address, without notable breakage, as it generally is a proposal to change the parsing of any macro call @foo to Expr(:begin, Expr(:line, ...), Expr(:macrocall, :@foo)) from Expr(:macrocall, :@foo, Expr(:line))

The reason is that many expressions will get to the runtime with the form @eval @test foo. So since @test was not initially appearing inside a block with outer source location (the only source location is the macro, and the implied node that we forward as the __source__ argument), thus we currently expect that it will expand to an expression which does not have source location. We can attempt to enforce either on the macroexpansion that it attempt to preserve the __source__ location argument of many macrocalls, or on the parser, that it tries to tag everything with a source location. But right now, our parser (somewhat intentionally) is very bad at annotating locations of simple independent expressions, and we are not particularly reliable about handing that information around unaided for those cases, as can be seen from Meta.@lower which is missing the REPL[1]:1 line info.

julia> Meta.@lower @test x
:($(Expr(:thunk, CodeInfo(
     @ /data/vtjnash/julia/usr/share/julia/stdlib/v1.10/Test/src/Test.jl:474 within `top-level scope`

Usually, backtraces are okay because the function containing the statements has most of the nearby line numbers marked in the AST nearby, so it does pretty well at getting them right if they are embedded into any sort of block. So this PR tries to add a block in places where we didn't have one.

[vtjnash]

This is now my 4th attempt at this, haha. The main difference is that it is now a little better integrated as part of parsing / lowering and I think that should be a bit more reliable. I think this should happen to also be compatible with #6910, which is a significant advantage too. I would include more bits of that since I have them, but plan to do that in a later PR instead, once I have time to update that content.

@nanosoldier runtests()

[nanosoldier]

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

[vtjnash]

Some notable issues:

• FastClosures.jl needs us to allow let esc(x)=esc(x) to work, which seems implemented wrong in macroexpansion.scm right now anyways, but we need to not break whatever it is doing now
• FLoops.jl wants to call deepcopy on Expr (for NLLSsolver.jl), even though that is pretty thoroughly undefined behavior. Hopefully a straight copy there would work better (e.g. correctly)?
• JET.jl does not handle hygiene correctly (https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/ea9fde8_vs_6d70d2a/JET.primary.log), same for JSExpr.jl, MLStyle.jl, and likely quite a few others
• WatchJuliaBurn.jl seems to be toast, which seems like a possible bug
• TryCatch.jl seems to also not getting error handling correct after this, which seems like a possible bug

I think the best solution here will be to make a slow deprecation cycle here, so that people can fix their hygiene issues, then opt into having line info correct (there are a lot of tests that rely on macros not being able to track line numbers because they test explicitly that parsing and evaluating a macro from different lines gives egal results). So we can add a legacy=true keyword to the macroexpand function to preserve the old behavior for a bit. Before the next LTS, we can flip the default to legacy=false, then in a later LTS we can drop that flag.

[vtjnash]

Oh sigh. TryCatch.jl and WatchJuliaBurn.jl fail because I stopped having @test strip line number information, and that breaks lowering. Lowering assumes that the last expression is the return value of a block, but that assumption is invalid if the last expression happens to be a metadata node (such as a line number), as happens to be explicitly put into the wrong place in the AST by both of those packages.

[vtjnash]

This now gives up entirely on fixing macroexpand for now, but just keeps the patch for lowering, so that the uses with @test 0//0 works.

[timholy]

Thanks, @vtjnash! Looking forward to trying this out.