Re: [PATCH] c++: Fix up constexpr handling of break in expansion statements [PR125601]

On 6/5/26 2:35 AM, Jakub Jelinek wrote:
> Hi!
> 
> As the following testcase shows, we mishandle break statements in expansion
> statements during constant evaluation.
> finish_expansion_stmt changes the BREAK_STMT/CONTINUE_STMTs to GOTO_EXPRs
> to corresponding labels and marks those labels with
> LABEL_DECL_BREAK/LABEL_DECL_CONTINUE so that constexpr.cc is happy about
> those.  The continue label (if any is needed) is right after each
> iteration's instantiated body, the break label (if any is needed) is after
> the last body.
> Now, continue seems to work properly, when we encounter it, we set
> *jump_target to it and continues predicate is true on it, but
> cxx_eval_statement_list has
>        /* We've found a continue, so skip everything until we reach
>           the label its jumping to.  */
>        if (continues (jump_target))
>          {
>            if (label_matches (ctx, jump_target, stmt))
>              /* Found it.  */
>              *jump_target = NULL_TREE;
>            else
>              continue;
>          }
> ...
>        if (returns (jump_target)
>            || breaks (jump_target)
>            || throws (jump_target))
>          break;
> and so it properly iterates through statement lists until it finds
> the label decl.
> But unfortunately it doesn't work for break, we set *jump_target on
> the GOTO_EXPR, breaks predicate is true, but then break out of any
> STATEMENT_LISTs and the only way to resume processing of statements
> in that case is when cxx_eval_loop_expr does
>            if (breaks (jump_target))
>              {
>                *jump_target = NULL_TREE;
>                break;
>              }
> or similarly switch handling.  But for expansion stmt there is
> nothing like that in the IL, so either we break some outer loop
> (foo in the testcase) instead, or fail because we think there was no return
> in the function.
> 
> The following patch fixes this by wrapping the series of instantiated
> expansion stmt bodies (for all iterations) in an artificial
> do ... while (0); statement, but does that only if break; was actually
> needed (i.e. when we are emitting a break_label).

I suppose we could change break handling to iterate like continue, but 
this is a clever solution.  OK.

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2026-06-05  Jakub Jelinek  <[email protected]>
> 
> 	PR c++/125601
> 	* pt.cc (finish_expansion_stmt): If break; was seen in any of the
> 	expansion stmt bodies, wrap all the bodies in an artificial
> 	do ... while (0); stmt.
> 
> 	* g++.dg/cpp26/expansion-stmt43.C: New test.
> 
> --- gcc/cp/pt.cc.jj	2026-06-03 11:49:04.897785724 +0200
> +++ gcc/cp/pt.cc	2026-06-04 18:12:39.870465887 +0200
> @@ -33631,6 +33631,7 @@ finish_expansion_stmt (tree expansion_st
>         DECL_NAME (decl) = NULL_TREE;
>       }
>   
> +  tree stmt_list = push_stmt_list ();
>     expansion_stmt_bc bc_data = { NULL_TREE, NULL_TREE, NULL, loc, false };
>   
>     for (unsigned HOST_WIDE_INT i = 0; i < n; ++i)
> @@ -33830,7 +33831,19 @@ finish_expansion_stmt (tree expansion_st
>   	}
>       }
>     if (bc_data.break_label)
> -    add_stmt (build1 (LABEL_EXPR, void_type_node, bc_data.break_label));
> +    {
> +      /* If break; is seen, wrap all the expansion stmt bodies in
> +	 a single artificial do ... while (0); statement, so that
> +	 constant evaluation handles break; correctly.  */
> +      tree do_stmt
> +	= build_stmt (loc, DO_STMT, NULL_TREE, NULL_TREE, NULL_TREE);
> +      DO_COND (do_stmt) = boolean_false_node;
> +      DO_BODY (do_stmt) = pop_stmt_list (stmt_list);
> +      add_stmt (do_stmt);
> +      add_stmt (build1 (LABEL_EXPR, void_type_node, bc_data.break_label));
> +    }
> +  else
> +    add_stmt (pop_stmt_list (stmt_list));
>     if (args == NULL_TREE)
>       {
>         TREE_TYPE (range_decl) = error_mark_node;
> --- gcc/testsuite/g++.dg/cpp26/expansion-stmt43.C.jj	2026-06-04 18:17:40.123509169 +0200
> +++ gcc/testsuite/g++.dg/cpp26/expansion-stmt43.C	2026-06-04 18:17:03.575503867 +0200
> @@ -0,0 +1,64 @@
> +// PR c++/125601
> +// { dg-do run { target c++14 } }
> +// { dg-options "-O2" }
> +
> +constexpr int
> +foo (int x)
> +{
> +  int a = 0, b = 3;
> +  while (b > 0)
> +    {
> +      ++a;
> +      --b;
> +      template for (constexpr int value : { 10, 20, 30 })	// { dg-warning "'template for' only available with" "" { target c++23_down } }
> +        {
> +          a += value;
> +	  if (x == 0)
> +	    break;
> +	  else if (x == 1)
> +	    continue;
> +	  a += 42;
> +        }
> +    }
> +  return a;
> +}
> +
> +constexpr int
> +bar (int x)
> +{
> +  int a = 0;
> +  template for (constexpr int value : { 10, 20, 30 })		// { dg-warning "'template for' only available with" "" { target c++23_down } }
> +    {
> +      a += value;
> +      if (x == 0)
> +	break;
> +      else if (x == 1)
> +	continue;
> +      a += 42;
> +    }
> +  return a;
> +}
> +
> +static_assert (foo (0) == 3 * (1 + 10), "");
> +static_assert (foo (1) == 3 * (1 + 10 + 20 + 30), "");
> +static_assert (foo (2) == 3 * (1 + 10 + 20 + 30 + 3 * 42), "");
> +static_assert (bar (0) == 10, "");
> +static_assert (bar (1) == 10 + 20 + 30, "");
> +static_assert (bar (2) == 10 + 20 + 30 + 3 * 42, "");
> +
> +int
> +main ()
> +{
> +  if (foo (0) != 3 * (1 + 10))
> +    __builtin_abort ();
> +  if (foo (1) != 3 * (1 + 10 + 20 + 30))
> +    __builtin_abort ();
> +  if (foo (2) != 3 * (1 + 10 + 20 + 30 + 3 * 42))
> +    __builtin_abort ();
> +  if (bar (0) != 10)
> +    __builtin_abort ();
> +  if (bar (1) != 10 + 20 + 30)
> +    __builtin_abort ();
> +  if (bar (2) != 10 + 20 + 30 + 3 * 42)
> +    __builtin_abort ();
> +}
> 
> 	Jakub
>