Skip to content

Conversation

@timholy
Copy link
Owner

@timholy timholy commented Mar 13, 2025

UPDATED 2025-12-29

Revise should now be able to handle many cases of updating struct definitions.
It would be great if you could give it another try, and ping me (@aviatesk) if you find any issues.

Status

  • Core data structure refactoring (MethodInfo -> ExInfo)
  • Generalize ExInfo data structure to have SigInfo & TypeInfo
  • Implement type information tracking
  • Implement correct revision logic
  • Add test cases
  • Update/add docstrings/comments
  • Integrate type redefinition failure case with Revise's error handling system
  • [-] Handle simple type alias updates
  • [-] Correct re-evaluation order
  • Performance optimization
  • Pass all tests
  • Update documentation

The aim here is to provide full support for struct/const revision. This first commit just adds simple tests, but it already reveals something that needs to be addressed: when a struct gets revised, methods defined for that struct aren't automatically re-evaluated for the new type. Specifically, in the added tests, after revising the definition of Point we get

julia> methods(StructConst.firstval)
# 1 method for generic function "firstval" from StructConst:
 [1] firstval(p::@world(StructConst.Point, 40598:40739))
     @ /tmp/2wfLvVmwY7/StructConst/src/StructConst.jl:11

and there is no defined method for a currently-valid StructConst.Point.

@Keno, if you have a moment, let me check my understanding of the current situation:

  • method applicability is assessed by type-intersection, as usual, but now types (via their bindings) can have a world-age
  • the MethodInstances and CodeInstances involving the old type are still valid (their world age is not capped, but of course they require inputs with old types)
  • there is no invalidation step needed for changes of the kind tested with the revision of Point (invalidate_code_for_globalref! is not called?)
  • we can't count on binding.backedges to list everything that uses this binding
  • the right solution is for Revise to:
    • detect that we're about to redefine a struct
    • search the entire system for any methods where that type appears in a signature
    • parse the source files, if necessary
    • redefine the struct
    • re-evaluate the method definitions

Presumably, waiting to do the last step as the last stage of a revision would be wise, as it is possible that more than one struct will be revised at the same time, and one might as well do each evaluation only once.

@Keno
Copy link
Collaborator

Keno commented Mar 13, 2025

  • method applicability is assessed by type-intersection, as usual, but now types (via their bindings) can have a world-age

I think that's a valid way to say it, but just to clarify, nothing about the type object itself changes, just the name by which you have to access it

  • the MethodInstances and CodeInstances involving the old type are still valid (their world age is not capped, but of course they require inputs with old types)

yes

  • there is no invalidation step needed for changes of the kind tested with the revision of Point (invalidate_code_for_globalref! is not called?)

I don't understand what you're asking

  • we can't count on binding.backedges to list everything that uses this binding

It lists all cross-module edges to get everything, you also need to iterate all methods in the same module as the binding.

  • the right solution is for Revise to:

    • detect that we're about to redefine a struct
    • search the entire system for any methods where that type appears in a signature
    • parse the source files, if necessary
    • redefine the struct
    • re-evaluate the method definitions

In the fullness of time, the correct way to implement Revise is to run the toplevel expressions in a non-standard evaluation mode that tracks dependency edges for any evaluated constants. However, that should maybe wait until after the JuliaLowering changes and the interpreter rewrite. I think your proposed strategy is reasonable in the meantime although not fully complete.

@timholy timholy force-pushed the teh/struct_revision branch from e1a5084 to 3f878e3 Compare March 13, 2025 12:15
@timholy
Copy link
Owner Author

timholy commented Mar 13, 2025

there is no invalidation step needed for changes of the kind tested with the revision of Point (invalidate_code_for_globalref! is not called?)

I don't understand what you're asking

I was wondering whether some kind of callback in there could be helpful in short-circuiting the process of determining which methoddefs need to be re-evaluated, but if it's not called then that won't help. From what I can tell, that's not going to work anyway because it only traverses the module containing the type definition. But your comment

