Make unpacking a Tuple or NamedTuple Composite type-inferrable

[sethaxen]

This PR resolves two issues that make unpacking a Composite uninferrable. On Julia 1.5, currently the following operations are uninferrable:

function foo(x)
    a, b = x
    return (a, b)
end

struct Bar
    a
    b
end

bar(x) = (x.a, x.b)

julia> using LinearAlgebra, ChainRulesCore, Test

julia> x = (3.0, 4); ∂x = Composite{typeof(x)}(5.0, 6);

julia> @inferred foo(∂x)
ERROR: return type Tuple{Float64,Int64} does not match inferred return type Tuple{Union{Float64, Int64},Union{Float64, Int64}}
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] top-level scope at REPL[9]:1

julia> y = Bar(3.0, 4); ∂y = Composite{typeof(y)}(a=5.0, b=6);

julia> @inferred bar(∂y)
ERROR: return type Tuple{Float64,Int64} does not match inferred return type Tuple{Float64,Union{Zero, Int64}}
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] top-level scope at REPL[13]:1

These uninferrabilities came up in JuliaDiff/ChainRules.jl#193 and JuliaDiff/ChainRules.jl#323 (comment), respectively.

With this PR, these tests now pass.

[codecov-io]

Codecov Report

Merging #279 (1acdd3f) into master (0e8f9c4) will decrease coverage by 1.73%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #279      +/-   ##
==========================================
- Coverage   89.58%   87.85%   -1.74%     
==========================================
  Files          13       13              
  Lines         461      420      -41     
==========================================
- Hits          413      369      -44     
- Misses         48       51       +3     

Impacted Files	Coverage Δ
src/ChainRulesCore.jl	100.00% <ø> (ø)
src/differentials/composite.jl	79.80% <100.00%> (-2.50%)	⬇️
src/differentials/thunks.jl	56.52% <0.00%> (-7.48%)	⬇️
src/ruleset_loading.jl	94.73% <0.00%> (-3.14%)	⬇️
src/rule_definition_tools.jl	93.57% <0.00%> (-0.74%)	⬇️
src/accumulation.jl	96.66% <0.00%> (-0.40%)	⬇️
src/differentials/abstract_zero.jl	93.75% <0.00%> (-0.37%)	⬇️
src/differential_arithmetic.jl	96.15% <0.00%> (-0.10%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e8f9c4...1acdd3f. Read the comment docs.

[mzgubic]

That's really cool, thanks for the PR! It looks good (after version bump) to go but I am reluctant to approve anything I don't fully understand.

It is the function Base.indexed_iterate, what is its purpose? There is no docstrings and I tried looking at the source. My understanding from that is that it iterates from the index i onwards for Tuples and Arrays, but from state onwards in the fallback case. I can' really see why this is needed?

[sethaxen]

It is the function Base.indexed_iterate, what is its purpose? There is no docstrings and I tried looking at the source. My understanding from that is that it iterates from the index i onwards for Tuples and Arrays, but from state onwards in the fallback case. I can' really see why this is needed?

indexed_iterate is what the expression a, b = (1, 2) gets lowered to. e.g.

julia> function foo(t)
           a, b = t
           return a, b
       end
foo (generic function with 1 method)

julia> @code_lowered foo((1,2))
CodeInfo(
1 ─ %1 = Base.indexed_iterate(t, 1)
│        a = Core.getfield(%1, 1)
│        @_3 = Core.getfield(%1, 2)
│   %4 = Base.indexed_iterate(t, 2, @_3)
│        b = Core.getfield(%4, 1)
│   %6 = Core.tuple(a, b)
└──      return %6

By default, indexed_iterate calls iterate, but this ends up not being type-inferrable. For Tuples, Arrays, etc, it seems indexed_iterate has been overloaded so that it is type-inferrable. But you're right, this feels strange that we need this.
And it seems that it is not a supported function (JuliaLang/julia#31765 (comment)). An alternative is to design iterate for tuple composites to default to using Val's for indices but this also seems like it might have some drawbacks.

[simeonschaub]

Doesn't have to be in this PR, but might also make sense to overload Base.rest in Julia >=1.6, so the newly added lhs slurping is also inferable.

[mzgubic]

Thanks for the explanation! I can't think of a better solution, and if this breaks in the future we can fix it then

[mzgubic]

Just bump the version please

[oxinabox]

I do recall reading a comment from a core developer that one shouldn't actually ever touch indexed_iterate.
that it was supposed to be purely an internal implementation detail.
but 🤷