Revise and DispatchDoctor

[thofma]

I have been playing around with https://github.com/MilesCranmer/DispatchDoctor.jl/ (v0.4.2) and found it quite useful. It also seems to work well with Revise.includet:

shell> cat test.jl
@stable f(x) = x == 1 ? 1 : 1.0

julia> Revise.includet("/Users/thofma/test2.jl")

julia> f(1)
ERROR: TypeInstabilityError: Instability detected in `f` defined at test.jl:1 with arguments `(Int64,)`. Inferred to be `Union{Float64, Int64}`, which is not a concrete type.

If I then change test.jl, i.e. change f from unstable -> stable, things are properly revised (even changing f from unstable -> unstable works.)

But it does not seem to work well within a module. If I have a module with the following content:

module A
using DispatchDoctor
include("test.jl")
end

Now if I do using Revise, A etc, and change f from unstable -> unstable, the error is not reported anymore.

As far as I understand, DispatchDoctor.jl replaces A.include with https://github.com/MilesCranmer/DispatchDoctor.jl/blob/569aac2fc3a904aa6e499a6a0b406aaa3a3ed849/src/stabilization.jl#L196-L205, and it seems this does not work properly in connection with Revise.

I am not sure how realistic it is to have both packages work nicely together. So feel free to close this issue, if it is technically not possible.

CC: @MilesCranmer

[MilesCranmer]

@timholy does Revise work by mapping include to includet? I don’t really have a good feel for where the interaction is coming from. If it’s just something like that I guess I could add a Revise extension to map A.include in the generated code to [something analogous to] A.includet?

[MilesCranmer]

I think it's due to #634?

Roughly, @stable include("file.jl") will create the following function:

function inner(ex)
    new_ex, _ = _stabilize_all(ex, DownwardMetadata(); kws...)
    return new_ex
end

and insert

include($(inner), "file.jl")

This recursively applies _stabilize_all to all included code – basically the same as if you had put @stable on every single function definition.

[MilesCranmer]

@timholy friendly ping :)

[MilesCranmer]

@thofma see #634 (comment).

I feel like Revise.jl needs very deep changes for this to happen. Therefore I think it would be better we just find a way to work around this limitation rather than fix it at the source.

[MilesCranmer]

I think perhaps a simpler option would be to allow per-file mapexpr to be added manually, similar to the callbacks interface –

Then I could do the rest on DispatchDoctor.jl side (adding the right functions to it).