JuliaLang / JuliaSyntax.jl

The Julia compiler frontend
Other
272 stars 35 forks source link

`A where {T} == Z` parsing inconsistency with flisp parser #395

Open c42f opened 10 months ago

c42f commented 10 months ago

Tests are now failing on main due to a parsing inconsistency between JuliaSyntax and the flisp parser and a line in the JuliaLang/julia tests at

https://github.com/JuliaLang/julia/blob/6e23543bc477eb46e5fc8d5cab119190b990ed7c/test/core.jl#L8070

This can be reduced to an inconsistency parsing A where {T} == Z. One might expect this to parse with the == at precedence below the where, and indeed this is what happens in JuliaSyntax:

julia> JuliaSyntax.parsestmt(SyntaxNode, "A where {T} == Z")
line:col│ tree                                   │ file_name
   1:1  │[call-i]
   1:1  │  [where]
   1:1  │    A
   1:9  │    [braces]
   1:10 │      T
   1:13 │  ==
   1:16 │  Z

However, the flisp parser uses the following, apparently nonsensical precedence:

julia> dump(JuliaSyntax.fl_parse(Expr, "A where {T} == Z"))
Expr
  head: Symbol where
  args: Array{Any}((2,))
    1: Symbol A
    2: Expr
      head: Symbol call
      args: Array{Any}((3,))
        1: Symbol ==
        2: Expr
          head: Symbol braces
          args: Array{Any}((1,))
            1: Symbol T
        3: Symbol Z

The reason for this is that the flisp parser uses parse-comparsion for the trailing form because we want expressions without {} to allow subtype comparisons. That is, we want A where T <: Z to parse with the where at lower precedence than the <: as (where A (<: T Z)):

julia> JuliaSyntax.parsestmt(SyntaxNode, "A where T <: Z")
line:col│ tree                                   │ file_name
   1:1  │[where]
   1:1  │  A
   1:9  │  [<:]
   1:9  │    T
   1:14 │    Z

JuliaSyntax has a special case for trailing { for technical reasons which were originally unrelated to precedence. But it's arguably more useful and "correct" behavior.

So now we've got to figure out what to do about this. I think we should

  1. Make the tests tolerant to this difference (short term fix for the tests)
  2. Figure out a better, more stable way to do regression testing than always testing against the flisp parser and whatever code happens to be in Base on that version of Julia.

Alternatively we could revert to the behavior of the flisp parser. But given that alternative precedence seems confusing and non-useful I don't exactly favor this.

Originally discussed at https://github.com/JuliaLang/JuliaSyntax.jl/issues/380#issuecomment-1825564067

c42f commented 10 months ago

@vtjnash this is the root cause of https://github.com/JuliaLang/JuliaSyntax.jl/issues/380#issuecomment-1823210627

oscardssmith commented 4 months ago

Is https://github.com/JuliaLang/julia/issues/51999 also caused by this?

LilithHafner commented 3 months ago

This causes #444

LilithHafner commented 3 months ago

Probably related, and also probably for the best, the parsing of

@test Tuple{Integer, X, Vararg{X}} where {X<:Int} <: Tuple{Any, Vararg{X}} where {X>:Int}

Added by JuliaLang/julia#53034 has changed