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

0.4.8 lets a.[1] parse and lower #410

Open
LilithHafner opened this issue Feb 1, 2024 · 11 comments
Open

0.4.8 lets a.[1] parse and lower #410

LilithHafner opened this issue Feb 1, 2024 · 11 comments

Comments

@LilithHafner
Copy link
Member

This came up at JuliaLang/julia#53119

@LilithHafnerBot bisect()

using JuliaSyntax
JuliaSyntax.parse("a.[1]")
@savq
Copy link
Contributor

savq commented Feb 1, 2024

Note that this was allowed in the flisp parser.

julia +lts -E ':(a.[1])'                                                                                                                                      
:(a.:([1]))

@LilithHafner
Copy link
Member Author

There was a bug in the bisector backend. Should hopefully be fixed now.

@LilithHafnerBot bisect()

using JuliaSyntax
JuliaSyntax.parse(Expr, "a.[1]")

@LilithHafnerBot
Copy link

❌ Bisect failed

Commit stdout
b1c6d54 a.:([1])
a6f2d15 a.:([1])

@LilithHafner
Copy link
Member Author

@LilithHafnerBot bisect()

using JuliaSyntax
JuliaSyntax.enable_in_core!()
eval(Meta.parse("""
Meta.lower(Main, :(a.[1]))
"""))

@LilithHafnerBot
Copy link

✅ Bisect succeeded! The first new commit is 296cd5e

Commit stdout
b1c6d54 $(Expr(:error, "invalid syntax "a.[1]""))
e3e447d $(Expr(:error, "invalid syntax "a.[1]""))
8731bab $(Expr(:error, "invalid syntax "a.[1]""))
6da0fc4 $(Expr(:error, "invalid syntax "a.[1]""))
5aad812 $(Expr(:error, "invalid syntax "a.[1]""))
296cd5e $(Expr(:thunk, CodeInfo(⏎ @ none within top-level scope⏎1 ─ %1 = Base.getproperty(a, $(Quote...
df84e02 $(Expr(:thunk, CodeInfo(⏎ @ none within top-level scope⏎1 ─ %1 = Base.getproperty(a, $(Quote...
a6f2d15 $(Expr(:thunk, CodeInfo(⏎ @ none within top-level scope⏎1 ─ %1 = Base.getproperty(a, $(Quote...

@LilithHafner
Copy link
Member Author

@LilithHafnerBot bisect()

using JuliaSyntax
dump(JuliaSyntax.parse(Expr, "a.[1]"))

@LilithHafnerBot
Copy link

✅ Bisect succeeded! The first new commit is 296cd5e

Commit stdout
b1c6d54 Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: Expr⏎ head: Symbol quo...
e3e447d Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: Expr⏎ head: Symbol quo...
8731bab Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: Expr⏎ head: Symbol quo...
6da0fc4 Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: Expr⏎ head: Symbol quo...
5aad812 Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: Expr⏎ head: Symbol quo...
296cd5e Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: QuoteNode⏎ value: Expr...
df84e02 Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: QuoteNode⏎ value: Expr...
a6f2d15 Expr⏎ head: Symbol .⏎ args: Array{Any}((2,))⏎ 1: Symbol a⏎ 2: QuoteNode⏎ value: Expr...

@LilithHafner
Copy link
Member Author

Alas, @savq, the old and new parsing results both print to :(a.:([1])), but they are not the same.

julia> old = Expr(:., :a, Expr(:quote, :([1])))
:(a.:([1]))

julia> new = Expr(:., :a, QuoteNode(:([1])))
:(a.:([1]))

julia> old == new
false

julia> Meta.lower(Main, old)
:($(Expr(:error, "invalid syntax \"a.[1]\"")))

julia> Meta.lower(Main, new)
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope'
1 ─ %1 = Base.getproperty(a, $(QuoteNode(:([1]))))
└──      return %1
))))

From the commit message, this looks intentional; perhaps the appropriate fix is to prohibit this in lowering (and possibly gate the new parsing to new versions of Julia)

@pfitzseb
Copy link
Member

pfitzseb commented Feb 1, 2024

Are Expr(:quote, ...) and QuoteNode(...) semantically identical?

@c42f
Copy link
Member

c42f commented Jul 23, 2024

Are Expr(:quote, ...) and QuoteNode(...) semantically identical?

No, but the difference is subtle.

  • quote does $ interpolation (in lisp, this is called quasiquote)
  • QuoteNode (or equivalently, Expr(:inert)) doesn't participate in $ interpolation (somewhat confusingly, this is called quote in lisp)

@c42f
Copy link
Member

c42f commented Jul 23, 2024

Note that the old parser uses QuoteNode in the special case that $ interpolation comes after the .:

julia> dump(JuliaSyntax.fl_parse(raw"x.$y"))
Expr
  head: Symbol .
  args: Array{Any}((2,))
    1: Symbol x
    2: QuoteNode
      value: Expr
        head: Symbol $
        args: Array{Any}((1,))
          1: Symbol y

QuoteNode rather than quote is required in this case so that the following works

julia> let
           field_name = :a
           quote
               x.$field_name
           end
       end
quote
    #= REPL[10]:4 =#
    x.a
end

But in the old parser, the following doesn't work because use of quote for anything other than $ after the x. means that the $field_name is double quoted (ie, "inside two quotes") in the Expr tree:

julia> let
           field_name = :a
           quote
               x.[$field_name]
           end
       end
quote
    #= REPL[11]:4 =#
    x.:([$(Expr(:$, :field_name))])
end

However, after #324, we have

julia> JuliaSyntax.enable_in_core!()

julia> let
           field_name = :a
           quote
               x.[$field_name]
           end
       end
quote
    #= REPL[13]:4 =#
    x.:([a])
end

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

No branches or pull requests

5 participants