Re: [PATCH v2 2/2] pkl: trans: get rid of pkl_ast_finish_breaks

[Jose E. Marchesi]

Hi.

> diff --git a/libpoke/pkl-trans.c b/libpoke/pkl-trans.c
> index dcb3d613..07204ab2 100644
> --- a/libpoke/pkl-trans.c
> +++ b/libpoke/pkl-trans.c
> @@ -107,6 +107,30 @@ pkl_trans_in_functions (struct pkl_trans_function_ctx 
> functions[],
>      }                                                           \
>    while (0)
>  

Please add a comment at this point:

/* Handling of the stack of escapable constructions.  */

> +#define PKL_TRANS_ESCAPABLE                                     \
> +  (PKL_TRANS_PAYLOAD->next_escapable == 0                       \
> +   ? NULL                                                       \
> +   : &PKL_TRANS_PAYLOAD->escapables[PKL_TRANS_PAYLOAD->next_escapable - 1])
> +
> +#define PKL_TRANS_PUSH_ESCAPABLE(b)                             \
> +  do                                                            \
> +    {                                                           \
> +      assert (PKL_TRANS_PAYLOAD->next_escapable < 
> PKL_TRANS_MAX_COMP_STMT_NEST);\
> +      
> PKL_TRANS_PAYLOAD->escapables[PKL_TRANS_PAYLOAD->next_escapable].nframes  \
> +        = 0;                                                    \
> +      
> PKL_TRANS_PAYLOAD->escapables[PKL_TRANS_PAYLOAD->next_escapable++].node \
> +        = (b);                                                  \
> +    }                                                           \
> +  while (0)
> +
> +#define PKL_TRANS_POP_ESCAPABLE                                 \
> +  do                                                            \
> +    {                                                           \
> +      assert (PKL_TRANS_PAYLOAD->next_escapable > 0);           \
> +      PKL_TRANS_PAYLOAD->next_escapable--;                      \
> +    }                                                           \
> +  while (0)
> +
>  /* The following handler is used in all trans phases and initializes
>     the phase payload.  */
>  
> @@ -1094,6 +1118,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_trans1_pr_comp_stmt)
>            PKL_TRANS_FUNCTION->npopes++;
>          }
>      }
> +
> +  if (PKL_TRANS_ESCAPABLE)
> +    PKL_TRANS_ESCAPABLE->nframes++;
>  }
>  PKL_PHASE_END_HANDLER

Please update the comment for the handler explaining that the number of
lexical frames of the containing escapable construction (if any) is
increased.

> @@ -1123,6 +1150,8 @@ PKL_PHASE_BEGIN_HANDLER (pkl_trans1_pr_loop_stmt)
>  
>    if (PKL_TRANS_FUNCTION && PKL_AST_LOOP_STMT_ITERATOR (stmt))
>      PKL_TRANS_FUNCTION->ndrops += 3;
> +
> +  PKL_TRANS_PUSH_ESCAPABLE (stmt);
>  }
>  PKL_PHASE_END_HANDLER
>  
> @@ -1139,6 +1168,8 @@ PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_loop_stmt)
>    if (PKL_TRANS_FUNCTION
>        && (PKL_TRANS_FUNCTION && PKL_AST_LOOP_STMT_ITERATOR (stmt)))
>      PKL_TRANS_FUNCTION->ndrops -= 3;
> +
> +  PKL_TRANS_POP_ESCAPABLE;
>  }
>  PKL_PHASE_END_HANDLER

Likewise.

> @@ -1182,6 +1213,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_comp_stmt)
>            PKL_TRANS_FUNCTION->npopes--;
>          }
>      }
> +
> +  if (PKL_TRANS_ESCAPABLE)
> +    PKL_TRANS_ESCAPABLE->nframes--;
>  }
>  PKL_PHASE_END_HANDLER

Likewise.

> @@ -1237,6 +1271,9 @@ PKL_PHASE_BEGIN_HANDLER (pkl_trans1_pr_try_stmt_body)
>  {
>    if (PKL_TRANS_FUNCTION)
>      PKL_TRANS_FUNCTION->npopes++;
> +
> +  if (PKL_AST_TRY_STMT_KIND (PKL_PASS_PARENT) == PKL_AST_TRY_STMT_KIND_UNTIL)
> +    PKL_TRANS_PUSH_ESCAPABLE (PKL_PASS_PARENT);
>  }
>  PKL_PHASE_END_HANDLER


Likewise.

> @@ -1248,6 +1285,61 @@ PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_try_stmt_body)
>  {
>    if (PKL_TRANS_FUNCTION)
>      PKL_TRANS_FUNCTION->npopes--;
> +
> +  if (PKL_AST_TRY_STMT_KIND (PKL_PASS_PARENT) == PKL_AST_TRY_STMT_KIND_UNTIL)
> +    PKL_TRANS_POP_ESCAPABLE;
> +}
> +PKL_PHASE_END_HANDLER

Likewise.

> +PKL_PHASE_BEGIN_HANDLER (pkl_trans1_pr_try_stmt_handler)
> +{
> +  if (PKL_TRANS_ESCAPABLE)
> +    PKL_TRANS_ESCAPABLE->nframes++;
> +}
> +PKL_PHASE_END_HANDLER

Likewise.

> +PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_try_stmt_handler)
> +{
> +  if (PKL_TRANS_ESCAPABLE)
> +    PKL_TRANS_ESCAPABLE->nframes--;
> +}
> +PKL_PHASE_END_HANDLER

Likewise.

> +PKL_PHASE_BEGIN_HANDLER (pkl_trans1_pr_try_stmt)
> +{
> +  if (PKL_AST_TRY_STMT_KIND (PKL_PASS_NODE) == PKL_AST_TRY_STMT_KIND_UNTIL)
> +    PKL_TRANS_PUSH_ESCAPABLE (PKL_PASS_NODE);
> +}
> +PKL_PHASE_END_HANDLER

Likewise.

> +PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_try_stmt)
> +{
> +  if (PKL_AST_TRY_STMT_KIND (PKL_PASS_NODE) == PKL_AST_TRY_STMT_KIND_UNTIL)
> +    PKL_TRANS_POP_ESCAPABLE;
> +}
> +PKL_PHASE_END_HANDLER

Likewise.

> +PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_break_stmt)
> +{
> +  if (PKL_TRANS_ESCAPABLE)
> +    {
> +      PKL_AST_BREAK_STMT_ENTITY (PKL_PASS_NODE)
> +        = PKL_TRANS_ESCAPABLE->node;
> +      PKL_AST_BREAK_STMT_NFRAMES (PKL_PASS_NODE)
> +        = PKL_TRANS_ESCAPABLE->nframes;
> +    }
> +}
> +PKL_PHASE_END_HANDLER

Likewise.

> +PKL_PHASE_BEGIN_HANDLER (pkl_trans1_ps_continue_stmt)
> +{
> +  if (PKL_TRANS_ESCAPABLE)
> +    {
> +      PKL_AST_CONTINUE_STMT_ENTITY (PKL_PASS_NODE)
> +        = PKL_TRANS_ESCAPABLE->node;
> +      PKL_AST_CONTINUE_STMT_NFRAMES (PKL_PASS_NODE)
> +        = PKL_TRANS_ESCAPABLE->nframes;
> +    }
>  }
>  PKL_PHASE_END_HANDLER

Likewise.

