On Mon, Oct 27, 2025 at 4:22 PM Anna (navi) Figueiredo Gomes
<[email protected]> wrote:
>
> represent the start and end of scope which a local jump is not allowed
> to cross as a jump barrier, moving this logic out into a function allows
> for defer statements to use the same codepath
>
> gcc/c/ChangeLog:
>
> * c-decl.cc (struct c_jump_barrier): New structure.
> (struct c_spot_bindings): Adjust.
> (set_spot_bindings): Likewise.
> (c_binding_adjust_jump_barrier): New function.
> (c_bindings_start_stmt_expr): Adjust.
> (c_bindings_end_stmt_expr): Likewise.
> (lookup_label_for_goto): Likewise.
> (check_earlier_gotos): Likewise.
> (c_release_switch_bindings): Likewise.
> (c_check_switch_jump_warnings): Likewise.
>
> Signed-off-by: Anna (navi) Figueiredo Gomes <[email protected]>
> ---
> gcc/c/c-decl.cc | 72 +++++++++++++++++++++++++------------------------
> 1 file changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 7850365f35c..56ca3078226 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -379,6 +379,16 @@ c_tree_size (enum tree_code code)
> scopes are popped, we update these structures and gather the decls
> that matter at that time. */
>
> +struct GTY(()) c_jump_barrier {
> + /* The number of barriers that have started since this label or goto
> + statement was defined. This is zero if we are at the same barrier.
> + It is positive if we are in a barrier that started since this spot.
> + If it goes negative, it is reset to zero and .left is set to true. */
> + int count;
> + /* Whether we started in a barrier but are no longer in it. */
> + bool left;
> +};
> +
> struct GTY(()) c_spot_bindings {
> /* The currently open scope which holds bindings defined when the
> label was defined or the goto statement was found. */
> @@ -387,16 +397,7 @@ struct GTY(()) c_spot_bindings {
> of the label or goto. This lets us look at older or newer
> bindings in the scope, as appropriate. */
> struct c_binding *bindings_in_scope;
> - /* The number of statement expressions that have started since this
> - label or goto statement was defined. This is zero if we are at
> - the same statement expression level. It is positive if we are in
> - a statement expression started since this spot. It is negative
> - if this spot was in a statement expression and we have left
> - it. */
> - int stmt_exprs;
> - /* Whether we started in a statement expression but are no longer in
> - it. This is set to true if stmt_exprs ever goes negative. */
> - bool left_stmt_expr;
> + struct c_jump_barrier stmt_exprs;
Remove the struct tag marking, this is C++ code. I just noticed the
struct tag marking is still in the file. I think most other parts of
GCC had it removed which is why I mentioned it here. I think keeping
it here is fine as that is how the rest of the file looks.
> };
>
> /* This structure is used to keep track of bindings seen when a goto
> @@ -983,8 +984,8 @@ set_spot_bindings (struct c_spot_bindings *p, bool
> defining)
> p->scope = NULL;
> p->bindings_in_scope = NULL;
> }
> - p->stmt_exprs = 0;
> - p->left_stmt_expr = false;
> + p->stmt_exprs.count = 0;
> + p->stmt_exprs.left = false;
> }
>
> /* Update spot bindings P as we pop out of SCOPE. Return true if we
> @@ -1603,6 +1604,18 @@ finish_underspecified_init (tree name, unsigned int
> prev_state)
>
> /* Adjust the bindings for the start of a statement expression. */
>
> +static void
> +c_binding_adjust_jump_barrier (struct c_spot_bindings *binding, bool start)
Also the comment above was originally for c_bindings_start_stmt_expr
but now there is no comment above c_bindings_start_stmt_expr.
Comments before the function should describe what the function does
and what the arguments mean.
> +{
> + struct c_jump_barrier *barrier = &binding->stmt_exprs;
> + barrier->count += start ? 1 : -1;
I find the above style odd and hard to understand; It is hard to
figure count is incrementing by 1 when start is true; otherwise
decrementing by one.
Maybe this is better:
if (start)
barrier->count++;
else
barrier->count--;
Thanks,
Andrew
> + if (barrier->count < 0)
> + {
> + barrier->count = 0;
> + barrier->left = true;
> + }
> +}
> +
> void
> c_bindings_start_stmt_expr (struct c_spot_bindings* switch_bindings)
> {
> @@ -1624,14 +1637,15 @@ c_bindings_start_stmt_expr (struct c_spot_bindings*
> switch_bindings)
> if (TREE_CODE (b->decl) != LABEL_DECL)
> continue;
> label_vars = b->u.label;
> - ++label_vars->label_bindings.stmt_exprs;
> +
> + c_binding_adjust_jump_barrier (&label_vars->label_bindings, true);
> FOR_EACH_VEC_SAFE_ELT (label_vars->gotos, ix, g)
> - ++g->goto_bindings.stmt_exprs;
> + c_binding_adjust_jump_barrier (&g->goto_bindings, true);
> }
> }
>
> if (switch_bindings != NULL)
> - ++switch_bindings->stmt_exprs;
> + c_binding_adjust_jump_barrier (switch_bindings, true);
> }
>
> /* Adjust the bindings for the end of a statement expression. */
> @@ -1657,28 +1671,16 @@ c_bindings_end_stmt_expr (struct c_spot_bindings
> *switch_bindings)
> if (TREE_CODE (b->decl) != LABEL_DECL)
> continue;
> label_vars = b->u.label;
> - --label_vars->label_bindings.stmt_exprs;
> - if (label_vars->label_bindings.stmt_exprs < 0)
> - {
> - label_vars->label_bindings.left_stmt_expr = true;
> - label_vars->label_bindings.stmt_exprs = 0;
> - }
> + c_binding_adjust_jump_barrier (&label_vars->label_bindings, false);
> FOR_EACH_VEC_SAFE_ELT (label_vars->gotos, ix, g)
> - {
> - --g->goto_bindings.stmt_exprs;
> - if (g->goto_bindings.stmt_exprs < 0)
> - {
> - g->goto_bindings.left_stmt_expr = true;
> - g->goto_bindings.stmt_exprs = 0;
> - }
> - }
> + c_binding_adjust_jump_barrier (&g->goto_bindings, false);
> }
> }
>
> if (switch_bindings != NULL)
> {
> - --switch_bindings->stmt_exprs;
> - gcc_assert (switch_bindings->stmt_exprs >= 0);
> + c_binding_adjust_jump_barrier (switch_bindings, defer, false);
> + gcc_assert (switch_bindings->stmt_exprs.count >= 0);
> }
> }
>
> @@ -4149,7 +4151,7 @@ lookup_label_for_goto (location_t loc, tree name)
> FOR_EACH_VEC_SAFE_ELT (label_vars->decls_in_scope, ix, decl)
> warn_about_goto (loc, label, decl);
>
> - if (label_vars->label_bindings.left_stmt_expr)
> + if (label_vars->label_bindings.stmt_exprs.left)
> {
> auto_diagnostic_group d;
> error_at (loc, "jump into statement expression");
> @@ -4240,7 +4242,7 @@ check_earlier_gotos (tree label, struct c_label_vars*
> label_vars)
> }
> }
>
> - if (g->goto_bindings.stmt_exprs > 0)
> + if (g->goto_bindings.stmt_exprs.count > 0)
> {
> auto_diagnostic_group d;
> error_at (g->loc, "jump into statement expression");
> @@ -4331,7 +4333,7 @@ c_get_switch_bindings (void)
> void
> c_release_switch_bindings (struct c_spot_bindings *bindings)
> {
> - gcc_assert (bindings->stmt_exprs == 0 && !bindings->left_stmt_expr);
> + gcc_assert (bindings->stmt_exprs.count == 0 && !bindings->stmt_exprs.left);
> XDELETE (bindings);
> }
>
> @@ -4395,7 +4397,7 @@ c_check_switch_jump_warnings (struct c_spot_bindings
> *switch_bindings,
> }
> }
>
> - if (switch_bindings->stmt_exprs > 0)
> + if (switch_bindings->stmt_exprs.count > 0)
> {
> saw_error = true;
> auto_diagnostic_group d;
> --
> 2.51.0
>