It lists all cross-module edges to get everything, you also need to iterate all methods in the same module as the binding.

seems to be what I was looking for.

This seems fairly straightforward, thanks for the excellent groundwork.

@Keno
Copy link
Collaborator

Keno commented Mar 13, 2025

I was wondering whether some kind of callback in there could be helpful in short-circuiting the process of determining which methoddefs need to be re-evaluated

No, there's no edges of any kind for signatures. You need to scan the entire system. However, an earlier version of that code did the same whole-system scan, so you can steal some code for whole-system scanning.

@timholy
Copy link
Owner Author

timholy commented Mar 13, 2025

I added a cross-module test (the StructConstUser module), expecting this might populate b.bindings, but I get

julia> b = convert(Core.Binding, GlobalRef(StructConst, :Point))
Binding StructConst.Point
   40743:- constant binding to StructConst.Point
   40598:40742 - constant binding to @world(StructConst.Point, 40598:40742)
   1:0 - backdated constant binding to @world(StructConst.Point, 40598:40742)
   1:0 - backdated constant binding to @world(StructConst.Point, 40598:40742)

julia> b.backedges
ERROR: UndefRefError: access to undefined reference
Stacktrace:
 [1] getproperty(x::Core.Binding, f::Symbol)
   @ Base ./Base_compiler.jl:55
 [2] top-level scope
   @ REPL[6]:1

so I don't think I understand what

It lists all cross-module edges

really means. It also doesn't populate if I add

struct PointWrapper
    p::StructConst.Point
end

to that module.

I understand that I have to traverse the whole system, I'm just curious about what b.backedges stores.

@Keno
Copy link
Collaborator

Keno commented Mar 13, 2025

There's two kinds of edges that are tracked there. One is explicit import/using. The other is to lowered code of method definitions (but only after the first inference). At no point is an edge ever added for an evaluation of a binding, only for compilation of code that references the binding.

@timholy
Copy link
Owner Author

timholy commented Mar 13, 2025

Just to cross-check:

                module StructConstUser
                using StructConst: Fixed, Point
                struct PointWrapper
                    p::Point
                end
                scuf(f::Fixed) = 33 * f.x
                scup(p::Point) = 44 * p.x
                end

yields a backedge for the using StructConst: Fixed, Point statement, but I don't see anything related to scup even if I evaluate it:

julia> e = only(b.backedges)
Binding StructConstUser.Point
   40598:- explicit `using` from StructConst.Point
   1:0 - undefined binding - guard entry

@Keno
Copy link
Collaborator

Keno commented Mar 13, 2025

but I don't see anything related to scup even if I evaluate it:

That's because scup doesn't use any bindings other than * and getproperty in its lowered code (as I said, no edges for signatures). However, even if you had like mkscup() = Point(), that binding would be to StructConstUser. To actually get a cross-module edge is not that easy, but you could do @eval mkscup() = $(GlobalRef(StructConst, :Point))(). The most common case to get cross-module edges is macros. Also, not a lot of edges is good, that is the design goal, since we don't want to store them :).

@timholy
Copy link
Owner Author

timholy commented Mar 14, 2025

This is getting close, but there's a philosophical question to answer: should Revise try to hew to Base behavior as closely as possible, or should it add/modify behaviors for what might be a nicer interactive experience?

In this case, the issue is the following: when I redefine Point, in Base you can still call previously-defined functions on "old data." However, in this PR, calling functions on old data throws (or is intended to throw) a MethodError. Why might you want that? Because I imagine that a common user-error will be to forget to refresh your data with updated types, and if your whole code pipeline runs (using all the old definitions), I imagine people will bang their heads against walls trying to figure out why their edits aren't having the intended effect. In contrast, if they get a no method foo(::@world(Point, m:n)), they'll pretty quickly learn how you fix it.

