`methoddef!`: use `mt => sig` format when filling in `signatures`

[serenity4]

trafficstars

This allows Revise to work with https://github.com/timholy/CodeTracking.jl/pull/140. Requires https://github.com/JuliaDebug/JuliaInterpreter.jl/pull/680.

This change is breaking, so we may either:

• Tag a new breaking release.
• Keep backwards compatibility by supporting a signatures vector that uses the previous sig format.

[serenity4]

Looks great. You'd want to bump the [compat] for CodeTracking to 2.

CodeTracking is not a direct dependency of LoweredCodeUtils, but since it indirectly is via JuliaInterpreter I'll add it.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 76.90%. Comparing base (550a2a4) to head (154f937). :warning: Report is 48 commits behind head on master.

Files with missing lines	Patch %	Lines
src/signatures.jl	92.00%	2 Missing :warning:
src/codeedges.jl	87.50%	1 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #125       +/-   ##
===========================================
- Coverage   88.69%   76.90%   -11.80%     
===========================================
  Files           6        6               
  Lines        1442     1524       +82     
===========================================
- Hits         1279     1172      -107     
- Misses        163      352      +189     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[serenity4]

The failing test at https://github.com/JuliaDebug/LoweredCodeUtils.jl/actions/runs/14648771143/job/41109358572?pr=125#step:7:112 asserts that a function object depends on its first method definition. I slightly restructured the implementation for method code edge dependencies in https://github.com/JuliaDebug/LoweredCodeUtils.jl/pull/125/commits/854867989b157f0813bc680b1dd50a3b339e3f6f, and naturally expected a function binding to depend on its declaration, but not on the method definition. @aviatesk or @timholy would that be more correct according to you, or is it a regression?

In the code below, the defintion of Main.ModEval.revise538 now depends on $(Expr(:method, :(Main.ModEval.revise538))) only, and not on the actual method definition:

CodeInfo(
    @ /home/serenity4/.julia/dev/LoweredCodeUtils/test/codeedges.jl:339 within `unknown scope`
1 ─ %1  = enter #4
    @ /home/serenity4/.julia/dev/LoweredCodeUtils/test/codeedges.jl:340 within `unknown scope`
2 ─       global revise538
│         $(Expr(:latestworld))
│         $(Expr(:method, :(Main.ModEval.revise538)))
│         $(Expr(:latestworld))
│         $(Expr(:latestworld))
│   %7  = Main.ModEval.revise538
│   %8  =   dynamic Core.Typeof(%7)
│   %9  = Main.ModEval.Float32
│   %10 =   builtin Core.svec(%8, %9)
│   %11 =   builtin Core.svec()
│   %12 =   builtin Core.svec(%10, %11, $(QuoteNode(:(#= /home/serenity4/.julia/dev/LoweredCodeUtils/test/codeedges.jl:340 =#))))
│         $(Expr(:method, :(Main.ModEval.revise538), :(%12), CodeInfo(
    @ /home/serenity4/.julia/dev/LoweredCodeUtils/test/codeedges.jl:341 within `unknown scope`
1 ─ %1 = Main.ModEval.println
│   %2 =   dynamic (%1)("F32")
└──      return %2
)))
│         $(Expr(:latestworld))
│   %15 = Main.ModEval.revise538
└──       $(Expr(:leave, :(%1)))
3 ─       return %15
4 ┄       e = $(Expr(:the_exception))
│   @ /home/serenity4/.julia/dev/LoweredCodeUtils/test/codeedges.jl:344 within `unknown scope`
│   %19 = Main.ModEval.println
│   %20 =   dynamic (%19)("caught error")
│         $(Expr(:pop_exception, :(%1)))
└──       return %20
)

julia> lr[13]
false

Nothing in the refactor depends on that so I'm fine removing this change if necessary.

[serenity4]

Any input regarding my comment above? If unsure, I can restore the previous behavior.

[serenity4]

This should be ready, unless someone objects to https://github.com/JuliaDebug/LoweredCodeUtils.jl/pull/125#issuecomment-2828562564. I adjusted the related test in https://github.com/JuliaDebug/LoweredCodeUtils.jl/pull/125/commits/e624e99f1e16571752d40c10a2db1c7d4b16e81b so that it passes with the new behavior.

Before merging, I would first register version 2.0 of CodeTracking, then update JuliaInterpreter, then remove the dev patches in this PR then it will be good to go.

[aviatesk]

I slightly restructured the implementation for method code edge dependencies in 8548679, and naturally expected a function binding to depend on its declaration, but not on the method definition. @aviatesk or @timholy would that be more correct according to you, or is it a regression?

I apologize for the delayed response. I have now conducted the review. Regarding this point, I think the new behavior is fine. I believe it's more natural to depend on the 1-arg :method. By the way, could you explain why that change resolves this test failure? I was thinking that the function declaration would be selected in any case when tracked from GlobalRef(ModEval, :revise538).

Also, how would you like to proceed with the entire ecosystem to adapt to this change? I don't have an write access to CodeTracking.jl so we need to ask @timholy to make a release if we need to release a new version of it first.

[serenity4]

By the way, could you explain why that change resolves this test failure? I was thinking that the function declaration would be selected in any case when tracked from GlobalRef(ModEval, :revise538).

The test was working fine before that, it's just that one of the changes I made goes in the direction of only reevaluating the function definition, not the method definition. It is a small change that can be removed without compromising this PR, that I thought would be good to have. As we were testing that the method definition was reevaluated, I simply updated the test to test that no method gets reevaluated, because we then only reevaluate the function definition.

[serenity4]

For the release of CodeTracking v2, I opened https://github.com/timholy/CodeTracking.jl/pull/142.

[timholy]

I also agree that the new behavior is better.

[timholy]

Green! Merge at will, @serenity4