Re: [PATCH 1/2] pkl: Fix leaking of exceptions inside try blocks

[Jose E. Marchesi]

Hi Mohammad.

> Without this patch the code generator leaves the exception(s) in
> exception stack for the following code:
>
>   try return something;
>   catch {}
>
> 2021-12-26  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>       * libpoke/pkl-ast.h (struct pkl_ast_return_stmt): Add new field
>       `npopes` (number of `pope`s).
>       (PKL_AST_RETURN_STMT_NPOPES): New macro.
>       * libpoke/pkl-ast.c (pkl_ast_finish_returns_1): Add new arg `npopes`
>       to count the `pope`s.
>       (pkl_ast_finish_returns): Likewise.
>       * libpoke/pkl-gen.c (pkl_gen_ps_return_stmt): Insert necessary `pope`
>       instruction(s).

Very nice catch (:P) and OK for master.
Thanks!

> ---
>  ChangeLog         | 11 +++++++++++
>  libpoke/pkl-ast.c | 24 +++++++++++++++---------
>  libpoke/pkl-ast.h |  5 +++++
>  libpoke/pkl-gen.c |  4 ++++
>  4 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index ad1af792..0432f5d4 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,14 @@
> +2021-12-26  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * libpoke/pkl-ast.h (struct pkl_ast_return_stmt): Add new field
> +     `npopes` (number of `pope`s).
> +     (PKL_AST_RETURN_STMT_NPOPES): New macro.
> +     * libpoke/pkl-ast.c (pkl_ast_finish_returns_1): Add new arg `npopes`
> +     to count the `pope`s.
> +     (pkl_ast_finish_returns): Likewise.
> +     * libpoke/pkl-gen.c (pkl_gen_ps_return_stmt): Insert necessary `pope`
> +     instruction(s).
> +
>  2021-12-24  Jose E. Marchesi  <jemarch@gnu.org>
>  
>       * libpoke/pkl-gen-builtins.pks: New file.
> diff --git a/libpoke/pkl-ast.c b/libpoke/pkl-ast.c
> index d0573a87..fdfccf99 100644
> --- a/libpoke/pkl-ast.c
> +++ b/libpoke/pkl-ast.c
> @@ -2515,7 +2515,7 @@ pkl_ast_type_incr_step (pkl_ast ast, pkl_ast_node type)
>  
>  static void
>  pkl_ast_finish_returns_1 (pkl_ast_node function, pkl_ast_node stmt,
> -                          int *nframes, int *ndrops)
> +                          int *nframes, int *ndrops, int *npopes)
>  {
>    /* STMT can be a statement or a declaration.  */
>  
> @@ -2525,6 +2525,7 @@ pkl_ast_finish_returns_1 (pkl_ast_node function, 
> pkl_ast_node stmt,
>        PKL_AST_RETURN_STMT_FUNCTION (stmt) = function; /* Note no ASTREF.  */
>        PKL_AST_RETURN_STMT_NFRAMES (stmt) = *nframes;
>        PKL_AST_RETURN_STMT_NDROPS (stmt) = *ndrops;
> +      PKL_AST_RETURN_STMT_NPOPES (stmt) = *npopes;
>        break;
>      case PKL_AST_COMP_STMT:
>        {
> @@ -2533,7 +2534,7 @@ pkl_ast_finish_returns_1 (pkl_ast_node function, 
> pkl_ast_node stmt,
>          *nframes += 1;
>          for (t = PKL_AST_COMP_STMT_STMTS (stmt); t;
>               t = PKL_AST_CHAIN (t))
> -          pkl_ast_finish_returns_1 (function, t, nframes, ndrops);
> +          pkl_ast_finish_returns_1 (function, t, nframes, ndrops, npopes);
>  
>          /* Pop the frame of the compound itself.  */
>          *nframes -= 1;
> @@ -2542,11 +2543,11 @@ pkl_ast_finish_returns_1 (pkl_ast_node function, 
> pkl_ast_node stmt,
>      case PKL_AST_IF_STMT:
>        pkl_ast_finish_returns_1 (function,
>                                  PKL_AST_IF_STMT_THEN_STMT (stmt),
> -                                nframes, ndrops);
> +                                nframes, ndrops, npopes);
>        if (PKL_AST_IF_STMT_ELSE_STMT (stmt))
>          pkl_ast_finish_returns_1 (function,
>                                    PKL_AST_IF_STMT_ELSE_STMT (stmt),
> -                                  nframes, ndrops);
> +                                  nframes, ndrops, npopes);
>        break;
>      case PKL_AST_LOOP_STMT:
>        {
> @@ -2554,23 +2555,27 @@ pkl_ast_finish_returns_1 (pkl_ast_node function, 
> pkl_ast_node stmt,
>            *ndrops += 3;
>          pkl_ast_finish_returns_1 (function,
>                                    PKL_AST_LOOP_STMT_BODY (stmt),
> -                                  nframes, ndrops);
> +                                  nframes, ndrops, npopes);
>          if (PKL_AST_LOOP_STMT_ITERATOR (stmt))
>            *ndrops -= 3;
>          break;
>        }
>      case PKL_AST_TRY_CATCH_STMT:
> +      *npopes += 1;
>        pkl_ast_finish_returns_1 (function,
>                                  PKL_AST_TRY_CATCH_STMT_CODE (stmt),
> -                                nframes, ndrops);
> +                                nframes, ndrops, npopes);
> +      *npopes -= 1;
>        pkl_ast_finish_returns_1 (function,
>                                  PKL_AST_TRY_CATCH_STMT_HANDLER (stmt),
> -                                nframes, ndrops);
> +                                nframes, ndrops, npopes);
>        break;
>      case PKL_AST_TRY_UNTIL_STMT:
> +      *npopes += 1;
>        pkl_ast_finish_returns_1 (function,
>                                  PKL_AST_TRY_UNTIL_STMT_CODE (stmt),
> -                                nframes, ndrops);
> +                                nframes, ndrops, npopes);
> +      *npopes -= 1;
>        break;
>      case PKL_AST_DECL:
>      case PKL_AST_EXP_STMT:
> @@ -2592,8 +2597,9 @@ pkl_ast_finish_returns (pkl_ast_node function)
>  {
>    int nframes = 0;
>    int ndrops = 0;
> +  int npopes = 0;
>    pkl_ast_finish_returns_1 (function, PKL_AST_FUNC_BODY (function),
> -                            &nframes, &ndrops);
> +                            &nframes, &ndrops, &npopes);
>  }
>  
>  int
> diff --git a/libpoke/pkl-ast.h b/libpoke/pkl-ast.h
> index d5f42f65..e7e32247 100644
> --- a/libpoke/pkl-ast.h
> +++ b/libpoke/pkl-ast.h
> @@ -1707,12 +1707,16 @@ pkl_ast_node pkl_ast_make_loop_stmt_iterator (pkl_ast 
> ast,
>     NDROPS is the number of stack elements to drop before returning
>     from the function.
>  
> +   NPOPES is the number of exception stack elements to drop before
> +   returning from the function.
> +
>     FUNCTION is the PKL_AST_FUNCTION containing this return
>     statement. */
>  
>  #define PKL_AST_RETURN_STMT_EXP(AST) ((AST)->return_stmt.exp)
>  #define PKL_AST_RETURN_STMT_NFRAMES(AST) ((AST)->return_stmt.nframes)
>  #define PKL_AST_RETURN_STMT_NDROPS(AST) ((AST)->return_stmt.ndrops)
> +#define PKL_AST_RETURN_STMT_NPOPES(AST) ((AST)->return_stmt.npopes)
>  #define PKL_AST_RETURN_STMT_FUNCTION(AST) ((AST)->return_stmt.function)
>  
>  struct pkl_ast_return_stmt
> @@ -1723,6 +1727,7 @@ struct pkl_ast_return_stmt
>    union pkl_ast_node *function;
>    int nframes;
>    int ndrops;
> +  int npopes;
>  };
>  
>  pkl_ast_node pkl_ast_make_return_stmt (pkl_ast ast, pkl_ast_node exp);
> diff --git a/libpoke/pkl-gen.c b/libpoke/pkl-gen.c
> index bd5c1cf5..fd43b0ad 100644
> --- a/libpoke/pkl-gen.c
> +++ b/libpoke/pkl-gen.c
> @@ -1259,6 +1259,10 @@ PKL_PHASE_BEGIN_HANDLER (pkl_gen_ps_return_stmt)
>    pkl_ast_node return_stmt = PKL_PASS_NODE;
>    pkl_ast_node function = PKL_AST_RETURN_STMT_FUNCTION (return_stmt);
>    pkl_ast_node function_type = PKL_AST_TYPE (function);
> +  int i;
> +
> +  for (i = 0; i < PKL_AST_RETURN_STMT_NPOPES (PKL_PASS_NODE); ++i)
> +    pkl_asm_insn (PKL_GEN_ASM, PKL_INSN_POPE);
>  
>    /* In a void function, return PVM_NULL in the stack.  */
>    if (PKL_AST_TYPE_CODE (PKL_AST_TYPE_F_RTYPE (function_type))