elseif raises an error when used with both short-circuiting evaluation and anonymous function

[adienes]

See some MWE here https://discourse.julialang.org/t/weird-syntax-error/89591/6, seems to occur on all recent versions.

[vtjnash]

julia> if false
       elseif false || (()->true)()
       end
ERROR: syntax: invalid syntax false || begin
    nothing
    begin
        nothing
        #5 = (new #5#6)
        (unnecessary #5)
    end
end()
Stacktrace:
 [1] top-level scope
   @ REPL[1]:1

[vtjnash]

Note this was a regression in v1.6 (vs. v1.5). From the error message, it looks like it accidentally pulled apart the call expression too much and lost the ->. Broken for both && and ||. Note that it fails for all function definition syntaxes (named and anonymous). Note that it only fails for toplevel expressions, but works correctly inside a function.

julia> if false
              elseif false && (f() = true)()
              end
ERROR: syntax: invalid syntax false && begin
    (method (outerref f))
    begin
        (method (outerref f) (call (core svec) (call (core svec) (call (core Typeof) (outerref f))) (call (core svec)) (inert #0=(line 2 REPL[1]))) (lambda (#self#) (((#self# (core Any) 0)) () 0 ()) (block #0# (true))))
    end
    (unnecessary (outerref f))
end()
Stacktrace:
 [1] top-level scope
   @ REPL[1]:1

side-note: precedence of this operator printing is not great:

./julia --lisp
> (deparse '(call (-> (tuple) (block nothing))))
() -> begin
    nothing
end()

since this would actually be a call to implicit multiplication of *(::var"#5#6", ::Tuple{})

[Moelf]

is there a faster way to bisect this other than doing it wholesale?

[KristofferC]

Note this was a regression in v1.6 (vs. v1.5).

So could at least narrow it down to that range.

[vtjnash]

you can do git bisect start origin/release-1.6 origin/release-1.5 -- src/julia-syntax.scm, which really cuts down the number of commits to check to just these:

$ git log --oneline origin/release-1.5..origin/release-1.6 -- src/julia-syntax.scm 
3c8241cef6 fix #44239, regression in keyword args in getindex (#44246)
8a02c32252 fix #43960, evaluation order of splat inside ref (#44024)
dede9df042 fix #39705: lowering of Expr(:||) (#39709)
e4b9fd9da0 improve constraint propagation with multiple || (#39618)
85b97525dd fix #41416, splatted default argument lost with keyword argument (#41427)
38ef73982d handle kwarg lowering for vararg with default value (#40977)
b97d605264 fix #40258: nested string interpolation (#40261)
295995aa07 fix #39600: broadcast fusion broken for comparison (#39602)
5f6094a6e2 fix bug in `let` when a global var is both shadowed and used in an RHS (#39570)
88f445a7b8 fix #38501, wrap interpolated literal strings in `:string` exprs (#38692)
7d5c0c5633 fix part of #38650, `struct` should be a hard scope (#38663)
59d676c957 streamline a[begin] lowering via firstindex(a, n) (#35779)
28ff641fa9 no call to Base.rest if vararg is all-underscore (#38403)
9f0682d28f fix #33460: better error messages on multiple semicolons in calls (#38202)
280b9eafbf fix bug in lowering `if` with certain `block` exprs as the condition (#38222)
78ade6c5f2 add `rest` for `NamedTuple` and fix some destructuring cases (#38214)
6edf6d9e15 allow slurping in lhs of assignment (#37410)
31994ac8e2 produce `LineInfoNode` directly in lowering (#37954)
5208a509e0 fix #37690, bug in `lambda-optimize-vars!` on outer variables assigned in loops (#37736)
c402113cbc fix #37890, fix `new` error checks with varargs (#37894)
62eda52b39 allow standalone dotted ops, parse .op as (. op) (#37583)
2479691a5a fix #37677, unreliable lowering of assignments to gensym'd names (#37717)
dbd3d4cd07 fix #37413, propagating `&&` conditions with `elseif` (#37453)
5cba88c5bb allow ccall library name to be non-constant (#37123)
f3c3720eb9 lowering: Drop unused name expr for non-symbol names (#37287)
35ac635c95 fix exception stack pop for UndefVarError in catch block (#37269)
662e24ccd4 fix #37228, lowering regression in `if a && ...` (#37245)
9ae0a73869 fix #37126, spurious soft scope warning on hidden name (#37178)
371bfa89d4 normalize parsing of `-->` (#36793)
5c70ee6dca speed up part of resolve-scopes lowering pass
1f8ace674a fix #36572, destructuring syntax for called object (#36586)
16ba0dd6ee Fix exception stack lowering in finally handlers (#36480)
35c1f87176 fix #36230, more efficient lowering of `if` with a chain of `&&` (#36355)
70fe622ee4 remove unnecessary Box when an argument is used before assigned (#36245)
095e92d4e9 fix #36104, assign global name during type definitions (#36121)
39e360c5a8 fix #34516, several issues with nospecialize on keyword arguments (#36019)
759e78ccff Display argument that is duplicated in function calls (#35857)
db9063138b allow `where` syntax in constructor definitions (#35861)

[pjssilva]

Be careful,

@rdeits said in Discourse that the code does not error in 1.6.2 for him. See

https://discourse.julialang.org/t/weird-syntax-error/89591/8?u=pjssilva

It does not work in 1.6.7 for me. So the regression may have happened between point releases.

[gbaraldi]

This was added between 1.6.2 and 1.6.3, which reduces it to

 git log --oneline v1.6.2..v1.6.3 -- src/julia-syntax.scm               (base)
dede9df042 fix #39705: lowering of Expr(:||) (#39709)
e4b9fd9da0 improve constraint propagation with multiple || (#39618)

[Moelf]

I mean it could have been "good" -- "bad" -- "good" -- "bad"

we probably want the last bad? LOL, those two commites makes sense...

• fix #39705: lowering of Expr(:||) #39709
• improve constraint propagation with multiple || #39618