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

Implement for f.(args...) syntax #31553

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Implement for f.(args...) syntax #31553

wants to merge 3 commits into from

Conversation

tkf
Copy link
Member

@tkf tkf commented Mar 30, 2019

This PR implements for f.(args...) syntax as discussed in #19198 and planned in #31088 (comment).

close #19198
close #31088

Implementation detail

  • A new Expr(:fordot, :(broadcasted.(expression))) AST node is introduced to represent for broadcasted.(expression).
  • This is lowered to a chain of broadcasted calls by using the pre-existing code in julia-syntax.scm but skips the outer-most materialize call.
  • for non(dot(call)) is a syntax error at the moment.
  • Lowering manually constructed for non(dot(call)) is an error at the moment.
  • There is no performance specialization in this PR. It is done separately in Faster mapreduce for Broadcasted #31020.

Demo

julia> ex = :(for f.(x))
:(for f.(x))

julia> ex.head
:fordot

julia> ex.args
1-element Array{Any,1}:
 :(f.(x))

julia> Meta.lower(Main, ex)
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope'
1 ─ %1 = Base.broadcasted(f, x)
└──      return %1
))))

julia> :(for f(x))
ERROR: syntax: invalid expression after `for`

julia> nondot = Expr(:fordot, :(f(x)))
:(for f(x))

julia> Meta.lower(Main, nondot)
:($(Expr(:error, "non-dot call after `for`")))

@@ -59,7 +62,6 @@ call. Finally, chains of comparisons have their own special expression structure
| `a==b` | `(call == a b)` |
| `1<i<=n` | `(comparison 1 < i <= n)` |
| `a.b` | `(. a (quote b))` |
| `a.(b)` | `(. a b)` |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed a.(b) in "Operators" table (here) and mention it as f.(x) in "Calls" table since it seems the documentation does not match with the implementation:

> (julia-parse "a.(b)")
(|.| a (tuple b))

@stevengj
Copy link
Member

Does it support dot operators? e.g. for x .+ sqrt.(y)?

@tkf
Copy link
Member Author

tkf commented Mar 30, 2019

Yes

julia> x = .- (1:3)
-1:-1:-3

julia> y = x .^ 2
3-element Array{Int64,1}:
 1
 4
 9

julia> collect(for x .+ sqrt.(y))
3-element Array{Float64,1}:
 0.0
 0.0
 0.0

Its parsing and lowering are tested like

julia/test/syntax.jl

Lines 1846 to 1847 in 1882eae

@test Meta.parse("for .+ x") == Expr(:fordot, :(.+ x))
@test Meta.parse("for .+ g(x)") == Expr(:fordot, :(.+ g(x)))

and

julia/test/syntax.jl

Lines 1865 to 1874 in 1882eae

@testset for valid in [
"for f.(x)"
"for f.(g(x))"
"for .+ x"
"for .+ g(x)"
"for f.(args[1], args[2])"
"for f.(args...)"
]
@test Meta.lower(@__MODULE__, Meta.parse(valid)).head == :thunk
end

Let me know if there are other useful test cases.

@chethega
Copy link
Contributor

So the correct use for iterating would be e.g. for i in for (0:10).^2 ... end resp for i = for (0:10).^2 ... end?

The doubling looks weirder than for i = @: (0:10).^2 ... end or for i = @lazy (0:10).^2 ... end.

@antoine-levitt
Copy link
Contributor

I would imagine this pattern is less common than reducers (e.g. sum(for a.*b)): if you're going to iterate it, might as well do full iteration: for i=0:10 and then do something with i^2.

@chethega
Copy link
Contributor

Fair enough. But Keno's original proposal in #31088 (comment) brought up exactly the point of lowering for i = (1:10).^2 body end into a non-materializing form that currently is spelled for i = for (0:10).^2 body end, at least if I understand this PR correctly.

@tkf
Copy link
Member Author

tkf commented Mar 31, 2019

a non-materializing form that currently is spelled for i = for (0:10).^2 body end, at least if I understand this PR correctly

@chethega Yes, that's correct. But this is not a new design. This is a consequence of what was proposed in the triage.

@diegozea
Copy link
Contributor

diegozea commented Apr 4, 2019

I feel like it would be better/more explicit a @lazy macro or similar than adding new syntax to something so basic like for

@tkf
Copy link
Member Author

tkf commented Apr 4, 2019

I've implemented the macro-based solution in #31088 (I used different macro name but renaming it is quite easy). At this point I think it's up to triage to decide and merge the preferable one.

@StefanKarpinski
Copy link
Member

I should mention that the for f.(args...) syntax was not universally loved and there was another syntax that got mentioned on another triage call that was a little strange at first but kind of makes sense: if we use &x to create a value reference and &v[i] to create an array reference and &x.f to create a field reference, then it could make sense to have &f(x) be a reference to the result of a function call, which would be lazy.

@tkf
Copy link
Member Author

tkf commented Apr 4, 2019

it could make sense to have &f(x) be a reference to the result of a function call

I think it does not work well if you want to denote Ref by & as well for the arguments of dot calls #27563 (comment). The tricky part here is that we have two "quasiquote/unquote-like" relationships: materialization/laziness and broadcasting/scalarization. Maybe it would be better to discuss #27563 and #19198 together?

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.

Notation for lazy map
6 participants