I should say that initially I wasn't aiming in this direction, and this current proposal arose from noticing some unexpected subtleties about the order in which Revise's cache files get set up: the order in which you parse the source, lower the code, and rebind the name matters quite a lot, and some of the choices in this version are designed to compensate for the fact that Revise normally doesn't parse a file unless it's been edited. (Now, we'll also have to parse any file with a method whose signature references an updated type.) I think it's possible to hew to the Base behavior, if we decide that's better, but the initial bugs I faced led me to ask what behavior we actually want, and I came to the conclusion that it's likely a better user experience if we invalidate methods that only work on old types.

@timholy timholy changed the title [WIP] Fix struct/const revision Fix struct/const revision Mar 15, 2025
@Keno
Copy link
Collaborator

Keno commented Mar 15, 2025

However, in this PR, calling functions on old data throws (or is intended to throw) a MethodError. Why might you want that?

I think that's fine. Conceptually Revise does both adding new methods and deleting old ones. Base is always "append only".

@timholy
Copy link
Owner Author

timholy commented Mar 16, 2025

The collateral damage to tests is something I'm looking into; if one runs all the tests, there appear to be some failures that predate this effort, including an incomplete updating of the ecosystem to JuliaLang/julia#52415. I'll work on that.

But meanwhile, this seems to be working:

tim@diva:~/.julia/dev/Revise$ ~/src/juliaw/julia --startup=no --project -e 'using Pkg; Pkg.test(; test_args=["struct/const revision"], coverage=false)'
     Testing Revise
      Status `/tmp/jl_4EbO8B/Project.toml`
# Pkg output deleted
     Testing Running tests...
Test Summary: | Pass  Total   Time
Revise        |   30     30  10.3s
beginning cleanup

and may be ready for anyone who wants to review it.

@timholy timholy force-pushed the teh/struct_revision branch 2 times, most recently from e8ba7ab to 7b6d113 Compare April 11, 2025 17:27
@timholy
Copy link
Owner Author

timholy commented Apr 12, 2025

@Keno @vtjnash @aviatesk @JeffBezanson here's a fun one:

julia> using Revise
Precompiling Revise finished.
  1 dependency successfully precompiled in 6 seconds. 19 already precompiled.

julia> Revise.track(Core.Compiler)

julia> fieldnames(Core.Compiler.NativeInterpreter)
(:world, :method_table, :inf_cache, :codegen, :inf_params, :opt_params)

#
# edited Compiler/src/types.jl to add a field `dummy` to `NativeInterpreter`
#

julia> 1+1
WARNING: Detected access to binding `Compiler.#NativeInterpreter#476` in a world prior to its definition world.
  Julia 1.12 has introduced more strict world age semantics for global bindings.
  !!! This code may malfunction under Revise.
  !!! This code will error in future versions of Julia.
Hint: Add an appropriate `invokelatest` around the access to this binding.
     # lots more like this
Compiling the compiler. This may take several minutes ...
Base.Compiler ──── 1.72376 seconds
2

julia> fieldnames(Core.Compiler.NativeInterpreter)
(:world, :method_table, :inf_cache, :codegen, :inf_params, :opt_params, :dummy)

julia> function f(::Integer)
           Base.Experimental.@force_compile
           return 1
       end
f (generic function with 1 method)

julia> f(5)
1

@aviatesk
Copy link
Collaborator

This is interesting.

So it looks like this PR tracks edges not only from binding to method signature, but also from binding to method body?

I think a similar mechanism will be needed when developing a new language server. If it's already implemented in Revise, I'd like to reuse it.

I haven't had a chance to look at this PR closely yet, but I'll read it later.

@timholy
Copy link
Owner Author

timholy commented Apr 12, 2025

Yes, when a type gets rebound, Revise will scan the entire system for methods m::Method where that type appears in m.sig. It then tries to find the expression that defined that method, deletes the old method, and re-evaluates the expression. That way it regenerates methods for the new meaning of the type.

There's a separate bit of novelty for Core.Compiler: this PR also checks whether any of the entry points to type inference have been invalidated, and if so calls bootstrap!

@timholy
Copy link
Owner Author

timholy commented Apr 12, 2025

I'll be curious to see whether it improves the efficiency of hacking on inference.

@Keno
Copy link
Collaborator

Keno commented Apr 12, 2025

There's a separate bit of novelty for Core.Compiler: this PR also checks whether any of the entry points to type inference have been invalidated, and if so calls bootstrap!

I don't know if I like that. We already have @activate Compiler[:codegen] for this use case.

@timholy
Copy link
Owner Author

timholy commented Apr 13, 2025

I didn't know about @activate. Is there a demo of the workflow somewhere? I've seen the docs, and they're good, but I suspect there's a bit more wisdom around effective usage than they convey.

I can back that commit out, if there are other mechanisms to achieve the same aim. Are you saying that Revise.track(Core.Compiler) should be deprecated?

@KristofferC
Copy link
Collaborator

KristofferC commented Apr 16, 2025

I tried this. On:

  • Revise (this PR)
  • LoweredCodeUtils master
  • JuliaInterpreter master
  • Julia 1.12.0-beta1

What I did was:

  • Load Revise, Tensors.
  • Make the following revision to Tensors:
    diff --git a/src/Tensors.jl b/src/Tensors.jl
    index 4cd677b..5f04312 100644
    --- a/src/Tensors.jl
    +++ b/src/Tensors.jl
    @@ -63,6 +63,7 @@ julia> Tensor{1,3,Float64}((1.0, 2.0, 3.0))
     """
     struct Tensor{order, dim, T, M} <: AbstractTensor{order, dim, T}
         data::NTuple{M, T}
    +    x::Int
         Tensor{order, dim, T, M}(data::NTuple) where {order, dim, T, M} = new{order, dim, T, M}(data)
     end
  • Press 1 in the REPL
  • Press 1 in the REPL again

with the following result:

julia> using Revise, Tensors

# updates Tensors file here

julia> 1
ERROR: UndefVarError: `Broadcasted` not defined in `StaticArrays.StableFlatten`
Suggestion: check for spelling errors or missing imports.
Stacktrace:
 [1] top-level scope
   @ ~/.julia/packages/StaticArrays/LSPcF/src/broadcast.jl:180
Revise evaluation error at /Users/kristoffercarlsson/.julia/packages/StaticArrays/LSPcF/src/broadcast.jl:180

Stacktrace:
  [1] methods_by_execution!(recurse::Any, methodinfo::Revise.CodeTrackingMethodInfo, docexprs::Dict{…}, mod::Module, ex::Expr; mode::Symbol, disablebp::Bool, always_rethrow::Bool, kwargs::@Kwargs{})
    @ Revise ~/JuliaPkgs/Revise.jl/src/lowered.jl:306
  [2] eval_with_signatures(mod::Module, ex::Expr; mode::Symbol, kwargs::@Kwargs{})
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:556
  [3] eval_with_signatures
    @ ~/JuliaPkgs/Revise.jl/src/packagedef.jl:553 [inlined]
  [4] instantiate_sigs!(modexsigs::OrderedCollections.OrderedDict{…}; mode::Symbol, kwargs::@Kwargs{})
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:564
  [5] instantiate_sigs!
    @ ~/JuliaPkgs/Revise.jl/src/packagedef.jl:560 [inlined]
  [6] maybe_extract_sigs_for_meths(meths::Set{Method})
    @ Revise ~/JuliaPkgs/Revise.jl/src/pkgs.jl:162
  [7] (::Revise.var"#107#108"{Bool})()
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:941
  [8] lock(f::Revise.var"#107#108"{Bool}, l::ReentrantLock)
    @ Base ./lock.jl:335
  [9] #revise#104
    @ ~/JuliaPkgs/Revise.jl/src/packagedef.jl:844 [inlined]
 [10] revise()
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:842
 [11] top-level scope
    @ REPL:1

caused by: UndefVarError: `Broadcasted` not defined in `StaticArrays.StableFlatten`
Suggestion: check for spelling errors or missing imports.
Stacktrace:
  [1] lookup_var
    @ ~/.julia/packages/JuliaInterpreter/J7J9G/src/interpret.jl:5 [inlined]
  [2] step_expr!(recurse::Any, frame::JuliaInterpreter.Frame, node::Any, istoplevel::Bool)
    @ JuliaInterpreter ~/.julia/packages/JuliaInterpreter/J7J9G/src/interpret.jl:44
  [3] signature(recurse::Any, frame::JuliaInterpreter.Frame, stmt::Any, pc::Int64)
    @ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/BIVzf/src/signatures.jl:50
  [4] methoddef!(recurse::Any, signatures::Vector{Any}, frame::JuliaInterpreter.Frame, stmt::Any, pc::Int64; define::Bool)
    @ LoweredCodeUtils ~/.julia/packages/LoweredCodeUtils/BIVzf/src/signatures.jl:595
  [5] methoddef!
    @ ~/.julia/packages/LoweredCodeUtils/BIVzf/src/signatures.jl:534 [inlined]
  [6] methods_by_execution!(recurse::Any, methodinfo::Revise.CodeTrackingMethodInfo, docexprs::Dict{…}, frame::JuliaInterpreter.Frame, isrequired::Vector{…}; mode::Symbol, skip_include::Bool)
    @ Revise ~/JuliaPkgs/Revise.jl/src/lowered.jl:354
  [7] methods_by_execution!
    @ ~/JuliaPkgs/Revise.jl/src/lowered.jl:317 [inlined]
  [8] methods_by_execution!(recurse::Any, methodinfo::Revise.CodeTrackingMethodInfo, docexprs::Dict{…}, mod::Module, ex::Expr; mode::Symbol, disablebp::Bool, always_rethrow::Bool, kwargs::@Kwargs{})
    @ Revise ~/JuliaPkgs/Revise.jl/src/lowered.jl:296
  [9] eval_with_signatures(mod::Module, ex::Expr; mode::Symbol, kwargs::@Kwargs{})
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:556
 [10] eval_with_signatures
    @ ~/JuliaPkgs/Revise.jl/src/packagedef.jl:553 [inlined]
 [11] instantiate_sigs!(modexsigs::OrderedCollections.OrderedDict{…}; mode::Symbol, kwargs::@Kwargs{})
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:564
 [12] instantiate_sigs!
    @ ~/JuliaPkgs/Revise.jl/src/packagedef.jl:560 [inlined]
 [13] maybe_extract_sigs_for_meths(meths::Set{Method})
    @ Revise ~/JuliaPkgs/Revise.jl/src/pkgs.jl:162
 [14] (::Revise.var"#107#108"{Bool})()
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:941
 [15] lock(f::Revise.var"#107#108"{Bool}, l::ReentrantLock)
    @ Base ./lock.jl:335
 [16] #revise#104
    @ ~/JuliaPkgs/Revise.jl/src/packagedef.jl:844 [inlined]
 [17] revise()
    @ Revise ~/JuliaPkgs/Revise.jl/src/packagedef.jl:842
 [18] top-level scope
    @ REPL:1
Some type information was truncated. Use `show(err)` to see complete types.

julia> 1
Compiling the compiler. This may take several minutes ...
Base.Compiler ──── 3.43502 seconds
1

The UndefVarError seems correct:

julia> Tensors.StaticArrays.StableFlatten.Broadcasted
ERROR: UndefVarError: `Broadcasted` not defined in `StaticArrays.StableFlatten`
Suggestion: check for spelling errors or missing imports.
Stacktrace:
 [1] getproperty(x::Module, f::Symbol)
   @ Base ./Base_compiler.jl:48
 [2] top-level scope
   @ REPL[4]:1

so I guess the question is why it is tried to be looked up. And also, why does the Compiler recompile when I press another time 1 with no new modifications?

timholy added a commit that referenced this pull request Apr 19, 2025
Fixes #903

This is split out from #894.
timholy added a commit that referenced this pull request Apr 19, 2025
@aviatesk
Copy link
Collaborator

I updated against the latest master branch.

I tested this branch with the simple case, but it seems that for exported bindings, additional edge tracking is needed?
In particular, the world ages of bindings that are usinged need to be updated, it seems.
For example, let's say we have an Example package like this:

module Example
export hello, Hello

struct Hello
    who::String
end

hello(x::Hello) = hello(x.who)

"""
    hello(who::String)

Return "Hello, `who`".
"""
hello(who::String) = "Hello, $who"

end

We get the following error:

julia> using Revise

julia> using Example

julia> hello(Hello("world"))
"Hello, world"

julia> # Apply the following diff
       # ```diff
       # diff --git a/src/Example.jl b/src/Example.jl
       # index 65c7eae..c631814 100644
       # --- a/src/Example.jl
       # +++ b/src/Example.jl
       # @@ -2,10 +2,10 @@ module Example
       #  export hello, Hello
       #  
       #  struct Hello
       # -    who::String
       # +    who2::String
       #  end
       #  
       # -hello(x::Hello) = hello(x.who)
       # +hello(x::Hello) = hello(x.who2 * " (changed)")
       #  
       #  """
       #      hello(who::String)
       # ```

julia> hello(Hello("world"))
ERROR: MethodError: no method matching @world(Example.Hello, 38355:38383)(::String)
The type `@world(Example.Hello, 38355:38383)` exists, but no method is defined for this combination of argument types when trying to construct it.
Stacktrace:
 [1] top-level scope
   @ REPL[5]:1

julia> hello(Example.Hello("world")) # but this works
"Hello, world (changed)"

@aviatesk aviatesk force-pushed the teh/struct_revision branch 7 times, most recently from 60a75c8 to 792e578 Compare December 29, 2025 09:09
@aviatesk aviatesk changed the title Fix struct/const revision Support struct revision Dec 29, 2025
@aviatesk aviatesk force-pushed the teh/struct_revision branch from 792e578 to 5c0766a Compare December 29, 2025 09:31
@aviatesk
Copy link
Collaborator

Ah, it's been a long journey. This work is finally wrapping up.

I think Revise can now handle many cases for updating struct definitions. On my end, I've confirmed that all the issues brought up in this long thread are resolved, so it would be great if you could give it another try when you have some time.

Starting today, I'll spend a few days testing it, and the plan is to merge it around the new year.

@timholy I've re-evaluated and implemented the invalidations Revise needs to handle to enable struct revision in most of realistic cases. This ended up adding nearly 1000 new lines of changes, and the logic has been significantly rewritten from the initial struct revision implementation, so I'd like to leave a few points to note for future reference:

  • To add information to the pkgdatas cache about which type definition an expression is associated with, I've generalized the ModuleExprsSigs type to ModuleExprsInfos, and its leaf type has been changed from the previous SigInfo to Union{SigInfo,TypeInfo}.

    • However, for TypeInfo, I haven't prepared a lightweight cache on the CodeTracking side that corresponds to CodeTracking.method_info yet. Revise didn't need it for a correct implementation, and it would be a hassle to maintain compatibility with CodeTracking, so I decided it's not worth doing right now. But it might be useful for optimization, so I've left a TODO comment in packagedef.jl just in case.
  • Thanks to Julia's binding partition support, Revise could, in principle, correctly propagate the effects associated with binding redefinition to the active session (regardless of whether the binding is const or not), not just the struct revision implemented in this PR, by cleverly using delete_binding and similar functions. This is a superset of the "struct revision" implemented by this PR, which I'm calling "binding revision.", meaning it enables revision in cases like the following:

      global n::Int = 10 # and then modified to `n::Int = 5`
      for i = 1:n
          @eval func(::Val{$i}) = $i
      end

    However, to handle this correctly, we'd need to explicitly manage access to bindings accompanied with top-level code execution and track their edges, which is quite technically challenging. I might do that in the future, but I don't know when. For now, this PR only achieves "struct revision," so I've updated the PR description based on that.

  • Regarding the invalidation logic that happens with type redefinition, you can understand it by looking at record_invalidations_for_type_deletion!. Tests were added also.

  • I've removed the logic that automatically performs Compiler.jl's bootstrap. I wasn't entirely confident if the logic was correct, and I also prefer calling bootstrap manually. Also, I didn't think there was a strong need to add such additional revision logic specifically for Compiler.jl, so I've removed it for now.

  • One of the minor things I may finish before merging is the revision ordering problem. Currently, revision follows a simple flow like this:

    • First, execute the code modified by the user.
    • Redefine type definitions related to that code.
    • Redefine methods related to that code.
      However, this approach might cause errors if the user's modified code includes methods that should be defined using new types. We might need to implement a revision order that first defines all related type definitions, including those changed by the user, and then redefines related method definitions. But for now, the current logic works fine in the scope of my testing, so I'm keeping this naive logic for the time being.

@aviatesk
Copy link
Collaborator

@jkrumbiegel As far as I've tested on my end, the issue you found seems to be resolved. It would be helpful if you could try it again on your side too.

julia> using Revise
Precompiling Revise finished.
  1 dependency successfully precompiled in 6 seconds

julia> using CairoMakie

julia> fieldnames(CairoMakie.Makie.ScrollZoom)
(:speed, :reset_timer, :prev_xticklabelspace, :prev_yticklabelspace, :reset_delay)

julia> lines(cumsum(randn(100)))

julia> fieldnames(CairoMakie.Makie.ScrollZoom) # after edit (reset_timer -> reset_time)
(:speed, :reset_time, :prev_xticklabelspace, :prev_yticklabelspace, :reset_delay)

julia> lines(cumsum(randn(100)))

@jkrumbiegel
Copy link

jkrumbiegel commented Dec 29, 2025

@aviatesk, thanks for testing although I did make some changes to Makie which resolved the main issue. So this was not necessarily Revise's fault (it did not trip it up before of course). If you want to see whether your changes also made a difference you'd have to compare with a version before MakieOrg/Makie.jl#5402 was merged. The warnings I mention over there are gone in your example so that would be a big improvement anyway.

@aviatesk
Copy link
Collaborator

I ran the same test on Makie and CairoMakie with MakieOrg/Makie.jl#5402 reverted, and it seems to be working without warnings.

@aviatesk aviatesk force-pushed the teh/struct_revision branch from 5c0766a to 4ad1e1a Compare December 29, 2025 14:04
@topolarity
Copy link
Contributor

This is working pretty well for me locally so far! Nice work @aviatesk .

it enables revision in cases like the following:

  global n::Int = 10 # and then modified to `n::Int = 5`
  for i = 1:n
      @eval func(::Val{$i}) = $i
  end

I suspect there are a lot of complexities associated with trying to respond to updates to mutable bindings so I wouldn't expect that to work (yet?), but I did almost immediately run into the case that const updates do not cascade:

const IdVector = Vector{Int} # edit and `Graph` is not updated
struct Graph
    allocated::IdVector
    conflicted::IdVector
    resolved::IdVector
end

@topolarity
Copy link
Contributor

Same limitation seems to apply for type-aliases:

IdArray{N} = Array{Int64,N} # change and `Graph` is not updated
struct Graph
    allocated::IdArray{1}
    conflicted::IdArray{1}
    resolved::IdArray{1}
end

Is that expected?

This one is a little weird, because it is a type you're re-defining at top-level. It's just not a struct definition.

@aviatesk
Copy link
Collaborator

aviatesk commented Dec 29, 2025

Same limitation seems to apply for type-aliases:

IdArray{N} = Array{Int64,N} # change and `Graph` is not updated
struct Graph
    allocated::IdArray{1}
    conflicted::IdArray{1}
    resolved::IdArray{1}
end

Is that expected?

This one is a little weird, because it is a type you're re-defining at top-level. It's just not a struct definition.

Thanks for testing this! And yeah, Revise doesn't handle this case right now. It's what I'd call a 'binding revision' case, rather than a 'struct revision'. But I think we could support it by adding a simple 'ad-hoc hack'. The same goes for the first case you mentioned; we could add that before merging this PR (added a new TODO item for the OP).
Generalizing the hack would require some pretty advanced techniques.

@aviatesk aviatesk force-pushed the teh/struct_revision branch from 4ad1e1a to c609a91 Compare December 29, 2025 18:54
@aviatesk
Copy link
Collaborator

Gave it a shot, but it turned out to be tricky to handle even simple cases where const binding or type alias is used in struct fields with just a simple ad-hoc hack. believe we need a more comprehensive approach to support "binding revision", but I don't think it should be implemented in this PR. So I've removed it from this PR's blocking items.

aviatesk and others added 3 commits December 31, 2025 04:27
Add a dependent module test

Add method scan

Implement recursive processing

The `PointWrapper` binding invalidation only happens when we re-evaluate
the constructor with the new binding for `Point`.

Broaden recursive processing

Fix type-change check, invokelatest

`Core._equiv_typedef` only checks the fieldnames, not their types.

Wrapping `eval_with_signatures` in `invokelatest` ensures that lowering will use the new binding resolutions.

Use subtyping for where constraints

delete debugging code

Perform type-comparisons after unwrapping

Unwrap before querying typename

Check equivalence recursively (fix recursive types)

Types like
```
struct Foo
    x::Vector{Foo}
end
```
were getting needlessly redefined.

Fix paths/cachefiles for stdlibs, Compiler

With the removal of some stdlibs, the corresponding cachefiles are no
longer in the base source cache.

The independent Compiler module also needs path fixes.

Increase `mtimedelay`

On fast systems the current setting is too aggressive

Disable tests: tracking CoreCompiler, coverage

For reasons not yet understood, tracking Core.Compiler deletes methods
for `NativeCompiler`.

The "misc - coverage" tests also fail in interactive mode
(`throwto_repl` errors, as it should, in interactive mode).

Improve the mtimedelay change

Avoid redefining types unless required

Improve method redefinition

improve struct revision tests

bootstrap the compiler if invalidated

Update to #918

Update to julia#58131

More mtimedelay tweaking

WSL was still flaky

Fix struct/const revise

Minor Recipes fix

Add aviatesk example

use `Core.methodtable` instead of `Core.GlobalMethod`

Don't call `Compiler.bootstrap!`

Fix 1.10

Force some precompilation

Restore & refine compiler revision

Undo bce9202, but track world ages to distinguish method redefinitions
triggered by Revise from those triggered by ordinary code loading.

More logging improvements

Add timing info, and show MethodSummary arrays.

Update to CodeTracking v2

Back out the test-package precompilaton

Was causing failures in extra tests

includet: avoid "prior to def. world" warning

Add test where field type changes

Fix primitive type comparison

add test with additional constructors

Fix custom constructors

Co-authored-by: Tim Holy <tim.holy@gmail.com>
@aviatesk aviatesk force-pushed the teh/struct_revision branch from 3fbcba8 to 4ee6596 Compare December 30, 2025 19:27
@lassepe
Copy link

lassepe commented Dec 30, 2025

I've been using this for ~6h in a side project of mine since the latest set of commits was pushed yesterday, and I get a lot of value out of it! Thanks for this great work! I didn't do any dedicated "let's try to break this" but rather just used it organically as part of my regular workflow. I've found it to be very robust in that context.

@aviatesk
Copy link
Collaborator

@lassepe, thanks for testing it. Glad it worked for your case.

@aviatesk
Copy link
Collaborator

Alright, let's merge this.
After merging, I'll update NEWS.md and release v3.13.

@aviatesk aviatesk merged commit 3bdeb41 into master Dec 31, 2025
23 of 24 checks passed
@aviatesk aviatesk deleted the teh/struct_revision branch December 31, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.