> @@ -1271,7 +1363,13 @@ struct pkl_phase pkl_phase_trans1 =
>     PKL_PHASE_PS_HANDLER (PKL_AST_LOOP_STMT_ITERATOR, 
> pkl_trans1_ps_loop_stmt_iterator),
>     PKL_PHASE_PR_HANDLER (PKL_AST_LOOP_STMT, pkl_trans1_pr_loop_stmt),
>     PKL_PHASE_PS_HANDLER (PKL_AST_LOOP_STMT, pkl_trans1_ps_loop_stmt),
> +   PKL_PHASE_PS_HANDLER (PKL_AST_BREAK_STMT, pkl_trans1_ps_break_stmt),
> +   PKL_PHASE_PS_HANDLER (PKL_AST_CONTINUE_STMT, pkl_trans1_ps_continue_stmt),
> +   PKL_PHASE_PR_HANDLER (PKL_AST_TRY_STMT_BODY, pkl_trans1_pr_try_stmt),
> +   PKL_PHASE_PS_HANDLER (PKL_AST_TRY_STMT_BODY, pkl_trans1_ps_try_stmt),
>     PKL_PHASE_PR_HANDLER (PKL_AST_TRY_STMT_BODY, pkl_trans1_pr_try_stmt_body),
> +   PKL_PHASE_PS_HANDLER (PKL_AST_TRY_STMT_HANDLER, 
> pkl_trans1_ps_try_stmt_handler),
> +   PKL_PHASE_PR_HANDLER (PKL_AST_TRY_STMT_HANDLER, 
> pkl_trans1_pr_try_stmt_handler),
>     PKL_PHASE_PS_HANDLER (PKL_AST_TRY_STMT_BODY, pkl_trans1_ps_try_stmt_body),
>     PKL_PHASE_PR_HANDLER (PKL_AST_STRUCT_TYPE_FIELD, 
> pkl_trans1_pr_struct_type_field),
>     PKL_PHASE_PS_HANDLER (PKL_AST_STRUCT_TYPE_FIELD, 
> pkl_trans1_ps_struct_type_field),
> diff --git a/libpoke/pkl-trans.h b/libpoke/pkl-trans.h
> index 8d3b1f6f..e5ca68d4 100644
> --- a/libpoke/pkl-trans.h
> +++ b/libpoke/pkl-trans.h
> @@ -53,6 +53,23 @@ struct pkl_trans_function_ctx
>    struct pkl_trans_function_ctx *next;
>  };
>  
> +/* The trans phases keep a stack of escapable entities (loops and
> +   try-until statements) and the corresponding `break' and `continue'
> +   keywords, while they operate on the AST.
> +
> +   The following struct implements each entry on the escapable/continuable
> +   entities stack.

You mean breakable/continuable there.

> +   NODE is the escapable entity (loop or try-until statement).
> +
> +   NFRAMES is the number of frames in the body of NODE.  */
> +
> +struct pkl_trans_escapable_ctx
> +{
> +  pkl_ast_node node;
> +  int nframes;
> +};
> +
>  /* The following struct defines the payload of the trans phases.
>  
>     ERRORS is the number of errors detected while running the phase.
> @@ -69,10 +86,16 @@ struct pkl_trans_function_ctx
>     when mapping and writing integral types.
>  
>     CUR_ENDIAN is the index to ENDIAN and marks the top of the stack of
> -   endianness.  Initially PKL_AST_ENDIAN_DFL.  */
> +   endianness.  Initially PKL_AST_ENDIAN_DFL.
> +
> +   ESCAPABLES is a stack of escapables.
> +
> +   NEXT_ESCAPABLE - 1 is the index for the enclosing escapable
> +   entity.  */
>  
>  #define PKL_TRANS_MAX_FUNCTION_NEST 32
>  #define PKL_TRANS_MAX_ENDIAN 25
> +#define PKL_TRANS_MAX_COMP_STMT_NEST 32
>  
>  struct pkl_trans_payload
>  {
> @@ -82,6 +105,8 @@ struct pkl_trans_payload
>    int next_function;
>    enum pkl_ast_endian endian[PKL_TRANS_MAX_ENDIAN];
>    int cur_endian;
> +  struct pkl_trans_escapable_ctx escapables[PKL_TRANS_MAX_COMP_STMT_NEST];
> +  int next_escapable;
>  };
>  
>  typedef struct pkl_trans_payload *pkl_trans_payload;