Skip to content

Commit 3dcc960

Browse files
c42fc42f
authored
Simplify child layout for try (#234)
The child layout of try-catch-else-finally is awkward because several of the subclauses are optional. Particularly this has led to problems for `else` in `Expr` which needed to be tacked onto the end for compatibility. This change clarifies the situation a bit and makes it more future proof by wrapping the subclauses in their own expression heads.
1 parent f762e76 commit 3dcc960

File tree

6 files changed

+77
-70
lines changed

6 files changed

+77
-70
lines changed

‎src/expr.jl

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ function _to_expr(node::SyntaxNode; iteration_spec=false, need_linenodes=true,
5353
# TODO: Get non-token error messages in here as well, somehow?
5454
# There's an awkward mismatch between the out-of-tree
5555
# `Vector{Diagnostic}` vs Expr(:error) being part of the tree.
56-
return Expr(:error,
57-
"$(_token_error_descriptions[nodekind]): `$(sourcetext(node))`"
58-
)
56+
return nodekind == K"error" ?
57+
Expr(:error) :
58+
Expr(:error, "$(_token_error_descriptions[nodekind]): `$(sourcetext(node))`")
5959
else
6060
return val
6161
end
@@ -215,33 +215,38 @@ function _to_expr(node::SyntaxNode; iteration_spec=false, need_linenodes=true,
215215
@check all(Meta.isexpr(args[j], :error) for j in 2:length(args))
216216
return Expr(:block, args...)
217217
end
218-
elseif headsym in (:try, :try_finally_catch)
218+
elseif headsym === :try
219219
# Try children in source order:
220220
# try_block catch_var catch_block else_block finally_block
221221
# Expr ordering:
222222
# try_block catch_var catch_block [finally_block] [else_block]
223-
catch_ = nothing
224-
if headsym === :try_finally_catch
225-
catch_ = pop!(args)
226-
catch_var = pop!(args)
227-
end
228-
finally_ = pop!(args)
229-
else_ = pop!(args)
230-
if headsym === :try_finally_catch
231-
pop!(args)
232-
pop!(args)
233-
push!(args, catch_var)
234-
push!(args, catch_)
223+
try_ = args[1]
224+
catch_var = false
225+
catch_ = false
226+
else_ = false
227+
finally_ = false
228+
for i in 2:length(args)
229+
a = args[i]
230+
if Meta.isexpr(a, :catch)
231+
catch_var = a.args[1]
232+
catch_ = a.args[2]
233+
elseif Meta.isexpr(a, :else)
234+
else_ = only(a.args)
235+
elseif Meta.isexpr(a, :finally)
236+
finally_ = only(a.args)
237+
elseif Meta.isexpr(a, :error)
238+
finally_ = Expr(:block, a) # Unclear where to put this but here will do?
239+
else
240+
@check false "Illegal $a subclause in `try`"
241+
end
235242
end
236-
# At this point args is
237-
# [try_block catch_var catch_block]
243+
args = Any[try_, catch_var, catch_]
238244
if finally_ !== false || else_ !== false
239245
push!(args, finally_)
246+
if else_ !== false
247+
push!(args, else_)
248+
end
240249
end
241-
if else_ !== false
242-
push!(args, else_)
243-
end
244-
headsym = :try
245250
elseif headsym === :filter
246251
pushfirst!(args, last(args))
247252
pop!(args)

‎src/kinds.jl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -907,8 +907,6 @@ const _kind_names =
907907
"flatten"
908908
"comprehension"
909909
"typed_comprehension"
910-
# Special kind for compatibility with the ever-ugly try-finally-catch ordering
911-
"try_finally_catch"
912910
"END_SYNTAX_KINDS"
913911
]
914912

‎src/parser.jl

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,97 +2201,89 @@ end
22012201

22022202
# Parse a try block
22032203
#
2204-
# try \n x \n catch e \n y \n finally \n z end ==> (try (block x) e (block y) false (block z))
2205-
#v1.8: try \n x \n catch e \n y \n else z finally \n w end ==> (try (block x) e (block y) (block z) (block w))
2204+
# try \n x \n catch e \n y \n finally \n z end ==> (try (block x) (catch e (block y)) (finally (block z)))
2205+
#v1.8: try \n x \n catch e \n y \n else z finally \n w end ==> (try (block x) (catch e (block y)) (else (block z)) (finally (block w)))
22062206
#
22072207
# flisp: embedded in parse_resword
22082208
function parse_try(ps)
2209-
out_kind = K"try"
22102209
mark = position(ps)
22112210
bump(ps, TRIVIA_FLAG)
22122211
parse_block(ps)
22132212
has_catch = false
2214-
has_else = false
22152213
has_finally = false
22162214
bump_trivia(ps)
2217-
flags = EMPTY_FLAGS
2218-
bump_trivia(ps)
22192215
if peek(ps) == K"catch"
22202216
has_catch = true
22212217
parse_catch(ps)
2222-
else
2223-
bump_invisible(ps, K"false")
2224-
bump_invisible(ps, K"false")
22252218
end
22262219
bump_trivia(ps)
22272220
if peek(ps) == K"else"
22282221
# catch-else syntax: https://github.com/JuliaLang/julia/pull/42211
22292222
#
2230-
#v1.8: try catch ; else end ==> (try (block) false (block) (block) false)
2231-
has_else = true
2223+
#v1.8: try catch ; else end ==> (try (block) (catch false (block)) (else (block)))
22322224
else_mark = position(ps)
22332225
bump(ps, TRIVIA_FLAG)
22342226
parse_block(ps)
22352227
if !has_catch
2236-
#v1.8: try else x finally y end ==> (try (block) false false (error (block x)) (block y))
2228+
#v1.8: try else x finally y end ==> (try (block) (else (error (block x))) (finally (block y)))
22372229
emit(ps, else_mark, K"error", error="Expected `catch` before `else`")
22382230
end
2239-
#v1.7: try catch ; else end ==> (try (block) false (block) (error (block)) false)
2231+
#v1.7: try catch ; else end ==> (try (block) (catch false (block)) (else (error (block))))
22402232
min_supported_version(v"1.8", ps, else_mark, "`else` after `catch`")
2241-
else
2242-
bump_invisible(ps, K"false")
2233+
emit(ps, else_mark, K"else")
22432234
end
22442235
bump_trivia(ps)
22452236
if peek(ps) == K"finally"
2246-
# try x finally y end ==> (try (block x) false false false (block y))
2237+
finally_mark = position(ps)
2238+
# try x finally y end ==> (try (block x) (finally (block y)))
22472239
has_finally = true
22482240
bump(ps, TRIVIA_FLAG)
22492241
parse_block(ps)
2250-
else
2251-
bump_invisible(ps, K"false")
2242+
emit(ps, finally_mark, K"finally")
22522243
end
22532244
# Wart: the flisp parser allows finally before catch, the *opposite* order
22542245
# in which these blocks execute.
22552246
bump_trivia(ps)
22562247
if !has_catch && peek(ps) == K"catch"
2257-
# try x finally y catch e z end ==> (try_finally_catch (block x) false false false (block y) e (block z))
2258-
out_kind = K"try_finally_catch"
2248+
# try x finally y catch e z end ==> (try (block x) (finally (block y)) (catch e (block z)))
22592249
m = position(ps)
22602250
parse_catch(ps)
22612251
emit_diagnostic(ps, m,
22622252
warning="`catch` after `finally` will execute out of order")
22632253
end
22642254
missing_recovery = !has_catch && !has_finally
22652255
if missing_recovery
2266-
# try x end ==> (try (block x) false false false false (error-t))
2256+
# try x end ==> (try (block x) (error-t))
22672257
bump_invisible(ps, K"error", TRIVIA_FLAG)
22682258
end
22692259
bump_closing_token(ps, K"end")
2270-
emit(ps, mark, out_kind, flags)
2260+
emit(ps, mark, K"try")
22712261
if missing_recovery
22722262
emit_diagnostic(ps, mark, error="try without catch or finally")
22732263
end
22742264
end
22752265

22762266
function parse_catch(ps::ParseState)
2267+
mark = position(ps)
22772268
bump(ps, TRIVIA_FLAG)
22782269
k = peek(ps)
22792270
if k in KSet"NewlineWs ;" || is_closing_token(ps, k)
2280-
# try x catch end ==> (try (block x) false (block) false false)
2281-
# try x catch ; y end ==> (try (block x) false (block y) false false)
2282-
# try x catch \n y end ==> (try (block x) false (block y) false false)
2271+
# try x catch end ==> (try (block x) (catch false (block)))
2272+
# try x catch ; y end ==> (try (block x) (catch false (block y)))
2273+
# try x catch \n y end ==> (try (block x) (catch false (block y)))
22832274
bump_invisible(ps, K"false")
22842275
else
2285-
# try x catch e y end ==> (try (block x) e (block y) false false)
2286-
# try x catch $e y end ==> (try (block x) ($ e) (block y) false false)
2287-
mark = position(ps)
2276+
# try x catch e y end ==> (try (block x) (catch e (block y)))
2277+
# try x catch $e y end ==> (try (block x) (catch ($ e) (block y)))
2278+
m = position(ps)
22882279
parse_eq_star(ps)
22892280
if !(peek_behind(ps).kind in KSet"Identifier $")
2290-
# try x catch e+3 y end ==> (try (block x) (error (call-i e + 3)) (block y) false false)
2291-
emit(ps, mark, K"error", error="a variable name is expected after `catch`")
2281+
# try x catch e+3 y end ==> (try (block x) (catch (error (call-i e + 3)) (block y)))
2282+
emit(ps, m, K"error", error="a variable name is expected after `catch`")
22922283
end
22932284
end
22942285
parse_block(ps)
2286+
emit(ps, mark, K"catch")
22952287
end
22962288

22972289
# flisp: parse-do

‎test/expr.jl

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,19 @@
303303
Expr(:block, LineNumberNode(1), :y),
304304
Expr(:block, LineNumberNode(1), :w),
305305
Expr(:block, LineNumberNode(1), :z))
306+
# finally before catch
307+
@test parse(Expr, "try x finally y catch e z end", ignore_warnings=true) ==
308+
Expr(:try,
309+
Expr(:block, LineNumberNode(1), :x),
310+
:e,
311+
Expr(:block, LineNumberNode(1), :z),
312+
Expr(:block, LineNumberNode(1), :y))
313+
# empty recovery
314+
@test parse(Expr, "try x end", ignore_errors=true) ==
315+
Expr(:try,
316+
Expr(:block, LineNumberNode(1), :x),
317+
false, false,
318+
Expr(:block, Expr(:error)))
306319
end
307320

308321
@testset "juxtapose" begin

‎test/parser.jl

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -596,24 +596,23 @@ tests = [
596596
],
597597
JuliaSyntax.parse_try => [
598598
"try \n x \n catch e \n y \n finally \n z end" =>
599-
"(try (block x) e (block y) false (block z))"
599+
"(try (block x) (catch e (block y)) (finally (block z)))"
600600
((v=v"1.8",), "try \n x \n catch e \n y \n else z finally \n w end") =>
601-
"(try (block x) e (block y) (block z) (block w))"
602-
"try x catch end" => "(try (block x) false (block) false false)"
603-
"try x catch ; y end" => "(try (block x) false (block y) false false)"
604-
"try x catch \n y end" => "(try (block x) false (block y) false false)"
605-
"try x catch e y end" => "(try (block x) e (block y) false false)"
606-
"try x catch \$e y end" => "(try (block x) (\$ e) (block y) false false)"
607-
"try x catch e+3 y end" => "(try (block x) (error (call-i e + 3)) (block y) false false)"
608-
"try x finally y end" => "(try (block x) false false false (block y))"
601+
"(try (block x) (catch e (block y)) (else (block z)) (finally (block w)))"
602+
"try x catch end" => "(try (block x) (catch false (block)))"
603+
"try x catch ; y end" => "(try (block x) (catch false (block y)))"
604+
"try x catch \n y end" => "(try (block x) (catch false (block y)))"
605+
"try x catch e y end" => "(try (block x) (catch e (block y)))"
606+
"try x catch \$e y end" => "(try (block x) (catch (\$ e) (block y)))"
607+
"try x catch e+3 y end" => "(try (block x) (catch (error (call-i e + 3)) (block y)))"
608+
"try x finally y end" => "(try (block x) (finally (block y)))"
609609
# v1.8 only
610-
((v=v"1.8",), "try catch ; else end") => "(try (block) false (block) (block) false)"
611-
((v=v"1.8",), "try else x finally y end") => "(try (block) false false (error (block x)) (block y))"
612-
((v=v"1.7",), "try catch ; else end") => "(try (block) false (block) (error (block)) false)"
610+
((v=v"1.8",), "try catch ; else end") => "(try (block) (catch false (block)) (else (block)))"
611+
((v=v"1.8",), "try else x finally y end") => "(try (block) (else (error (block x))) (finally (block y)))"
612+
((v=v"1.7",), "try catch ; else end") => "(try (block) (catch false (block)) (else (error (block))))"
613613
# finally before catch :-(
614-
"try x finally y catch e z end" => "(try_finally_catch (block x) false false false (block y) e (block z))" =>
615-
Expr(:try, Expr(:block, :x), :e, Expr(:block, :z), Expr(:block, :y))
616-
"try x end" => "(try (block x) false false false false (error-t))"
614+
"try x finally y catch e z end" => "(try (block x) (finally (block y)) (catch e (block z)))"
615+
"try x end" => "(try (block x) (error-t))"
617616
],
618617
JuliaSyntax.parse_imports => [
619618
"import A as B: x" => "(import (: (error (as (. A) B)) (. x)))"

‎test/parser_api.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@
8181
parseshow(s;kws...) = sprint(show, MIME("text/x.sexpression"), parse(SyntaxNode, s; kws...))
8282
@test_throws JuliaSyntax.ParseError parseshow("try finally catch ex end")
8383
@test parseshow("try finally catch ex end", ignore_warnings=true) ==
84-
"(try_finally_catch (block) false false false (block) ex (block))"
84+
"(try (block) (finally (block)) (catch ex (block)))"
8585
# ignore_errors
8686
@test_throws JuliaSyntax.ParseError parseshow("[a; b, c]")
8787
@test_throws JuliaSyntax.ParseError parseshow("[a; b, c]", ignore_warnings=true)

0 commit comments

Comments
 (0)