Skip to content

fix zero-dimensional reverse! #58086

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

Merged
merged 1 commit into from
Apr 13, 2025
Merged

fix zero-dimensional reverse! #58086

merged 1 commit into from
Apr 13, 2025

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Apr 12, 2025

Also changed the check to compare against the tuple argument, instead of against the method static parameter for the tuple length. Should be better when the length isn't known at compile time.

Fixes #58085

Also changed the check to compare against the tuple argument, instead
of against the method static parameter for the tuple length. Should be
better when the length isn't known at compile time.

Fixes JuliaLang#58085
@nsajko nsajko added bugfix This change fixes an existing bug backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 arrays [a, r, r, a, y, s] collections Data structures holding multiple items, e.g. sets labels Apr 12, 2025
@nsajko
Copy link
Contributor Author

nsajko commented Apr 12, 2025

As mentioned in the PR description, I believe changing M == 0 to dims === () is an improvement, because it results in better code when M isn't known, which is any time the length of dims isn't exactly known. See code_typed:

julia> f(dims::NTuple{M}) where {M} = M == 0
f (generic function with 1 method)

julia> g(dims::NTuple{M}) where {M} = dims === ()
g (generic function with 1 method)

julia> code_typed(f, Tuple{Tuple{Any,Vararg}})
1-element Vector{Any}:
 CodeInfo(
1 ─ %1 = $(Expr(:static_parameter, 1))::Int64
│   %2 =   builtin (%1 === 0)::Bool
└──      return %2
) => Bool

julia> code_typed(g, Tuple{Tuple{Any,Vararg}})
1-element Vector{Any}:
 CodeInfo(
1 ─     return false
) => Bool

@jishnub jishnub merged commit b265dba into JuliaLang:master Apr 13, 2025
13 checks passed
@nsajko nsajko deleted the reverse0dim branch April 13, 2025 07:58
KristofferC pushed a commit that referenced this pull request Apr 14, 2025
Also changed the check to compare against the tuple argument, instead of
against the method static parameter for the tuple length. Should be
better when the length isn't known at compile time.

Fixes #58085

(cherry picked from commit b265dba)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 bugfix This change fixes an existing bug collections Data structures holding multiple items, e.g. sets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zero-dimensional reverse and reverse! for AbstractArray throw
2 participants