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

Assertion cycle == depth' failed when testing AxisKeys #50450

Closed
KristofferC opened this issue Jul 7, 2023 · 2 comments · Fixed by #50542
Closed

Assertion cycle == depth' failed when testing AxisKeys #50450

KristofferC opened this issue Jul 7, 2023 · 2 comments · Fixed by #50542
Labels
regression Regression in behavior compared to a previous version types and dispatch Types, subtyping and method dispatch
Milestone

Comments

@KristofferC
Copy link
Member

When testing AxisKeys it asserts with Assertion cycle == depth' failed:

Test Summary:            | Pass  Total   Time
fast findfirst & findall |  448    448  11.4s
julia: /source/src/gf.c:3429: sort_mlmatches: Assertion `cycle == depth' failed.

[30] signal (6.-6): Aborted
in expression starting at /home/pkgeval/.julia/packages/AxisKeys/QsNqy/test/runtests.jl:31
@KristofferC KristofferC added regression Regression in behavior compared to a previous version compiler:codegen Generation of LLVM IR and native code labels Jul 7, 2023
@KristofferC KristofferC added this to the 1.10 milestone Jul 7, 2023
@KristofferC KristofferC changed the title Assertion cycle == depth' failed` when testing AxisKeys Assertion cycle == depth' failed when testing AxisKeys Jul 7, 2023
@maleadt
Copy link
Member

maleadt commented Jul 13, 2023

This one is tough to reduce; removing code makes the bug not occur always anymore. The following MWE crashes every so often, and it seems more likely when running with --compiled-modules=false:

module AxisKeys
using Base: @propagate_inbounds, OneTo, RefValue
struct KeyedArray{T,N,AT,KT} <: AbstractArray{T,N} end
const KeyedVector = KeyedArray
const KeyedMatrix = KeyedArray
const KeyedVecOrMat = Union{KeyedVector, KeyedMatrix}
using NamedDims
Base.vcat(A::AbstractVecOrMat, B::KeyedVecOrMat, Cs::AbstractVecOrMat...) = nothing
end

using Test
detect_ambiguities(AxisKeys, Base)

@maleadt
Copy link
Member

maleadt commented Jul 13, 2023

Bisected to ac1cb1c, so #49664 (cc @vtjnash).

The method lookup that fails:

type = Tuple{typeof(Base.vcat), AbstractArray{T, 1} where T, AxisKeys.KeyedArray{T, 1, AT, KT} where KT where AT where T, Vararg{AbstractArray{T, 1} where T}}
Base._methods_by_ftype(type, -1, Base.get_world_counter())

... so this can be used instead of detect_ambiguities (although the import of Test still seems required).

@maleadt maleadt added types and dispatch Types, subtyping and method dispatch and removed compiler:codegen Generation of LLVM IR and native code labels Jul 13, 2023
vtjnash added a commit that referenced this issue Jul 13, 2023
We do not care about this condition (the point of this fast path is to
skip checking it).

Fix #50450
github-merge-queue bot pushed a commit that referenced this issue Jul 14, 2023
We do not care about this condition (the point of this fast path is to
skip checking it).

Fix #50450
KristofferC pushed a commit that referenced this issue Jul 17, 2023
We do not care about this condition (the point of this fast path is to
skip checking it).

Fix #50450

(cherry picked from commit 15bcf1b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants