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

subtyping covariance issue with Union{<:AbstractMatrix{P},P}} #45874

Closed
projekter opened this issue Jun 30, 2022 · 3 comments · Fixed by #49023
Closed

subtyping covariance issue with Union{<:AbstractMatrix{P},P}} #45874

projekter opened this issue Jun 30, 2022 · 3 comments · Fixed by #49023
Labels
types and dispatch Types, subtyping and method dispatch

Comments

@projekter
Copy link

After getting a "unreachable reached", I was able to construct the following MWE that reproduces the error:

struct Problem{P}
    item::AbstractVector{<:Union{P,<:AbstractMatrix{P}}}

    function Problem{P}(item::AbstractVector{<:Union{P,<:AbstractMatrix{P}}}) where {P}
        return new{P}(item)
    end
end

struct Container
    el::Problem
end

struct Failure
    function Failure(cnt::Container)
        element = cnt.el
        return typeof(element)(element.item)
    end
end

prob = Problem{Int}(Int[])
cnt = Container(prob)
Failure(cnt)

The construction of Failure in the last line triggers the error. Notably, only on Windows (Julia 1.7.3 x64), when running on Linux (Ubuntu WSL 1.7.3 x64), the code passes through. Of course, using the Debugger where everything is interpreted also works fine.
Some observations:

  • If I delete the constructor for Problem, the problem vanishes. Unfortunately, the actual constructor does a bit of work, so that this is not an option.
  • If I change Failure to accept a Problem directly, without the Container, the problem vanishes.
  • If the item is of a more simple type - e.g., replacing the Union by either P or <:AbstractMatrix{P} - the problem vanishes.
  • I was very surprised when I ended up with this MWE, since a construction like this (with a type like item, a container, a typeof-constructor call, the internal constructor like this) has been part of my code for weeks now and always worked. I did some other changes completely unrelated to this, which somehow ended up raising this error. Unfortunately, I cannot say what was responsible, since I mainly tested on WSL in the last days, where everything works.
julia> versioninfo()
Julia Version 1.7.3
Commit 742b9abb4d (2022-05-06 12:58 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-12.0.1 (ORCJIT, tigerlake)

julia> Failure(cnt)

Unreachable reached at 000000005f78134d

Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ILLEGAL_INSTRUCTION at 0x5f78134d -- Failure at .\REPL[3]:4
in expression starting at REPL[6]:1
Failure at .\REPL[3]:4
unknown function (ip: 000000005f78139c)
jl_apply at /cygdrive/c/buildbot/worker/package_win64/build/src\julia.h:1788 [inlined]
do_call at /cygdrive/c/buildbot/worker/package_win64/build/src\interpreter.c:126
eval_value at /cygdrive/c/buildbot/worker/package_win64/build/src\interpreter.c:215
eval_stmt_value at /cygdrive/c/buildbot/worker/package_win64/build/src\interpreter.c:166 [inlined]
eval_body at /cygdrive/c/buildbot/worker/package_win64/build/src\interpreter.c:583
jl_interpret_toplevel_thunk at /cygdrive/c/buildbot/worker/package_win64/build/src\interpreter.c:731
jl_toplevel_eval_flex at /cygdrive/c/buildbot/worker/package_win64/build/src\toplevel.c:885
jl_toplevel_eval_flex at /cygdrive/c/buildbot/worker/package_win64/build/src\toplevel.c:830
jl_toplevel_eval at /cygdrive/c/buildbot/worker/package_win64/build/src\toplevel.c:894 [inlined]
jl_toplevel_eval_in at /cygdrive/c/buildbot/worker/package_win64/build/src\toplevel.c:944
eval at .\boot.jl:373 [inlined]
eval_user_input at C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\REPL\src\REPL.jl:150
repl_backend_loop at C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\REPL\src\REPL.jl:246
start_repl_backend at C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\REPL\src\REPL.jl:231
#run_repl#47 at C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\REPL\src\REPL.jl:364
run_repl at C:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.7\REPL\src\REPL.jl:351
#936 at .\client.jl:394
jfptr_YY.936_22652.clone_1 at C:\Users\Benjamin\AppData\Local\Programs\Julia-1.7.3\lib\julia\sys.dll (unknown line)
jl_apply at /cygdrive/c/buildbot/worker/package_win64/build/src\julia.h:1788 [inlined]
jl_f__call_latest at /cygdrive/c/buildbot/worker/package_win64/build/src\builtins.c:757
#invokelatest#2 at .\essentials.jl:716 [inlined]
invokelatest at .\essentials.jl:714 [inlined]
run_main_repl at .\client.jl:379
exec_options at .\client.jl:309
_start at .\client.jl:495
jfptr__start_34800.clone_1 at C:\Users\Benjamin\AppData\Local\Programs\Julia-1.7.3\lib\julia\sys.dll (unknown line)
jl_apply at /cygdrive/c/buildbot/worker/package_win64/build/src\julia.h:1788 [inlined]
true_main at /cygdrive/c/buildbot/worker/package_win64/build/src\jlapi.c:559
jl_repl_entrypoint at /cygdrive/c/buildbot/worker/package_win64/build/src\jlapi.c:701
mainCRTStartup at /cygdrive/c/buildbot/worker/package_win64/build/cli\loader_exe.c:42
BaseThreadInitThunk at C:\WINDOWS\System32\KERNEL32.DLL (unknown line)
RtlUserThreadStart at C:\WINDOWS\SYSTEM32\ntdll.dll (unknown line)
Allocations: 2722 (Pool: 2711; Big: 11); GC: 0
@ViralBShah ViralBShah added the system:windows Affects only Windows label Jun 30, 2022
@vtjnash
Copy link
Member

vtjnash commented Jun 30, 2022

MWE:

julia> T = Tuple{Type{Val{P}} where P, AbstractVector{<:Union{P, <:AbstractMatrix{P}}} where P}
Tuple{Type{Val{P}} where P, AbstractVector{<:Union{P, var"#s4"} where var"#s4"<:AbstractMatrix{P}} where P}

julia> S = Tuple{Type{Val{P}}, AbstractVector{<:Union{P, <:AbstractMatrix{P}}}} where P
Tuple{Type{Val{P}}, AbstractVector{<:Union{P, var"#s4"} where var"#s4"<:AbstractMatrix{P}}} where P

julia> S<:T
true # returns `false` on Windows

@vtjnash
Copy link
Member

vtjnash commented Jul 1, 2022

everywhere else MWE:

julia> T = Tuple{Type{Val{P}} where P, AbstractVector{<:Union{<:AbstractMatrix{P},P}} where P}
Tuple{Type{Val{P}} where P, AbstractVector{<:Union{var"#s2", P} where var"#s2"<:AbstractMatrix{P}} where P}

julia> S = Tuple{Type{Val{P}}, AbstractVector{<:Union{<:AbstractMatrix{P},P}}} where P
Tuple{Type{Val{P}}, AbstractVector{<:Union{var"#s1", P} where var"#s1"<:AbstractMatrix{P}}} where P

julia> S<:T
false # returns `true` on Windows

@vtjnash vtjnash added types and dispatch Types, subtyping and method dispatch and removed system:windows Affects only Windows labels Jul 1, 2022
@vtjnash vtjnash changed the title Unreachable reached on Windows subtyping covariance issue with Union{<:AbstractMatrix{P},P}} Jul 1, 2022
@vtjnash
Copy link
Member

