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

Fix #18577, remove generated function broadcast_tup #18580

Merged
merged 1 commit into from
Sep 19, 2016

Conversation

pabloferz
Copy link
Contributor

No description provided.

@martinholters
Copy link
Member

This seems to work:

@generated function broadcast_tup{AT,nargs,n}(f, As::AT, ::Type{Val{nargs}}, ::Type{Val{n}})
    Expr(:tuple, (Expr(:call, :f, (:(_broadcast_getindex(As[$i], $k)) for i=1:nargs)...) for k=1:n)...)
end

along with a changed call from broadcast_c to give the last parameter as a Val, too. I'm not claiming this is the best solution, but at least it seems to be a solution.

@martinholters
Copy link
Member

IIUC, broadcast_c(f, ::Type{Tuple}, As...) is only called when all arguments As are Tuples or scalars. Then it might be worth to completely rewrite it to be inferable, but let's first try to fix the present issue ASAP in this PR.

@pabloferz pabloferz force-pushed the pz/broadcast_tup branch 2 times, most recently from 51dc53d to e4eafbc Compare September 19, 2016 14:24
@pabloferz
Copy link
Contributor Author

pabloferz commented Sep 19, 2016

@martinholters I updated with your version for now until we find a valid, inferrable and more efficient way of fixing this. By the way, at one moment I had Val{n} in #16986, but lost it somewhere between rebases.

@pabloferz
Copy link
Contributor Author

@vtjnash Do you have a better suggestion for fixing this?

@vtjnash
Copy link
Sponsor Member

vtjnash commented Sep 19, 2016

The non-generated version is also probably much more efficient:

function broadcast_tup(f, As, n)
    return (f((_broadcast_getindex(A, k) for A in As)...) for k = 1:n)...)
end

@pabloferz pabloferz changed the title Fix (#18577) invalid anonymous function in broadcast_tup generated function Fix #18577, remove generated function broadcast_tup Sep 19, 2016
@pabloferz
Copy link
Contributor Author

Removing the generated function seems the best option as this case wasn't inferable anyway (PR already updated accordingly).

@vtjnash vtjnash merged commit 6cde412 into JuliaLang:master Sep 19, 2016
@pabloferz pabloferz deleted the pz/broadcast_tup branch September 19, 2016 20:14
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 this pull request may close these issues.

3 participants