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.

Sign up for GitHub

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

Allow macrocall in function def syntax #456

Merged
merged 4 commits into from
Aug 2, 2024
Merged

Allow macrocall in function def syntax #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8543f48
Allow macrocall in function def syntax
Keno Jul 8, 2024
0dc7e7b
Update src/parser.jl
Keno Jul 26, 2024
5913117
Test to cover function declaration with `var""` syntax
c42f Jul 30, 2024
6f62399
Make `function (@f(x)) body end` an ambiguity error
c42f Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions src/parser.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2110,14 +2110,15 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
# * The whole function declaration, in parens
bump(ps, TRIVIA_FLAG)
is_empty_tuple = peek(ps, skip_newlines=true) == K")"
opts = parse_brackets(ps, K")") do _, _, _, _
opts = parse_brackets(ps, K")") do had_commas, had_splat, num_semis, num_subexprs
_parsed_call = was_eventually_call(ps)
_needs_parse_call = peek(ps, 2) ∈ KSet"( ."
_is_anon_func = !_needs_parse_call && !_parsed_call
_is_anon_func = (!_needs_parse_call && !_parsed_call) || had_commas
return (needs_parameters = _is_anon_func,
is_anon_func = _is_anon_func,
parsed_call = _parsed_call,
needs_parse_call = _needs_parse_call)
needs_parse_call = _needs_parse_call,
maybe_grouping_parens = !had_commas && !had_splat && num_semis == 0 && num_subexprs == 1)
end
is_anon_func = opts.is_anon_func
parsed_call = opts.parsed_call
Expand All @@ -2128,7 +2129,14 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
# function (x,y) end ==> (function (tuple-p x y) (block))
# function (x=1) end ==> (function (tuple-p (= x 1)) (block))
# function (;x=1) end ==> (function (tuple-p (parameters (= x 1))) (block))
# function (f(x),) end ==> (function (tuple-p (call f x)) (block))
ambiguous_parens = opts.maybe_grouping_parens &&
peek_behind(ps).kind in KSet"macrocall $"
emit(ps, mark, K"tuple", PARENS_FLAG)
if ambiguous_parens
# Got something like `(@f(x))`. Is it anon `(@f(x),)` or named sig `@f(x)` ??
emit(ps, mark, K"error", error="Ambiguous signature. Add a trailing comma if this is a 1-argument anonymous function; remove parentheses if this is a macro call acting as function signature.")
end
elseif is_empty_tuple
# Weird case which is consistent with parse_paren but will be
# rejected in lowering
Expand Down Expand Up @@ -2157,19 +2165,23 @@ function parse_function_signature(ps::ParseState, is_function::Bool)
end
end
end
if peek(ps, skip_newlines=true) == K"end" && !is_anon_func && !parsed_call
return false
end
if needs_parse_call
# Parse function argument list
# function f(x,y) end ==> (function (call f x y) (block))
# function f{T}() end ==> (function (call (curly f T)) (block))
# function A.f() end ==> (function (call (. A f)) (block))
parse_call_chain(ps, mark)
if peek_behind(ps).kind != K"call"
sig_kind = peek_behind(ps).kind
if sig_kind in KSet"Identifier var $" && peek(ps, skip_newlines=true) == K"end"
# function f end ==> (function f)
# function $f end ==> (function $f)
return false
elseif sig_kind == K"macrocall"
min_supported_version(v"1.12", ps, mark, "macro call as function signature")
elseif sig_kind != K"call"
# function f body end ==> (function (error f) (block body))
emit(ps, mark, K"error",
error="Invalid signature in $(is_function ? "function" : "macro") definition")
error="Invalid signature in $(is_function ? "function" : "macro") definition")
end
end
if is_function && peek(ps) == K"::"
Expand Down Expand Up @@ -3494,7 +3506,11 @@ function parse_atom(ps::ParseState, check_identifiers=true)
# + ==> +
# .+ ==> (. +)
# .= ==> (. =)
bump_dotsplit(ps, emit_dot_node=true)
if is_dotted(peek_token(ps))
bump_dotsplit(ps, emit_dot_node=true)
else
bump(ps, remap_kind=K"Identifier")
end
Comment on lines +3509 to +3513
Copy link
Member

Choose a reason for hiding this comment

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

bump_dotsplit has a remap_kind argument

Suggested change
if is_dotted(peek_token(ps))
bump_dotsplit(ps, emit_dot_node=true)
else
bump(ps, remap_kind=K"Identifier")
end
bump_dotsplit(ps, emit_dot_node=true, remap_kind=K"Identifier")

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't work, because that also remaps the dotted version, which is undesirable.

