Revise.jl icon indicating copy to clipboard operation
Revise.jl copied to clipboard

Recursive type definition causes "invalid redefinition of constant"

Open rasmushenningsson opened this issue 10 months ago • 2 comments

If I have the following struct in a module tracked by Revise:

module ReviseBug
struct Foo <: AbstractDict{String,Foo}
    d::Dict{String,Foo}
end
end

it gives the error message invalid redefinition of constant ReviseBug.Foo when revising (even if I don't change the definition of Foo).

On the other hand, this works well:

module ReviseBug
struct Foo
    d::Dict{String,Foo}
end
end

when revising.

I'm using Julia 1.10.2 and Revise v3.5.14.

rasmushenningsson avatar Apr 12 '24 15:04 rasmushenningsson

I have been hitting this so I looked into it, but this is not a Revise issue actually, it comes from upstream:

julia> struct NoRec end;

julia> struct NoRec end;

julia> struct Rec <: AbstractVector{Rec} end;

julia> struct Rec <: AbstractVector{Rec} end;
ERROR: invalid redefinition of constant Main.Rec
Stacktrace:
 [1] top-level scope
   @ REPL[2]:1

(on julia v1.10.4)

The root of the difference comes from Core._equiv_typedef, which is called when declaring a struct to check whether it matches a previously created struct of the same name. For two instances of NoRec, we have

julia> var"#1001#NoRec" = Core._structtype(Main, :NoRec, Core.svec(), Core.svec(), Core.svec(), false, 1); Core._setsuper!(var"#1001#NoRec", Any)

julia> var"#1000#NoRec" = Core._structtype(Main, :NoRec, Core.svec(), Core.svec(), Core.svec(), false, 1); Core._setsuper!(var"#1000#NoRec", Any)

julia> Core._equiv_typedef(var"#1000#NoRec", var"#1001#NoRec")
true

but for two instances of Rec it becomes:

julia> var"#1000#Rec" = Core._structtype(Main, :Rec, Core.svec(), Core.svec(), Core.svec(), false, 1); Core._setsuper!(var"#1000#Rec", Core.apply_type(AbstractVector, var"#1000#Rec"))

julia> var"#1001#Rec" = Core._structtype(Main, :Rec, Core.svec(), Core.svec(), Core.svec(), false, 1); Core._setsuper!(var"#1001#Rec", Core.apply_type(AbstractVector, var"#1001#Rec"))

julia> Core._equiv_typedef(var"#1000#Rec", var"#1001#Rec")
false

The fix would probably consist in teaching the equiv_type function (in src/builtins.c of julia) to handle this recursion, instead of directly relying on jl_types_equal to compare the supertypes, as is currently done.

Liozou avatar Jun 10 '24 15:06 Liozou

Nice find. Do you want to report it upstream?

rasmushenningsson avatar Jun 10 '24 16:06 rasmushenningsson

Really nice work @Liozou ! I'll close this since it's not revise-specific, but if the issue gets fixed upstream and Revise doesn't do the right thing, let's reopen.

timholy avatar Jul 20 '24 09:07 timholy