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

Destructuring not recognized #75

Open
henry2004y opened this issue Apr 10, 2022 · 6 comments
Open

Destructuring not recognized #75

henry2004y opened this issue Apr 10, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@henry2004y
Copy link

Hi,

I've encountered a case where Polyester.jl does not know how to handle the destructuring syntax introduced in Julia 1.7:

using Polyester

struct A
   a
   b
end

x = A(1,2)

Threads.@threads for i in 1:2
   (; a, b) = x
end

@batch for i in 1:2
   (; a, b) = x
end

The @batch loop returns

ERROR: LoadError: "Don't know how to handle:\n \$(Expr(:parameters, :a, :b))"
@chriselrod chriselrod added the bug Something isn't working label Apr 10, 2022
@Moelf
Copy link
Contributor

Moelf commented Oct 3, 2022

bump, do we need a pass in the macro to transform this into a bunch of getproperty?

@chriselrod
Copy link
Member

chriselrod commented Oct 3, 2022

bump, do we need a pass in the macro to transform this into a bunch of getproperty?

Yeah, that'd be a reasonable approach.
The alternative would be handling Expr(:parameters,...), where those parameters would be added to the symbols defined in the loop.

Want to take a stab at it?

@chriselrod
Copy link
Member

chriselrod commented Oct 3, 2022

function define_tup!(arguments::Vector{Symbol}, defined::Dict{Symbol,Symbol}, ex::Expr, mod)
for (i, a) enumerate(ex.args)
if a isa Symbol
ex.args[i] = getgensym!(defined, a)
elseif Meta.isexpr(a, :tuple)
define_tup!(defined, a)
elseif Meta.isexpr(a, :ref)
extractargs!(arguments, defined, a, mod)
else
throw("Don't know how to handle:\n $a")
end
end
end

can handle

julia> y = (1,2);

julia> @batch for i in 1:2
          (a, b) = y
       end

so it should be straightforward to adapt to support (; a, b) = x.

Although preprocessing this into getproperty calls would work too, and not require as much looking into the internals, at the cost of requiring an extra pass over the Expr making macro expansion slower.

@Moelf
Copy link
Contributor

Moelf commented Oct 4, 2022

where even is this signature defined

  define_tup!(defined, a) 

@Moelf
Copy link
Contributor

Moelf commented Oct 4, 2022

also this doesn't quite work, this logic assumes each element in ex.args gets replaced by a gensym

but when a is a :parameters expression, it needs to inflate into multiple entries in ex.args

@chriselrod
Copy link
Member

chriselrod commented Oct 4, 2022

where even is this signature defined

  define_tup!(defined, a) 

That method does not exist. That is a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants