Re: [PATCH v2 1/2] pkl: change AST structure for try-catch stmts

[Jose E. Marchesi]

Hi Mohammad.
Thanks for the patch.

> +/* PKL_AST_TRY_STMT_HANDLER nodes represent the handler part of the
> +   try-catch statements.  */

Please document CODE in the comment.
Something like: "CODE is a PKL_AST_COMP_STMT node, or NULL if the node
corresponds to a try-until statement."

> +#define PKL_AST_TRY_STMT_HANDLER_CODE(AST) ((AST)->try_stmt_handler.code)
> +
> +struct pkl_ast_try_stmt_handler
> +{
> +  struct pkl_ast_common common;
> +
> +  union pkl_ast_node *code;
> +};
> +
> +pkl_ast_node pkl_ast_make_try_stmt_handler (pkl_ast ast, pkl_ast_node code);
> +
>  /* PKL_AST_PRINT_STMT nodes represent `print' and `printf' statements.
>  
>     STR_EXP, if not NULL, is an expression node of type string.  In
> @@ -1970,6 +1985,7 @@ union pkl_ast_node
>    struct pkl_ast_exp_stmt exp_stmt;
>    struct pkl_ast_try_stmt try_stmt;
>    struct pkl_ast_try_stmt_body try_stmt_body;
> +  struct pkl_ast_try_stmt_body try_stmt_handler;
>    struct pkl_ast_break_stmt break_stmt;
>    struct pkl_ast_continue_stmt continue_stmt;
>    struct pkl_ast_raise_stmt raise_stmt;
> diff --git a/libpoke/pkl-gen.c b/libpoke/pkl-gen.c
> index 6ecd322a..e5f1abf3 100644
> --- a/libpoke/pkl-gen.c
> +++ b/libpoke/pkl-gen.c
> @@ -1762,6 +1762,17 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_try_stmt_body)
>  }
>  PKL_PHASE_END_HANDLER
>  
> +/*
> + * | CODE
> + * TRY_STMT_HANDLER
> + */
> +
> +PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_try_stmt_handler)
> +{
> +  /* Nothing to do here.  */
> +}
> +PKL_PHASE_END_HANDLER
> +
>  /*
>   * | BODY
>   * | [HANDLER]
> @@ -4639,6 +4650,7 @@ struct pkl_phase pkl_phase_gen =
>     PKL_PHASE_PS_HANDLER (PKL_AST_RAISE_STMT, pkl_gen_ps_raise_stmt),
>     PKL_PHASE_PR_HANDLER (PKL_AST_TRY_STMT, pkl_gen_pr_try_stmt),
>     PKL_PHASE_PS_HANDLER (PKL_AST_TRY_STMT_BODY, pkl_gen_ps_try_stmt_body),
> +   PKL_PHASE_PS_HANDLER (PKL_AST_TRY_STMT_HANDLER, 
> pkl_gen_ps_try_stmt_handler),
>     PKL_PHASE_PS_HANDLER (PKL_AST_FUNCALL_ARG, pkl_gen_ps_funcall_arg),
>     PKL_PHASE_PR_HANDLER (PKL_AST_FUNCALL, pkl_gen_pr_funcall),
>     PKL_PHASE_PR_HANDLER (PKL_AST_FUNC, pkl_gen_pr_func),
> diff --git a/libpoke/pkl-pass.c b/libpoke/pkl-pass.c
> index a6fabd65..ed7daf8c 100644
> --- a/libpoke/pkl-pass.c
> +++ b/libpoke/pkl-pass.c
> @@ -548,6 +548,9 @@ pkl_do_pass_1 (pkl_compiler compiler,
>      case PKL_AST_TRY_STMT_BODY:
>        PKL_PASS (PKL_AST_TRY_STMT_BODY_CODE (node));
>        break;
> +    case PKL_AST_TRY_STMT_HANDLER:
> +      PKL_PASS (PKL_AST_TRY_STMT_HANDLER_CODE (node));
> +      break;
>      case PKL_AST_TRY_STMT:
>        PKL_PASS (PKL_AST_TRY_STMT_BODY (node));
>        if (PKL_AST_TRY_STMT_HANDLER (node))
> diff --git a/libpoke/pkl-tab.y b/libpoke/pkl-tab.y
> index 334c3ec0..c12bb0f6 100644
> --- a/libpoke/pkl-tab.y
> +++ b/libpoke/pkl-tab.y
> @@ -2507,34 +2507,40 @@ stmt:
>                  }
>          | TRY stmt CATCH comp_stmt
>                  {
> -                  pkl_ast_node body = pkl_ast_make_try_stmt_body 
> (pkl_parser->ast,
> -                                                                  $2);
> +                  pkl_ast_node body
> +                    = pkl_ast_make_try_stmt_body (pkl_parser->ast, $2);
> +                  pkl_ast_node handler
> +                    = pkl_ast_make_try_stmt_handler (pkl_parser->ast, $4);
>  
>                    $$ = pkl_ast_make_try_stmt (pkl_parser->ast,
>                                                PKL_AST_TRY_STMT_KIND_CATCH,
> -                                              body, $4, NULL, NULL);
> +                                              body, handler, NULL, NULL);
>                    PKL_AST_LOC (body) = @2;
>                    PKL_AST_LOC ($$) = @$;

You forgot to set the location of the `handler' node.

PKL_AST_LOC (handler) = @4;

>                  }
>          | TRY stmt CATCH IF expression comp_stmt
>                  {
> -                  pkl_ast_node body = pkl_ast_make_try_stmt_body 
> (pkl_parser->ast,
> -                                                                  $2);
> +                  pkl_ast_node body
> +                    = pkl_ast_make_try_stmt_body (pkl_parser->ast, $2);
> +                  pkl_ast_node handler
> +                    = pkl_ast_make_try_stmt_handler (pkl_parser->ast, $6);
>  
>                    $$ = pkl_ast_make_try_stmt (pkl_parser->ast,
>                                                PKL_AST_TRY_STMT_KIND_CATCH,
> -                                              body, $6, NULL, $5);
> +                                              body, handler, NULL, $5);
>                    PKL_AST_LOC (body) = @2;
>                    PKL_AST_LOC ($$) = @$;
>                  }

Likewise.

>          | TRY stmt CATCH  '(' pushlevel function_arg ')' comp_stmt
>                  {
> -                  pkl_ast_node body = pkl_ast_make_try_stmt_body 
> (pkl_parser->ast,
> -                                                                  $2);
> +                  pkl_ast_node body
> +                    = pkl_ast_make_try_stmt_body (pkl_parser->ast, $2);
> +                  pkl_ast_node handler
> +                    = pkl_ast_make_try_stmt_handler (pkl_parser->ast, $8);
>  
>                    $$ = pkl_ast_make_try_stmt (pkl_parser->ast,
>                                                PKL_AST_TRY_STMT_KIND_CATCH,
> -                                              body, $8, $6, NULL);
> +                                              body, handler, $6, NULL);
>                    PKL_AST_LOC (body) = @2;
>                    PKL_AST_LOC ($$) = @$;

Likewise.

Other than the above the patch is OK for master.
Thanks!