Skip to content
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

Invalid redefinition of unchanged type #52686

Closed
timholy opened this issue Dec 31, 2023 · 1 comment · Fixed by #52748
Closed

Invalid redefinition of unchanged type #52686

timholy opened this issue Dec 31, 2023 · 1 comment · Fixed by #52748

Comments

@timholy
Copy link
Member

timholy commented Dec 31, 2023

Revise-free MWE from timholy/Revise.jl#770

tim@diva:~$ julia -q --startup=no
julia> mktemp() do f, io
           write(io, """
               struct A{T} end
               struct B{T, S}
                 a::A{<:T}
               end
               """)
           close(io)
           include(f)
           include(f)
       end
ERROR: LoadError: invalid redefinition of type B
Stacktrace:
 [1] top-level scope
   @ /tmp/jl_Gk79hD:2
 [2] include
   @ ./client.jl:489 [inlined]
 [3] (::var"#1#2")(f::String, io::IOStream)
   @ Main ./REPL[1]:10
 [4] mktemp(fn::var"#1#2", parent::String)
   @ Base.Filesystem ./file.jl:738
 [5] mktemp(fn::Function)
   @ Base.Filesystem ./file.jl:736
 [6] top-level scope
   @ REPL[1]:1
in expression starting at /tmp/jl_Gk79hD:2

julia> versioninfo()
Julia Version 1.10.0
Commit 3120989f39b (2023-12-25 18:01 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 16 × 12th Gen Intel(R) Core(TM) i7-1260P
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, alderlake)
  Threads: 1 on 16 virtual cores
Environment:
  JULIA_IMAGE_THREADS = 4

Changing a::A{<:T} to a::A{T} does not trigger the error, leading me to suspect that this may be a bug in Core._equiv_typedef (EDIT: nope, it returns true for both A and B, must be elsewhere):

julia> Meta.lower(Main, quote
       struct A{T} end
                      struct B{T, S}
                        a::A{<:T}
                      end
                      end)
:($(Expr(:thunk, CodeInfo(
     @ REPL[2]:2 within `top-level scope`
1 ──       global A
│          const A
│          Core.NewvarNode(:(#s5))
│          T@_2 = Core.TypeVar(:T)
│    %5  = Core.svec(T@_2)
│    %6  = Core.svec()
│    %7  = Core.svec()
│          A = Core._structtype(Main, :A, %5, %6, %7, false, 0)
│          Core._setsuper!(A, Core.Any)
│    %10 = $(Expr(:isdefined, :A))
└───       goto #6 if not %10
2 ── %12 = A
│    %13 = Core._equiv_typedef(%12, A)
└───       goto #4 if not %13
3 ──       A = %12%16 = Base.getproperty(%12, :body)
│    %17 = Base.getproperty(%16, :parameters)
│    %18 = Base.indexed_iterate(%17, 1)
│          T@_2 = Core.getfield(%18, 1)
└───       goto #5
4 ── %21 = A
└───       A = %21
5 ┄─       goto #7
6 ── %24 = A
└───       A = %24
7 ┄─ %26 = A
│    %27 = Core.svec()
│          Core._typebody!(%26, %27)
│          global A
│    %30 = Core.TypeVar(:T)
│          T@_4 = %30%32 = T@_4%33 = Core.apply_type(A, T@_4)
│    %34 = Core.apply_type(Core.Type, %33)
│    %35 = Core.UnionAll(%32, %34)
│    %36 = Core.svec(%35)
│    %37 = Core.svec()
│    %38 = Core.svec(%36, %37, $(QuoteNode(:(#= REPL[2]:2 =#))))
│          $(Expr(:method, nothing, :(%38), CodeInfo(
    @ REPL[2]:2 within `none`
1%1 = %new(#ctor-self#)
└──      return %1
)))
│    @ REPL[2]:3 within `top-level scope`global B
│          const B
│          Core.NewvarNode(:(@_5))
│          T@_7 = Core.TypeVar(:T)
│          S@_6 = Core.TypeVar(:S)
│    %45 = Core.svec(T@_7, S@_6)
│    %46 = Core.svec(:a)
│    %47 = Core.svec()
│          B = Core._structtype(Main, :B, %45, %46, %47, false, 1)
│          Core._setsuper!(B, Core.Any)
│    %50 = $(Expr(:isdefined, :B))
└───       goto #12 if not %50
8 ── %52 = B
│    %53 = Core._equiv_typedef(%52, B)
└───       goto #10 if not %53
9 ──       B = %52%56 = Base.getproperty(%52, :body)
│    %57 = Base.getproperty(%56, :body)
│    %58 = Base.getproperty(%57, :parameters)
│    %59 = Base.indexed_iterate(%58, 1)
│          T@_7 = Core.getfield(%59, 1)
│          @_5 = Core.getfield(%59, 2)
│    %62 = Base.indexed_iterate(%58, 2, @_5)
│          S@_6 = Core.getfield(%62, 1)
└───       goto #11
10%65 = B
└───       B = %65
11 ┄       goto #13
12%68 = B
└───       B = %68
13%70 = B
│    %71 = Core.TypeVar(Symbol("#s14"), T@_7)
│          @_9 = %71%73 = @_9%74 = Core.apply_type(A, @_9)
│    %75 = Core.UnionAll(%73, %74)
│    %76 = Core.svec(%75)
│          Core._typebody!(%70, %76)
│          global B
│    %79 = Core.TypeVar(:T)
│          T@_10 = %79%81 = T@_10%82 = Core.TypeVar(:S)
│          S@_11 = %82%84 = S@_11%85 = Core.apply_type(B, T@_10, S@_11)
│    %86 = Core.apply_type(Core.Type, %85)
│    %87 = Core.UnionAll(%84, %86)
│    %88 = Core.UnionAll(%81, %87)
│    %89 = Core.svec(%88, Core.Any)
│    %90 = Core.svec()
│    %91 = Core.svec(%89, %90, $(QuoteNode(:(#= REPL[2]:4 =#))))
│          $(Expr(:method, nothing, :(%91), CodeInfo(
    @ REPL[2]:4 within `none`
1%1 = Core.fieldtype(#ctor-self#, 1)@_3 = a
│   %3 = @_3 isa %1
└──      goto #3 if not %3
2 ─      goto #4
3@_3 = Base.convert(%1, @_3)
4%7 = @_3%8 = %new(#ctor-self#, %7)
└──      return %8
)))
└───       return nothing
))))
@vtjnash
Copy link
Member

vtjnash commented Jan 4, 2024

I think this would fix it:

diff --git a/src/builtins.c b/src/builtins.c
index 4a300c1f4d..58892b34b1 100644
--- a/src/builtins.c
+++ b/src/builtins.c
@@ -2046,7 +2046,7 @@ static int equiv_field_types(jl_value_t *old, jl_value_t *ft)
         jl_value_t *ta = jl_svecref(old, i);
         jl_value_t *tb = jl_svecref(ft, i);
         if (jl_has_free_typevars(ta)) {
-            if (!jl_has_free_typevars(tb) || !jl_egal(ta, tb))
+            if (!jl_has_free_typevars(tb) || !jl_types_egal(ta, tb))
                 return 0;
         }
         else if (jl_has_free_typevars(tb) || jl_typetagof(ta) != jl_typetagof(tb) ||

timholy added a commit that referenced this issue Jan 4, 2024
aviatesk added a commit that referenced this issue Jan 6, 2024
Fixes #52686
Fixes timholy/Revise.jl#770

---------

Co-authored-by: Jameson Nash <[email protected]>
Co-authored-by: Kristoffer Carlsson <[email protected]>
Co-authored-by: Shuhei Kadowaki <[email protected]>
aviatesk added a commit that referenced this issue Jan 6, 2024
Fixes #52686
Fixes timholy/Revise.jl#770

---------

Co-authored-by: Jameson Nash <[email protected]>
Co-authored-by: Kristoffer Carlsson <[email protected]>
Co-authored-by: Shuhei Kadowaki <[email protected]>
Drvi pushed a commit to RelationalAI/julia that referenced this issue Jun 7, 2024
Fixes JuliaLang#52686
Fixes timholy/Revise.jl#770

---------

Co-authored-by: Jameson Nash <[email protected]>
Co-authored-by: Kristoffer Carlsson <[email protected]>
Co-authored-by: Shuhei Kadowaki <[email protected]>
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 a pull request may close this issue.

2 participants