Copy link
Member

Choose a reason for hiding this comment

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

I think it works for SyntaxNode but then there's an error in Expr conversion. So we can leave that for now, I'll open a separate issue for it. But we should be consistent.

Why did you need to have bump(ps, remap_kind=K"Identifier") here anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why did you need to have bump(ps, remap_kind=K"Identifier") here anyway?

Just to avoid having an explit is_operator check on the sig_kind in KSet"Identifier var $". It also seems odd that e.g. isa gets turned into Identifiers, but + does not. I'm happy with either way though.

if check_identifiers && !is_valid_identifier(leading_kind)
# += ==> (error +=)
# ? ==> (error ?)
Expand Down
3 changes: 3 additions & 0 deletions test/diagnostics.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ end
@test diagnostic("\n+ (x, y)") ==
Diagnostic(3, 3, :error, "whitespace not allowed between prefix function call and argument list")

@test diagnostic("function (\$f) body end") ==
Diagnostic(10, 13, :error, "Ambiguous signature. Add a trailing comma if this is a 1-argument anonymous function; remove parentheses if this is a macro call acting as function signature.")

@test diagnostic("A.@B.x", only_first=true) ==
Diagnostic(3, 4, :error, "`@` must appear on first or last macro name component")
@test diagnostic("@M.(x)") ==
Expand Down
14 changes: 14 additions & 0 deletions test/parser.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,11 @@ tests = [
"function (x,y) end" => "(function (tuple-p x y) (block))"
"function (x=1) end" => "(function (tuple-p (= x 1)) (block))"
"function (;x=1) end" => "(function (tuple-p (parameters (= x 1))) (block))"
"function (f(x),) end" => "(function (tuple-p (call f x)) (block))"
"function (@f(x);) end" => "(function (tuple-p (macrocall-p @f x) (parameters)) (block))"
"function (@f(x)...) end" => "(function (tuple-p (... (macrocall-p @f x))) (block))"
"function (@f(x)) end" => "(function (error (tuple-p (macrocall-p @f x))) (block))"
"function (\$f) end" => "(function (error (tuple-p (\$ f))) (block))"
"function ()(x) end" => "(function (call (tuple-p) x) (block))"
"function (A).f() end" => "(function (call (. (parens A) f)) (block))"
"function (:)() end" => "(function (call (parens :)) (block))"
Expand All @@ -581,6 +586,7 @@ tests = [
"function f end" => "(function f)"
"function f \n\n end" => "(function f)"
"function \$f end" => "(function (\$ f))"
"function var\".\" end" => "(function (var .))"
"macro f end" => "(macro f)"
# Function argument list
"function f(x,y) end" => "(function (call f x y) (block))"
Expand All @@ -603,6 +609,11 @@ tests = [
# body
"function f() \n a \n b end" => "(function (call f) (block a b))"
"function f() end" => "(function (call f) (block))"
# Macrocall as sig
((v=v"1.12",), "function @callmemacro(a::Int) \n 1 \n end") => "(function (macrocall-p @callmemacro (::-i a Int)) (block 1))"
((v=v"1.12",), "function @callmemacro(a::T, b::T) where T <: Int64\n3\nend") => "(function (where (macrocall-p @callmemacro (::-i a T) (::-i b T)) (<: T Int64)) (block 3))"
((v=v"1.12",), "function @callmemacro(a::Int, b::Int, c::Int)::Float64\n4\nend") => "(function (::-i (macrocall-p @callmemacro (::-i a Int) (::-i b Int) (::-i c Int)) Float64) (block 4))"
((v=v"1.12",), "function @f()() end") => "(function (call (macrocall-p @f)) (block))"
# Errors
"function" => "(function (error (error)) (block (error)) (error-t))"
],
Expand Down Expand Up @@ -963,6 +974,9 @@ tests = [
"public[7] = 5" => "(= (ref public 7) 5)"
"public() = 6" => "(= (call public) 6)"
]),
JuliaSyntax.parse_stmts => [
((v = v"1.12",), "@callmemacro(b::Float64) = 2") => "(= (macrocall-p @callmemacro (::-i b Float64)) 2)"
],
JuliaSyntax.parse_docstring => [
""" "notdoc" ] """ => "(string \"notdoc\")"
""" "notdoc" \n] """ => "(string \"notdoc\")"
Expand Down
Loading