Skip to content

Commit

Permalink
Make left hand side of -> a tuple of arguments (#522)
Browse files Browse the repository at this point in the history
This makes argument lists on the left hand side of `->` parse as tuples.
In particular, this fixes a very inconsistent case where the left hand
side previously parsed as a `K"block"`; we now have:

* `(a;x=1)->b` ==> `(-> (tuple-p a (parameters (= x 1))) b`

rather than `(block a (= x 1))` on the left hand side.

In addition, the following forms are treated consistently

* `a->b`       ==> `(-> (tuple a) b)`
* `(a)->b`     ==> `(-> (tuple-p a) b)`
* `(a,)->b`    ==> `(-> (tuple-p-, a) b)`

The upside of this is that expression processing of `->` syntax should
be much easier.

There's one aberrant case involving `where` which is particularly
difficult and still not dealt with:

`(x where T) -> b` does not parse as `(where (tuple x) T)` on the left
hand side. However, `where` precedence involving `::` and `->` is
already horribly broken and this syntax will always be awful to write
unless we make breaking changes. So I'm too tempted to call this a lost
cause for now 😬.

Compatibility shims for converting the `SyntaxNode` form back to `Expr`
in order to keep `Expr` stable are included. (At some point we should
consider fixing this and deleting these shims because the new form is so
much more consistent and would be reflected neatly into `Expr` form.)
  • Loading branch information
c42f authored Dec 27, 2024
1 parent 5ff59c2 commit 4a7e846
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 6 deletions.
1 change: 1 addition & 0 deletions docs/src/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class of tokenization errors and lets the parser deal with them.
* Standalone dotted operators are always parsed as `(. op)`. For example `.*(x,y)` is parsed as `(call (. *) x y)` (#240)
* The `K"="` kind is used for keyword syntax rather than `kw`, to avoid various inconsistencies and ambiguities (#103)
* Unadorned postfix adjoint is parsed as `call` rather than as a syntactic operator for consistency with suffixed versions like `x'ᵀ` (#124)
* The argument list in the left hand side of `->` is always a tuple. For example, `x->y` parses as `(-> (tuple x) y)` rather than `(-> x y)` (#522)

### Improvements to awkward AST forms

Expand Down
26 changes: 25 additions & 1 deletion src/expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,35 @@ function _internal_node_to_Expr(source, srcrange, head, childranges, childheads,
# Block for conditional's source location
args[1] = Expr(:block, loc, args[1])
elseif k == K"->"
a1 = args[1]
if @isexpr(a1, :tuple)
# TODO: This makes the Expr form objectively worse for the sake of
# compatibility. We should consider deleting this special case in
# the future as a minor change.
if length(a1.args) == 1 &&
(!has_flags(childheads[1], PARENS_FLAG) ||
!has_flags(childheads[1], TRAILING_COMMA_FLAG)) &&
!Meta.isexpr(a1.args[1], :parameters)
# `(a) -> c` is parsed without tuple on lhs in Expr form
args[1] = a1.args[1]
elseif length(a1.args) == 2 && (a11 = a1.args[1]; @isexpr(a11, :parameters) &&
length(a11.args) <= 1 && !Meta.isexpr(a1.args[2], :(...)))
# `(a; b=1) -> c` parses args as `block` in Expr form :-(
if length(a11.args) == 0
args[1] = Expr(:block, a1.args[2])
else
a111 = only(a11.args)
assgn = @isexpr(a111, :kw) ? Expr(:(=), a111.args...) : a111
argloc = source_location(LineNumberNode, source, last(childranges[1]))
args[1] = Expr(:block, a1.args[2], argloc, assgn)
end
end
end
a2 = args[2]
# Add function source location to rhs; add block if necessary
if @isexpr(a2, :block)
pushfirst!(a2.args, loc)
else
# Add block for source locations
args[2] = Expr(:block, loc, args[2])
end
elseif k == K"function"
Expand Down
20 changes: 17 additions & 3 deletions src/parser.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1409,8 +1409,21 @@ function parse_decl_with_initial_ex(ps::ParseState, mark)
emit(ps, mark, K"::", INFIX_FLAG)
end
if peek(ps) == K"->"
# x -> y ==> (-> x y)
# a::b->c ==> (-> (::-i a b) c)
kb = peek_behind(ps).kind
if kb == K"tuple"
# (x,y) -> z
# (x) -> y
# (x; a=1) -> y
elseif kb == K"where"
# `where` and `->` have the "wrong" precedence when writing anon functons.
# So ignore this case to allow use of grouping brackets with `where`.
# This needs to worked around in lowering :-(
# (x where T) -> y ==> (-> (x where T) y)
else
# x -> y ==> (-> (tuple x) y)
# a::b->c ==> (-> (tuple (::-i a b)) c)
emit(ps, mark, K"tuple")
end
bump(ps, TRIVIA_FLAG)
# -> is unusual: it binds tightly on the left and loosely on the right.
parse_eq_star(ps)
Expand Down Expand Up @@ -3073,7 +3086,8 @@ function parse_paren(ps::ParseState, check_identifiers=true)
initial_semi = peek(ps) == K";"
opts = parse_brackets(ps, K")") do had_commas, had_splat, num_semis, num_subexprs
is_tuple = had_commas || (had_splat && num_semis >= 1) ||
(initial_semi && (num_semis == 1 || num_subexprs > 0))
(initial_semi && (num_semis == 1 || num_subexprs > 0)) ||
(peek(ps, 2) == K"->" && peek_behind(ps).kind != K"where")
return (needs_parameters=is_tuple,
is_tuple=is_tuple,
is_block=num_semis > 0)
Expand Down
18 changes: 18 additions & 0 deletions test/expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,31 @@
@testset "->" begin
@test parsestmt("a -> b") ==
Expr(:->, :a, Expr(:block, LineNumberNode(1), :b))
@test parsestmt("(a,) -> b") ==
Expr(:->, Expr(:tuple, :a), Expr(:block, LineNumberNode(1), :b))
@test parsestmt("(a where T) -> b") ==
Expr(:->, Expr(:where, :a, :T), Expr(:block, LineNumberNode(1), :b))
# @test parsestmt("a -> (\nb;c)") ==
# Expr(:->, :a, Expr(:block, LineNumberNode(1), :b))
@test parsestmt("a -> begin\nb\nc\nend") ==
Expr(:->, :a, Expr(:block,
LineNumberNode(1),
LineNumberNode(2), :b,
LineNumberNode(3), :c))
@test parsestmt("(a;b=1) -> c") ==
Expr(:->,
Expr(:block, :a, LineNumberNode(1), Expr(:(=), :b, 1)),
Expr(:block, LineNumberNode(1), :c))
@test parsestmt("(a...;b...) -> c") ==
Expr(:->,
Expr(:tuple, Expr(:parameters, Expr(:(...), :b)), Expr(:(...), :a)),
Expr(:block, LineNumberNode(1), :c))
@test parsestmt("(;) -> c") ==
Expr(:->,
Expr(:tuple, Expr(:parameters)),
Expr(:block, LineNumberNode(1), :c))
@test parsestmt("a::T -> b") ==
Expr(:->, Expr(:(::), :a, :T), Expr(:block, LineNumberNode(1), :b))
end

@testset "elseif" begin
Expand Down
13 changes: 11 additions & 2 deletions test/parser.jl
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,18 @@ tests = [
"begin x end::T" => "(::-i (block x) T)"
# parse_decl_with_initial_ex
"a::b" => "(::-i a b)"
"a->b" => "(-> a b)"
"a::b::c" => "(::-i (::-i a b) c)"
"a::b->c" => "(-> (::-i a b) c)"
"a->b" => "(-> (tuple a) b)"
"(a,b)->c" => "(-> (tuple-p a b) c)"
"(a;b=1)->c" => "(-> (tuple-p a (parameters (= b 1))) c)"
"x::T->c" => "(-> (tuple (::-i x T)) c)"
# `where` combined with `->` still parses strangely. However:
# * It's extra hard to add a tuple around the `x` in this syntax corner case.
# * The user already needs to add additional, ugly, parens to get this
# to parse correctly because the precendence of `where` is
# inconsistent with `::` and `->` in this case.
"(x where T)->c" => "(-> (parens (where x T)) c)"
"((x::T) where T)->c" => "(-> (parens (where (parens (::-i x T)) T)) c)"
],
JuliaSyntax.parse_unary_subtype => [
"<: )" => "<:"
Expand Down

0 comments on commit 4a7e846

Please sign in to comment.