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

Parsing of macro call with kw argument broken #20000

Closed
yuyichao opened this issue Jan 12, 2017 · 4 comments
Closed

Parsing of macro call with kw argument broken #20000

yuyichao opened this issue Jan 12, 2017 · 4 comments
Assignees
Labels
parser Language parsing and surface syntax

Comments

@yuyichao
Copy link
Contributor

julia> parse("@m(a; b=c)")
ERROR: ParseError("missing ) in argument list")
Stacktrace:
 [1] #parse#220(::Bool, ::Bool, ::Function, ::String, ::Int64) at ./parse.jl:180
 [2] (::Base.#kw##parse)(::Array{Any,1}, ::Base.#parse, ::String, ::Int64) at ./<missing>:0
 [3] #parse#221(::Bool, ::Function, ::String) at ./parse.jl:190
 [4] parse(::String) at ./parse.jl:190

This breaks the BenchmarkTools.jl test. Bisected to #19868 so I assume it isn't intentional. (I think we can make @m(a=b) parse as = while keep @(;a=b) to be parsed as :parameter since there isn't a similar case for parenthesisless syntax to be confused with)

@JeffBezanson
Copy link
Member

Should the expressions inside parameters be parsed as = or kw? Or maybe we should try to get rid of the kw expression head entirely?

@JeffBezanson JeffBezanson added the parser Language parsing and surface syntax label Jan 12, 2017
@yuyichao
Copy link
Contributor Author

Should the expressions inside parameters be parsed as = or kw?

Keep it as kw unless we are getting rid of kw altogether?

Or maybe we should try to get rid of the kw expression head entirely?

It's convenient to be able to do f($ex) for arbitrary expression and knowing that the result will be a call to f with the result as argument.

Not an argument for anything but I've just realized that we were always parsing = as kw in a call..... This seems pretty confusing too....

julia> Meta.show_sexpr(parse("m(a, (b[]=c))"))
(:call, :m, :a, (:kw, (:ref, :b), :c))

@andyferris
Copy link
Member

Segue: I'm not sure if "congratulations" is the correct word for the 20,000th issue (it means a lot of bug reports, improvements and hard work!), but it seems like a milestone to celebrate. 😄

@StefanKarpinski
Copy link
Member

20k issues is definitely a great milestone! It's a sure sign of an active, vibrant project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Language parsing and surface syntax
Projects
None yet
Development

No branches or pull requests

4 participants