Support for named tuple destructuring

[yha]

e.g.

@benchmark let 
    (; x, y) = $p
    f(x,y)
end

currently fails at macro expansion

[Zentrik]

Is it possible for after args = first(args).args for args to still be an Expr? Should we keep expanding till it's not?

I don't really know much about what this code is doing but it looks alright to me.

[willow-ahrens]

Do we know if this is robust across julia versions? I'd like to see a more selective test, that perhaps checks the head of some of these exprs. to see that they are namedtuples.

e.g. probably we should see :parameters somewhere in here:

julia> dump(:((; x, y) = a))
Expr
  head: Symbol =
  args: Array{Any}((2,))
    1: Expr
      head: Symbol tuple
      args: Array{Any}((1,))
        1: Expr
          head: Symbol parameters
          args: Array{Any}((2,))
            1: Symbol x
            2: Symbol y
    2: Symbol a
    ```

[yha]

Added more selective test, adding an error message in unrecognized cases.
The only cases I can think of that would initially parse but than trigger the error are the invalid forms (; x, y=3) = z and (x, y; z) = q.

[willow-ahrens]

This isn't even about the interpolation with dollar. For example, this also fails:

@benchmark let 
           (; x, y) = p
           f(x,y)
       end

[yha]

Is it possible for after args = first(args).args for args to still be an Expr? Should we keep expanding till it's not?

I don't really know much about what this code is doing but it looks alright to me.

I don't think that's possible, at least assuming valid Julia code is passed to @benchmark. I believe the args after that line can be an Expr only in a case like (; x = (a,b), y) which is not a valid lhs for assignment (and in that case the head of that would be :kw, not another :tuple, so it would require more complicated recursion if the goal is to make a better error message. But it's an unlikely mistake from the user anyway)

[willow-ahrens]

This is a good start, just a few small tweaks! Thanks for your contribution!

[willow-ahrens]

These tests are good, but if I understand correctly, I think this is missing the case where we interpolate a value from local scope with dollar, since that's part of the code that this PR changes.

[yha]

I've added tests for interpolated values both for the tuple and named tuple case (though I don't think the code I changed is specific to interpolation in any way).

[willow-ahrens]

Do we know if this is robust across julia versions? I'd like to see a more selective test, that perhaps checks the head of some of these exprs. to see that they are namedtuples.

e.g. probably we should see :parameters somewhere in here:

julia> dump(:((; x, y) = a))
Expr
  head: Symbol =
  args: Array{Any}((2,))
    1: Expr
      head: Symbol tuple
      args: Array{Any}((1,))
        1: Expr
          head: Symbol parameters
          args: Array{Any}((2,))
            1: Symbol x
            2: Symbol y
    2: Symbol a
    ```

[willow-ahrens]

Let's try to fix this for the general case

[willow-ahrens]

Okay, so long as we're fixing this, let's fix this for real. There are two cases of destructuring to support: namedtuple and possibly nested tuple (which may contain namedtuple).

Let's structure the code then as:

elseif isa(lhs, Expr) && lhs.head == :tuple && length(args) == 1 && lhs.args[1].head === :parameters && all(arg->isa(arg, Symbol)), lhs.args[1].args)
    append!(vars, lhs.args[1].args)
elseif isa(lhs, Expr) && lhs.head == :tuple
    for arg in lhs.args
        if arg isa Symbol
            push!(vars, arg)
        else
            collectvars(arg, vars)
        end
    end
elseif

and let's also add a test for (a, (b, c)) = rhs as well as (a, (;x, y)) = rhs