-
Notifications
You must be signed in to change notification settings - Fork 114
Fix struct/const revision #894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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
yes
I don't understand what you're asking
It lists all cross-module edges to get everything, you also need to iterate all methods in the same module as the binding.
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. |
e1a5084
to
3f878e3
Compare
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
seems to be what I was looking for. This seems fairly straightforward, thanks for the excellent groundwork. |
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. |
I added a cross-module test (the 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
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 |
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. |
Just to cross-check:
yields a backedge for the julia> e = only(b.backedges)
Binding StructConstUser.Point
40598:∞ - explicit `using` from StructConst.Point
1:0 - undefined binding - guard entry |
That's because |
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 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. |
I think that's fine. Conceptually Revise does both adding new methods and deleting old ones. Base is always "append only". |
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. |
e8ba7ab
to
7b6d113
Compare
@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 |
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. |
Yes, when a type gets rebound, Revise will scan the entire system for methods There's a separate bit of novelty for |
I'll be curious to see whether it improves the efficiency of hacking on inference. |
I don't know if I like that. We already have |
I didn't know about I can back that commit out, if there are other mechanisms to achieve the same aim. Are you saying that |
I tried this. On:
What I did was:
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? |
I updated against the latest master branch. I tested this branch with the simple case, but it seems that for 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)" |
db02a78
to
9b73a5d
Compare
Or just juliaup add the PR (prXYZ). |
@lassepe your issue is fixed in |
┌ Revise
│ ┌ Warning: precompile directive
│ │ precompile(Tuple{mbody.sig.parameters[1], Symbol, Bool, Bool, Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}}, typeof(methods_by_execution!), Compiled, MI, Module, Expr})
│ │ failed. Please report an issue in Revise (after checking for duplicates) or remove this directive.
│ └ @ Revise ~/.julia/packages/Revise/FV0Wc/src/precompile.jl:65
└ REPL outputjulia> Pkg.add(name="Revise"; rev="teh/struct_revision")
Updating git-repo `https://github.com/timholy/Revise.jl.git`
Resolving package versions...
Updating `~/.julia/environments/v1.12/Project.toml`
[295af30f] + Revise v3.9.0 `https://github.com/timholy/Revise.jl.git#teh/struct_revision`
Updating `~/.julia/environments/v1.12/Manifest.toml`
[da1fd8a2] + CodeTracking v2.0.0
[807dbc54] + Compiler v0.1.1
[aa1ae85d] + JuliaInterpreter v0.10.5
[6f1432cf] + LoweredCodeUtils v3.4.3
[bac558e1] + OrderedCollections v1.8.1
[ae029012] + Requires v1.3.1
[295af30f] + Revise v3.9.0 `https://github.com/timholy/Revise.jl.git#teh/struct_revision`
[56f22d72] + Artifacts v1.11.0
[2a0f44e3] + Base64 v1.11.0
[7b1f6079] + FileWatching v1.11.0
[b77e0a4c] + InteractiveUtils v1.11.0
[ac6e5ff7] + JuliaSyntaxHighlighting v1.12.0
[76f85450] + LibGit2 v1.11.0
[8f399da3] + Libdl v1.11.0
[d6f4376e] + Markdown v1.11.0
[ca575930] + NetworkOptions v1.3.0
[de0858da] + Printf v1.11.0
[3fa0cd96] + REPL v1.11.0
[9a3f8284] + Random v1.11.0
[ea8e919c] + SHA v0.7.0
[6462fe0b] + Sockets v1.11.0
[f489334b] + StyledStrings v1.11.0
[cf7118a7] + UUIDs v1.11.0
[4ec0a83e] + Unicode v1.11.0
[e37daf67] + LibGit2_jll v1.9.0+0
[29816b5a] + LibSSH2_jll v1.11.3+1
[458c3c95] + OpenSSL_jll v3.5.1+0
Precompiling packages finished.
1 dependency successfully precompiled in 12 seconds. 20 already precompiled.
1 dependency had output during precompilation:
┌ Revise
│ ┌ Warning: precompile directive
│ │ precompile(Tuple{mbody.sig.parameters[1], Symbol, Bool, Bool, Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}}, typeof(methods_by_execution!), Compiled, MI, Module, Expr})
│ │ failed. Please report an issue in Revise (after checking for duplicates) or remove this directive.
│ └ @ Revise ~/.julia/packages/Revise/FV0Wc/src/precompile.jl:65
└ versioninfojulia> versioninfo()
Julia Version 1.12.0-rc2
Commit 72cbf019d04 (2025-09-06 12:00 UTC)
Build Info:
Official https://julialang.org release
Platform Info:
OS: Linux (x86_64-linux-gnu)
CPU: 16 × AMD Ryzen 7 5700U with Radeon Graphics
WORD_SIZE: 64
LLVM: libLLVM-18.1.7 (ORCJIT, znver2)
GC: Built with stock GC
Threads: 1 default, 1 interactive, 1 GC (on 16 virtual cores) Pkg status(@v1.12) pkg> st
Status `~/.julia/environments/v1.12/Project.toml`
[295af30f] Revise v3.9.0 `https://github.com/timholy/Revise.jl.git#teh/struct_revision`
(@v1.12) pkg> st -m
Status `~/.julia/environments/v1.12/Manifest.toml`
[da1fd8a2] CodeTracking v2.0.0
[807dbc54] Compiler v0.1.1
[aa1ae85d] JuliaInterpreter v0.10.5
[6f1432cf] LoweredCodeUtils v3.4.3
[bac558e1] OrderedCollections v1.8.1
[ae029012] Requires v1.3.1
[295af30f] Revise v3.9.0 `https://github.com/timholy/Revise.jl.git#teh/struct_revision`
[56f22d72] Artifacts v1.11.0
[2a0f44e3] Base64 v1.11.0
[7b1f6079] FileWatching v1.11.0
[b77e0a4c] InteractiveUtils v1.11.0
[ac6e5ff7] JuliaSyntaxHighlighting v1.12.0
[76f85450] LibGit2 v1.11.0
[8f399da3] Libdl v1.11.0
[d6f4376e] Markdown v1.11.0
[ca575930] NetworkOptions v1.3.0
[de0858da] Printf v1.11.0
[3fa0cd96] REPL v1.11.0
[9a3f8284] Random v1.11.0
[ea8e919c] SHA v0.7.0
[6462fe0b] Sockets v1.11.0
[f489334b] StyledStrings v1.11.0
[cf7118a7] UUIDs v1.11.0
[4ec0a83e] Unicode v1.11.0
[e37daf67] LibGit2_jll v1.9.0+0
[29816b5a] LibSSH2_jll v1.11.3+1
[458c3c95] OpenSSL_jll v3.5.1+0 |
Thanks for the report. I need to fix that, but FYI it's nothing to worry about in terms of Revise working properly. |
This comment was marked as resolved.
This comment was marked as resolved.
@jakobnissen, isn't the problem simply that both your module and the struct are called |
A modified version of my original setting above still seems to be broken (in a different way) on julia 1.12-rc2 and the lateset version of this branch Steps to reproduce
module StructRevision
struct Foo
x::Int
end
bar(::Foo) = 1
end # module StructRevision
❯ julia +1.12 --startup-file=no --project
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.12.0-rc2 (2025-09-06)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org release
|__/ |
julia> using Revise, StructRevision
julia> foo1 = StructRevision.Foo(1)
StructRevision.Foo(1)
julia> StructRevision.bar(foo1)
1
julia> # after changing Foo.x from Int to Float64:
julia> foo2 = StructRevision.Foo(1)
StructRevision.Foo(1.0)
julia> StructRevision.bar(foo1)
1
julia> StructRevision.bar(foo2)
ERROR: MethodError: no method matching bar(::StructRevision.Foo)
The function `bar` exists, but no method is defined for this combination of argument types.
Closest candidates are:
bar(::@world(StructRevision.Foo, 38519:38547))
@ StructRevision ~/worktree/BugReports/StructRevision.jl/src/StructRevision.jl:7
Stacktrace:
[1] top-level scope
@ REPL[7]:1 |
Hmm, there are working tests that are awfully close to your example, e.g., Lines 2453 to 2458 in 0158326
I'll have to check and see what the difference may be. |
This comment was marked as resolved.
This comment was marked as resolved.
@lassepe fixed in 962c7cb |
PSA: anyone who is using this via |
Nice! I stress-tested it a bit more, and all the stuff I tried before now works flawlessly.
module StructRevision
export Foo, bar
struct Foo{T}
x::T
end
bar(::Foo{T}) where {T} = "parametric with $T"
end # module StructRevision
worktree/BugReports/StructRevision.jl is 📦 v0.1.0 via ஃ v1.10.10 took 5s
❯ julia +1.12 --project=. --startup-file=no
_
_ _ _(_)_ | Documentation: https://docs.julialang.org
(_) | (_) (_) |
_ _ _| |_ __ _ | Type "?" for help, "]?" for Pkg help.
| | | | | | |/ _` | |
| | |_| | | | (_| | | Version 1.12.0-rc2 (2025-09-06)
_/ |\__'_|_|_|\__'_| | Official https://julialang.org release
|__/ |
julia> using Revise, StructRevision
julia> foo = Foo(1)
Foo{Int64}(1)
julia> bar(foo)
"parametric with Int64"
julia> # change Foo to: `struct Foo x::Int end`
julia> foo2 = Foo(1)
Foo(1)
julia> bar(foo)
"parametric with Int64"
julia> bar(foo2)
ERROR: MethodError: no method matching bar(::Foo)
The function `bar` exists, but no method is defined for this combination of argument types.
Closest candidates are:
bar(::@world(Foo, 38519:38547){T}) where T
@ StructRevision ~/worktree/BugReports/StructRevision.jl/src/StructRevision.jl:9
Stacktrace:
[1] top-level scope
@ REPL[6]:1
julia> # now change Foo back to its original definition
julia> foo = Foo(1)
Foo{Int64}(1)
julia> bar(foo)
ERROR: MethodError: no method matching bar(::Foo{Int64})
The function `bar` exists, but no method is defined for this combination of argument types.
Closest candidates are:
bar(::@world(Foo, 38519:38547){T}) where T
@ StructRevision ~/worktree/BugReports/StructRevision.jl/src/StructRevision.jl:9
Stacktrace:
[1] top-level scope
@ REPL[8]:1 |
I tried it on Makie and it immediately failed. With CairoMakie and Makie dev'ed I did ┌ Error: Failed to revise /Users/krumbiegel/.julia/dev/Makie/Makie/src/makielayout/types.jl
│ exception =
│ MethodError: no method matching getfile(::Nothing)
│ The function `getfile` exists, but no method is defined for this combination of argument types.
│
│ Closest candidates are:
│ getfile(::JuliaInterpreter.Frame)
│ @ JuliaInterpreter ~/.julia/packages/JuliaInterpreter/378J1/src/utils.jl:421
│ getfile(::JuliaInterpreter.FrameCode, ::Any)
│ @ JuliaInterpreter ~/.julia/packages/JuliaInterpreter/378J1/src/utils.jl:416
│ getfile(::JuliaInterpreter.Frame, ::Any)
│ @ JuliaInterpreter ~/.julia/packages/JuliaInterpreter/378J1/src/utils.jl:421
│ ...
│
│ Stacktrace:
│ [1] top-level scope
│ @ ~/.julia/dev/Makie/Makie/src/makielayout/types.jl:190
│ Revise evaluation error at /Users/krumbiegel/.julia/dev/Makie/Makie/src/makielayout/types.jl:190
│
└ @ Revise ~/.julia/packages/Revise/il844/src/packagedef.jl:783
┌ Warning: The running code does not match the saved version for the following files:
│
│ /Users/krumbiegel/.julia/dev/Makie/Makie/src/makielayout/types.jl
│
│ If the error was due to evaluation order, it can sometimes be resolved by calling `Revise.retry()`.
│ Use Revise.errors() to report errors again. Only the first error in each file is shown.
│ Your prompt color may be yellow until the errors are resolved.
└ @ Revise ~/.julia/packages/Revise/il844/src/packagedef.jl:957 I then tried to julia> Revise.retry()
┌ Error: Failed to revise /Users/krumbiegel/.julia/dev/Makie/Makie/src/makielayout/types.jl
│ exception =
│ UndefVarError: `#1217###mixin#307` not defined in `Makie`
│ Suggestion: check for spelling errors or missing imports.
│ Stacktrace:
│ [1] top-level scope
│ @ ~/.julia/dev/Makie/Makie/src/colorsampler.jl:14
│ Revise evaluation error at /Users/krumbiegel/.julia/dev/Makie/Makie/src/colorsampler.jl:14
│
│ Stacktrace:
│ [1] top-level scope
│ @ ~/.julia/dev/Makie/Makie/src/makielayout/types.jl:190
│ Revise evaluation error at /Users/krumbiegel/.julia/dev/Makie/Makie/src/makielayout/types.jl:190
│
└ @ Revise ~/.julia/packages/Revise/il844/src/packagedef.jl:783 |
Ah, here is a bug: if in my example above you go "full circle" by adding back the parameter |
@jkrumbiegel, thanks for the example. I might need your help, because Makie's recipe system makes it a bit hard for me to look up how types are defined. I created a gist (because it exceeded GitHub's message limit) with a log consisting of:
So it seems to start trying to start redefining Axis (does that wrap Let me know if these somehow make sense or if Revise is just going off the rails somehow. |
Hm I'm not sure if it makes sense, the type Axis shouldn't change if ScrollZoom changes. ScrollZoom is used when instantiating an Axis via Maybe Makie's system where structs are defined via complicated macros just ties too many things together so that one small change ripples outward further than it should. The thing with the mixin kind of makes me think that, as it was definitely not designed with revise-ability in mind and is maybe a bit "weird" in places. Like this macro-within-macro thing here https://github.com/MakieOrg/Makie.jl/blob/46350344714203b165c696947e37051708bf332a/Makie/src/recipes.jl#L506-L508 in which there is indeed a gensym with |
Thanks for the perspective, that's extraordinarily helpful. Merely knowing that I should start at the "head" of this log (why did it redefine Re the gensyms and revisability, do keep in mind that the only reason Revise needs to go to all these shenanigans is because we don't yet have JuliaLowering: to track changes in line numbers and thus "repair" the line numbers in stacktraces, Revise has to map source text ↔ signatures in a manner more dynamic than plain line numbers (in case you insert code earlier in the file). Currently there isn't a way to do that besides reparse/lower/almost-evaluate all the source files, but JuliaLowering should provide a much better way. Since hopefully 🤞 JuliaLowering will land sometime in the Julia 1.13 or 1.14 timeframe, one option is to just wait it out. |
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 thatstruct
aren't automatically re-evaluated for the new type. Specifically, in the added tests, after revising the definition ofPoint
we getand 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:
invalidate_code_for_globalref!
is not called?)binding.backedges
to list everything that uses this bindingPresumably, 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.