Revise.jl
Revise.jl copied to clipboard
timholy
WIP: fix #552: use invoke_in_world
Depends on https://github.com/JuliaLang/julia/pull/37902 and still needs to be made backwards-compatible.
fixes #552 (at least on future Julia versions)
Doesn't it make more sense to only update the world age at the beginning of a revision? I worry that updating it during each package's deletion phase will just trigger the parent issue all over again. You might not see that in your tests, but add a second package with a deletion: then the update happens before Revise gets a chance to define the replacement, and I believe at that point you are in trouble.
The problem is that macro expansion at least needs to be aware of macros that Revise itself creates when revising code. (Perhaps there should be a separate worldage for that?) This is where the error from @testmac here comes from. I honestly don't quite know how to fix that, because as far as I understand it, for every file Revise revises, it first does all the relevant method deletions and only then evaluates the new definitions. For macros, this means that we would need to update our worldage between those evaluations, because otherwise, later method definitions using those macros wouldn't get macro expanded correctly, or error because the macro wasn't defined in the current worldage yet. If we did update our worldage between each of these definitions though, we wouldn't be able to solve #552 anymore. Do you have any ideas on how this could still work?
Oh, interesting. Perhaps the easiest would be to change the order: handle each package (both deletion and evaluation in one package before moving to the next).
In the old days, doing the deletions first was basically a hack to allow moving a method from one file to another (within the same package) without accidentally deleting it after evaluating in the new location. But Revise is now much smarter about that: it sorts packages and files in dependency order, and it supports having multiple locations transiently. So it seems like it should be safe to switch the strategy now.