vtjnash commented Jul 1, 2022

perhaps related or similar?

julia> S = Pair{Val{P}, AbstractVector{<:Union{P,<:AbstractMatrix{P}}}} where P
Pair{Val{P}, AbstractVector{<:Union{P, var"#s1"} where var"#s1"<:AbstractMatrix{P}}} where P

julia> T = Pair{Val{R}, AbstractVector{<:Union{P,<:AbstractMatrix{P}}}} where {P,R}
Pair{Val{R}, AbstractVector{<:Union{P, var"#s1"} where var"#s1"<:AbstractMatrix{P}}} where {P, R}

julia> S<:T
false # should be true?

julia> typeintersect(S,T)
StackOverflow

vtjnash added a commit that referenced this issue Jul 1, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874
vtjnash added a commit that referenced this issue Jul 1, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874
KristofferC pushed a commit that referenced this issue Jul 6, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874
KristofferC pushed a commit that referenced this issue Jul 7, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874

(cherry picked from commit 8cc5445)
KristofferC pushed a commit that referenced this issue Jul 8, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874

(cherry picked from commit 8cc5445)
ffucci pushed a commit to ffucci/julia that referenced this issue Aug 11, 2022
…45896)

Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs JuliaLang#45874
pcjentsch pushed a commit to pcjentsch/julia that referenced this issue Aug 18, 2022
…45896)

Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs JuliaLang#45874
KristofferC pushed a commit that referenced this issue Dec 21, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874

(cherry picked from commit 8cc5445)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874

(cherry picked from commit 8cc5445)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874

(cherry picked from commit 8cc5445)
staticfloat pushed a commit that referenced this issue Dec 23, 2022
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874

(cherry picked from commit 8cc5445)
N5N3 added a commit to N5N3/julia that referenced this issue Jan 31, 2023
This makes the result soundness at the cost of more subtype cost. We should try to fix this on `subtype_leftvar` side.

close JuliaLang#45874.
N5N3 added a commit to N5N3/julia that referenced this issue Mar 16, 2023
N5N3 added a commit to N5N3/julia that referenced this issue Mar 16, 2023
vtjnash pushed a commit that referenced this issue Mar 16, 2023
oscardssmith pushed a commit to oscardssmith/julia that referenced this issue Mar 20, 2023
Xnartharax pushed a commit to Xnartharax/julia that referenced this issue Apr 19, 2023
KristofferC pushed a commit that referenced this issue Oct 10, 2023
Different platforms implement qsort differently, leading to
platform-specific errors. This is a quick port of the ml_matches
algorithm for use instead. For small unions (almost always), this should
also be slightly faster, though insignificant.

Refs #45874

(cherry picked from commit 8cc5445)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants