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

bounds error on empty splat #37555

Closed
olof3 opened this issue Sep 13, 2020 · 2 comments · Fixed by #37634
Closed

bounds error on empty splat #37555

olof3 opened this issue Sep 13, 2020 · 2 comments · Fixed by #37634
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) regression Regression in behavior compared to a previous version
Milestone

Comments

@olof3
Copy link
Contributor

olof3 commented Sep 13, 2020

As stated in the documentation, one should use a semicolon before keyword arguments. However some packages, perhaps for historical reasons (?), use a comma instead (including a few spurious usages in stdlib).

In nightly, the following mwe throws a cryptic error message, while it worked in v1.5.

 function eigvals_mod(A::AbstractMatrix; kwargs...)
     @assert size(A)[1] == size(A)[2] "Matrix A needs to be square"
     eigvals!(copy(A), kwargs...)
 end

 > eigvals_mod(randn(2,2))

ERROR: BoundsError: attempt to access () at index [1]
Stacktrace:
 [1] getindex(t::Tuple, i::Int64)
   @ Base ./tuple.jl:29
 [2] iterate
   @ ./tuple.jl:66 [inlined]
 [3] iterate
   @ ./iterators.jl:254 [inlined]
 [4] #eigvals_mod#13
   @ ./REPL[15]:3 [inlined]
 [5] eigvals_mod(A::Matrix{Float64})
   @ Main ./REPL[15]:2
 [6] top-level scope
   @ REPL[16]:1

If the assertion is removed or the comma is replaced by a semicolon it works in nightly as well.

Perhaps it is related to #36774 ?

i guess it's not too much of an issue that incorrect usage gives rise to errors, but I guess it would be good to fix the handful of incorrect usages in stdlib, and possibly provide a better error message?

@c42f
Copy link
Member

c42f commented Sep 16, 2020

This is not a problem with syntax. It's ok to list keyword arguments without a ; when calling a function with keywords. (See https://docs.julialang.org/en/v1/manual/functions/#Keyword-Arguments)

Oddly enough, if I forget to do using LinearAlgebra before running your example and then correct the mistake interactively, this code works fine as written. Example session:

julia> function eigvals_mod(A::AbstractMatrix; kwargs...)
           @assert size(A)[1] == size(A)[2] "Matrix A needs to be square"
           eigvals!(copy(A), kwargs...)
       end
eigvals_mod (generic function with 1 method)

julia> eigvals_mod(randn(2,2))
ERROR: UndefVarError: eigvals! not defined
Stacktrace:
 [1] #eigvals_mod#7
   @ ./REPL[1]:3 [inlined]
 [2] eigvals_mod(A::Matrix{Float64})
   @ Main ./REPL[1]:2
 [3] top-level scope
   @ REPL[2]:1

julia> using LinearAlgebra

julia> eigvals_mod(randn(2,2))
2-element Vector{Float64}:
 -0.1764691396097775
  2.5547516089663445

So I think this is a compiler bug involving inference and iteration. @Keno could this be due to the changes in #36684?

@JeffBezanson JeffBezanson changed the title Comma before keyword args may give cryptic error in nightly bounds error on empty splat Sep 16, 2020
@JeffBezanson JeffBezanson added bug Indicates an unexpected problem or unintended behavior compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) regression Regression in behavior compared to a previous version labels Sep 16, 2020
@JeffBezanson
Copy link
Sponsor Member

Yes it looks like a compiler bug. Works fine with --compile=min.

@JeffBezanson JeffBezanson added this to the 1.6 features milestone Sep 16, 2020
@Keno Keno self-assigned this Sep 17, 2020
Keno added a commit that referenced this issue Sep 17, 2020
While working on IR, we give pending nodes SSA ids after the main
body of the function, and then we drop them in place during compaction.
Inlining was using thse IDs to try to determine which basic block
we're currently inlining into, but for pending blocks it was looking
at the raw ID rather than the insertion position, corrupting the CFG.

Fixes #37555
Fixes #37182
Keno added a commit that referenced this issue Sep 17, 2020
While working on IR, we give pending nodes SSA ids after the main
body of the function, and then we drop them in place during compaction.
Inlining was using thse IDs to try to determine which basic block
we're currently inlining into, but for pending blocks it was looking
at the raw ID rather than the insertion position, corrupting the CFG.

Fixes #37555
Fixes #37182
vchuravy pushed a commit that referenced this issue Oct 22, 2020
While working on IR, we give pending nodes SSA ids after the main
body of the function, and then we drop them in place during compaction.
Inlining was using thse IDs to try to determine which basic block
we're currently inlining into, but for pending blocks it was looking
at the raw ID rather than the insertion position, corrupting the CFG.

Fixes #37555
Fixes #37182

(cherry picked from commit ace08d8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants