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

Possible inference improvement in iterated assignment where the RHS uses splatting #41022

Closed
jishnub opened this issue May 31, 2021 · 0 comments · Fixed by #41839
Closed

Possible inference improvement in iterated assignment where the RHS uses splatting #41022

jishnub opened this issue May 31, 2021 · 0 comments · Fixed by #41839

Comments

@jishnub
Copy link
Contributor

jishnub commented May 31, 2021

julia> function f(r)
       a, b, c = r..., nothing
       end
f (generic function with 1 method)

julia> @code_warntype f(1:2)
MethodInstance for f(::UnitRange{Int64})
  from f(r) in Main at REPL[1]:1
Arguments
  #self#::Core.Const(f)
  r::UnitRange{Int64}
Locals
  @_3::Int64
  c::Any
  b::Any
  a::Any
Body::Tuple
1%1 = Core.tuple(Main.nothing)::Core.Const((nothing,))
│   %2 = Core._apply_iterate(Base.iterate, Core.tuple, r, %1)::Tuple%3 = Base.indexed_iterate(%2, 1)::Core.PartialStruct(Tuple{Any, Int64}, Any[Any, Core.Const(2)])
│        (a = Core.getfield(%3, 1))
│        (@_3 = Core.getfield(%3, 2))
│   %6 = Base.indexed_iterate(%2, 2, @_3::Core.Const(2))::Core.PartialStruct(Tuple{Any, Int64}, Any[Any, Core.Const(3)])
│        (b = Core.getfield(%6, 1))
│        (@_3 = Core.getfield(%6, 2))
│   %9 = Base.indexed_iterate(%2, 3, @_3::Core.Const(3))::Core.PartialStruct(Tuple{Any, Int64}, Any[Any, Core.Const(4)])
│        (c = Core.getfield(%9, 1))
└──      return %2

The RHS is inferred as a Tuple, although it's really a Tuple{Vararg{Union{Int, Nothing}}}

In this case the variables may at least be inferred as Union{Int, Nothing}. Ideally a and b may be inferred as Ints as the function will error if there are less than three elements on the right hand side.

julia> f(1:1)
ERROR: BoundsError: attempt to access Tuple{Int64, Nothing} at index [3]

Edit: A simpler example:

julia> function f(r)
       a, b, c = r..., 1
       end
f (generic function with 1 method)

julia> @code_warntype f(1:2)
Variables
  #self#::Core.Const(f)
  r::UnitRange{Int64}
  @_3::Int64
  c::Any
  b::Any
  a::Any

Body::Tuple
1%1 = Core.tuple(1)::Core.Const((1,))
│   %2 = Core._apply_iterate(Base.iterate, Core.tuple, r, %1)::Tuple%3 = Base.indexed_iterate(%2, 1)::Core.PartialStruct(Tuple{Any, Int64}, Any[Any, Core.Const(2)])
│        (a = Core.getfield(%3, 1))
│        (@_3 = Core.getfield(%3, 2))
│   %6 = Base.indexed_iterate(%2, 2, @_3::Core.Const(2))::Core.PartialStruct(Tuple{Any, Int64}, Any[Any, Core.Const(3)])
│        (b = Core.getfield(%6, 1))
│        (@_3 = Core.getfield(%6, 2))
│   %9 = Base.indexed_iterate(%2, 3, @_3::Core.Const(3))::Core.PartialStruct(Tuple{Any, Int64}, Any[Any, Core.Const(4)])
│        (c = Core.getfield(%9, 1))
└──      return %2

In this case the variables must all be Ints

martinholters added a commit that referenced this issue Aug 9, 2021
The first loop is left before `valtype` and `statetype` are updated from
the current `stateordonet`, so the second loop should first process it
before obtaining a new one instead of vice versa.

Fixes #41022.
martinholters added a commit that referenced this issue Aug 9, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Fixes #41022.
martinholters added a commit that referenced this issue Aug 18, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Fixes #41022.
martinholters added a commit that referenced this issue Aug 19, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Fixes #41022.
martinholters added a commit that referenced this issue Aug 24, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Fixes #41022.
vtjnash added a commit that referenced this issue Aug 30, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Explicitly test that inference produces a sound (and reasonably precise)
result when splatting an iterator (in this case a long range) that
allows constant-propagation up to the `MAX_TUPLE_SPLAT` limit.

Fixes #41022

Co-authored-by: Jameson Nash <[email protected]>
KristofferC pushed a commit that referenced this issue Aug 31, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Explicitly test that inference produces a sound (and reasonably precise)
result when splatting an iterator (in this case a long range) that
allows constant-propagation up to the `MAX_TUPLE_SPLAT` limit.

Fixes #41022

Co-authored-by: Jameson Nash <[email protected]>
(cherry picked from commit 92337b5)
KristofferC pushed a commit that referenced this issue Sep 3, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Explicitly test that inference produces a sound (and reasonably precise)
result when splatting an iterator (in this case a long range) that
allows constant-propagation up to the `MAX_TUPLE_SPLAT` limit.

Fixes #41022

Co-authored-by: Jameson Nash <[email protected]>
(cherry picked from commit 92337b5)
KristofferC pushed a commit that referenced this issue Sep 3, 2021
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Explicitly test that inference produces a sound (and reasonably precise)
result when splatting an iterator (in this case a long range) that
allows constant-propagation up to the `MAX_TUPLE_SPLAT` limit.

Fixes #41022

Co-authored-by: Jameson Nash <[email protected]>
(cherry picked from commit 92337b5)
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Feb 22, 2022
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Explicitly test that inference produces a sound (and reasonably precise)
result when splatting an iterator (in this case a long range) that
allows constant-propagation up to the `MAX_TUPLE_SPLAT` limit.

Fixes JuliaLang#41022

Co-authored-by: Jameson Nash <[email protected]>
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Mar 8, 2022
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Explicitly test that inference produces a sound (and reasonably precise)
result when splatting an iterator (in this case a long range) that
allows constant-propagation up to the `MAX_TUPLE_SPLAT` limit.

Fixes JuliaLang#41022

Co-authored-by: Jameson Nash <[email protected]>
staticfloat pushed a commit that referenced this issue Dec 23, 2022
If the first loop exits in the first iteration, the `statetype` is still
`Bottom`. In that case, the new `stateordonet` needs to be determined
with the two-arg version of `iterate` again.

Explicitly test that inference produces a sound (and reasonably precise)
result when splatting an iterator (in this case a long range) that
allows constant-propagation up to the `MAX_TUPLE_SPLAT` limit.

Fixes #41022

Co-authored-by: Jameson Nash <[email protected]>
(cherry picked from commit 92337b5)
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.

1 participant