aboutsummaryrefslogtreecommitdiff
path: root/gcc/c/c-parser.cc
diff options
context:
space:
mode:
authorJakub Jelinek <jakub@redhat.com>2025-02-07 17:07:23 +0100
committerJakub Jelinek <jakub@gcc.gnu.org>2025-02-07 17:07:23 +0100
commit35d40b56eb6e7ac63c790a799d3b367742d58a5e (patch)
tree65dca5061054d0cabe6f11a1de79afa042964257 /gcc/c/c-parser.cc
parente22962538f64bb6e5ac87977ec8a5d86f4ef21cb (diff)
downloadgcc-35d40b56eb6e7ac63c790a799d3b367742d58a5e.zip
gcc-35d40b56eb6e7ac63c790a799d3b367742d58a5e.tar.gz
gcc-35d40b56eb6e7ac63c790a799d3b367742d58a5e.tar.bz2
c++: Fix up handling of for/while loops with declarations in condition [PR86769]
As the following testcase show (note, just for-{3,4,6,7,8}.C, constexpr-86769.C and stmtexpr27.C FAIL without the patch, the rest is just that I couldn't find coverage for some details and so added tests we don't regress or for5.C is from Marek's attempt in the PR), we weren't correctly handling for/while loops with declarations as conditions. The C++ FE has the simplify_loop_decl_cond function which transforms such loops as mentioned in the comment: while (A x = 42) { } for (; A x = 42;) { } becomes while (true) { A x = 42; if (!x) break; } for (;;) { A x = 42; if (!x) break; } For for loops this is not enough, as the x declaration should be still in scope when expr (if any) is executed, and injecting the expr expression into the body in the FE needs to have the continue label in between, something normally added by the c-family genericization. One of my thoughts was to just add there an artificial label plus the expr expression in the FE and tell c-family about that label, so that it doesn't create it but uses what has been emitted. Unfortunately break/continue are resolved to labels only at c-family genericization time and by moving the condition (and its preparation statements such as the DECL_EXPR) into the body (and perhaps by also moving there the (increment) expr as well) we resolve incorrectly any break/continue statement appearing in cond (or newly perhaps also expr) expression(s). While in standard C++ one can't have something like that there, with statement expressions they are possible there, and we actually have testsuite coverage that when they appear outside of the body of the loop they bind to an outer loop rather than the inner one. When the FE moves everything into the body, c-family can't distinguish any more between the user body vs. the condition/preparation statements vs. expr expression. So, the following patch instead keeps them separate and does the merging only at the c-family loop genericization time. For that the patch introduces two new operands of FOR_STMT and WHILE_STMT, *_COND_PREP which is forced to be a BIND_EXPR which contains the preparation statements like DECL_EXPR, and the initialization of that variable, so basically what {FOR,WHILE}_BODY has when we encounter the function dealing with this, except one optional CLEANUP_STMT at the end which holds cleanups for the variable if it needs to be destructed. This CLEANUP_STMT is removed and the actual cleanup saved into another new operand, *_COND_CLEANUP. The c-family loop genericization handles such loops roughly the way https://eel.is/c++draft/stmt.for and https://eel.is/c++draft/stmt.while specifies, so the body is (if *_COND_CLEANUP is non-NULL) { A x = 42; try { if (!x) break; body; cont_label: expr; } finally { cleanup; } } and otherwise { A x = 42; if (!x) break; body; cont_label: expr; } i.e. the *_COND, *_BODY, optional continue label, FOR_EXPR are appended into the body of the *_COND_PREP BIND_EXPR. And when doing constexpr evaluation of such FOR/WHILE loops, we treat it similarly, first evaluate *_COND_PREP except the for (tree decl = BIND_EXPR_VARS (t); decl; decl = DECL_CHAIN (decl)) destroy_value_checked (ctx, decl, non_constant_p); part of BIND_EXPR handling for it, then evaluate *_COND (and decide based on whether it was false or true like before), then *_BODY, then FOR_EXPR, then *_COND_CLEANUP (similarly to the way how CLEANUP_STMT handling handles that) and finally do those destroy_value_checked. Note, the constexpr-86769.C testcase FAILs with both clang++ and MSVC (note, the rest of tests PASS with clang++) but I believe it must be just a bug in those compilers, new int is done in all the constructors and delete is done in the destructor, so when clang++ reports one of the new int weren't deallocated during constexpr evaluation I don't see how that would be possible. When the same test has all the constexpr stuff, all the new int are properly deleted at runtime when compiled by both compilers and valgrind is happy about it, no leaks. 2025-02-07 Jakub Jelinek <jakub@redhat.com> Jason Merrill <jason@redhat.com> PR c++/86769 gcc/c-family/ * c-common.def (FOR_STMT): Add 2 operands and document them. (WHILE_STMT): Likewise. * c-common.h (WHILE_COND_PREP, WHILE_COND_CLEANUP): Define. (FOR_COND_PREP, FOR_COND_CLEANUP): Define. * c-gimplify.cc (genericize_c_loop): Add COND_PREP and COND_CLEANUP arguments, handle them if they are non-NULL. (genericize_for_stmt, genericize_while_stmt, genericize_do_stmt): Adjust callers. gcc/c/ * c-parser.cc (c_parser_while_statement): Add 2 further NULL_TREE operands to build_stmt. (c_parser_for_statement): Likewise. gcc/cp/ * semantics.cc (set_one_cleanup_loc): New function. (set_cleanup_locs): Use it. (simplify_loop_decl_cond): Remove. (adjust_loop_decl_cond): New function. (begin_while_stmt): Add 2 further NULL_TREE operands to build_stmt. (finish_while_stmt_cond): Call adjust_loop_decl_cond instead of simplify_loop_decl_cond. (finish_while_stmt): Call do_poplevel also on WHILE_COND_PREP if non-NULL and also use pop_stmt_list rather than do_poplevel for WHILE_BODY in that case. Call set_one_cleanup_loc. (begin_for_stmt): Add 2 further NULL_TREE operands to build_stmt. (finish_for_cond): Call adjust_loop_decl_cond instead of simplify_loop_decl_cond. (finish_for_stmt): Call do_poplevel also on FOR_COND_PREP if non-NULL and also use pop_stmt_list rather than do_poplevel for FOR_BODY in that case. Call set_one_cleanup_loc. * constexpr.cc (cxx_eval_loop_expr): Handle {WHILE,FOR}_COND_{PREP,CLEANUP}. (check_for_return_continue): Handle {WHILE,FOR}_COND_PREP. (potential_constant_expression_1): RECUR on {WHILE,FOR}_COND_{PREP,CLEANUP}. gcc/testsuite/ * g++.dg/diagnostic/redeclaration-7.C: New test. * g++.dg/expr/for3.C: New test. * g++.dg/expr/for4.C: New test. * g++.dg/expr/for5.C: New test. * g++.dg/expr/for6.C: New test. * g++.dg/expr/for7.C: New test. * g++.dg/expr/for8.C: New test. * g++.dg/ext/stmtexpr27.C: New test. * g++.dg/cpp2a/constexpr-86769.C: New test. * g++.dg/cpp26/name-independent-decl7.C: New test. * g++.dg/cpp26/name-independent-decl8.C: New test.
Diffstat (limited to 'gcc/c/c-parser.cc')
-rw-r--r--gcc/c/c-parser.cc5
1 files changed, 3 insertions, 2 deletions
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index aa68ee2..106a5b4 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -8802,7 +8802,8 @@ c_parser_while_statement (c_parser *parser, bool ivdep, unsigned short unroll,
body = c_parser_c99_block_statement (parser, if_p, &loc_after_labels);
if (loop_name && !C_DECL_LOOP_SWITCH_NAME_USED (loop_name))
loop_name = NULL_TREE;
- add_stmt (build_stmt (loc, WHILE_STMT, cond, body, loop_name));
+ add_stmt (build_stmt (loc, WHILE_STMT, cond, body, loop_name, NULL_TREE,
+ NULL_TREE));
add_stmt (c_end_compound_stmt (loc, block, flag_isoc99));
c_parser_maybe_reclassify_token (parser);
if (num_names)
@@ -9207,7 +9208,7 @@ c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
add_stmt (build_stmt (for_loc, FOR_STMT, NULL_TREE, cond, incr,
body, NULL_TREE,
loop_name && C_DECL_LOOP_SWITCH_NAME_USED (loop_name)
- ? loop_name : NULL_TREE));
+ ? loop_name : NULL_TREE, NULL_TREE, NULL_TREE));
add_stmt (c_end_compound_stmt (for_loc, block,
flag_isoc99 || c_dialect_objc ()));
c_parser_maybe_reclassify_token (parser);