[WIP] Fix struct/const revision

[timholy]

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]

• 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]

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]

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]

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]

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]

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