Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
On Fri, Oct 04, 2019 at 03:34:39PM -0400, Jason Merrill wrote: > > @@ -1656,6 +1671,64 @@ cxx_eval_call_expression (const constexp > >lval, non_constant_p, overflow_p); > > if (!DECL_DECLARED_CONSTEXPR_P (fun)) > > { > > + if (cxx_dialect >= cxx2a > > + && IDENTIFIER_NEWDEL_OP_P (DECL_NAME (fun)) > > + && CP_DECL_CONTEXT (fun) == global_namespace) > > I'd still like to factor this test out. OK with that change. Ok, thanks, here is what I've committed after another bootstrap/regtest on x86_64-linux and i686-linux: 2019-10-05 Jakub Jelinek PR c++/91369 - Implement P0784R7: constexpr new c-family/ * c-cppbuiltin.c (c_cpp_builtins): Predefine __cpp_constexpr_dynamic_alloc=201907 for -std=c++2a. cp/ * cp-tree.h (enum cp_tree_index): Add CPTI_HEAP_UNINIT_IDENTIFIER, CPTI_HEAP_IDENTIFIER and CPTI_HEAP_DELETED_IDENTIFIER. (heap_uninit_identifier, heap_identifier, heap_deleted_identifier): Define. (type_has_constexpr_destructor, build_new_constexpr_heap_type, cxx_constant_dtor): Declare. * class.c (type_maybe_constexpr_default_constructor): Make static. (type_maybe_constexpr_destructor, type_has_constexpr_destructor): New functions. (finalize_literal_type_property): For c++2a, don't clear CLASSTYPE_LITERAL_P for types without trivial destructors unless they have non-constexpr destructors. (explain_non_literal_class): For c++2a, complain about non-constexpr destructors rather than about non-trivial destructors. * constexpr.c: Include stor-layout.h. (struct constexpr_global_ctx): New type. (struct constexpr_ctx): Add global field, remove values and constexpr_ops_count. (cxx_replaceable_global_alloc_fn): New inline function. (cxx_eval_call_expression): For c++2a allow calls to replaceable global allocation functions, for new return address of a heap uninit var, for delete record its deletion. Change ctx->values->{get,put} to ctx->global->values.{get,put}. (non_const_var_error): Add auto_diagnostic_group sentinel. Emit special diagnostics for heap variables. (cxx_eval_store_expression): Change ctx->values->{get,put} to ctx->global->values.{get,put}. (cxx_eval_loop_expr): Initialize jump_target if NULL. Change new_ctx.values->remove to ctx->global->values.remove. (cxx_eval_constant_expression): Change *ctx->constexpr_ops_count to ctx->global->constexpr_ops_count. Change ctx->values->{get,put} to ctx->global->values.{get,put}. : Formatting fix. On cast of replaceable global allocation function to some pointer type, adjust the type of the heap variable and change name from heap_uninit_identifier to heap_identifier. (find_heap_var_refs): New function. (cxx_eval_outermost_constant_expr): Add constexpr_dtor argument, handle evaluation of constexpr dtors and add tracking of heap variables. Use tf_no_cleanup for get_target_expr_with_sfinae. (cxx_constant_value): Adjust cxx_eval_outermost_constant_expr caller. (cxx_constant_dtor): New function. (maybe_constant_value, fold_non_dependent_expr_template, maybe_constant_init_1): Adjust cxx_eval_outermost_constant_expr callers. (potential_constant_expression_1): Ignore clobbers. Allow COND_EXPR_IS_VEC_DELETE for c++2a. * decl.c (initialize_predefined_identifiers): Add heap identifiers. (decl_maybe_constant_destruction): New function. (cp_finish_decl): Don't clear TREE_READONLY for constexpr variables with non-trivial, but constexpr destructors. (register_dtor_fn): For constexpr variables with constexpr non-trivial destructors call cxx_maybe_build_cleanup instead of adding destructor calls at runtime. (expand_static_init): For constexpr variables with constexpr non-trivial destructors call cxx_maybe_build_cleanup. (grokdeclarator): Allow constexpr destructors for c++2a. Formatting fix. (cxx_maybe_build_cleanup): For constexpr variables with constexpr non-trivial destructors call cxx_constant_dtor instead of adding destructor calls at runtime. * init.c: Include stor-layout.h. (build_new_constexpr_heap_type, maybe_wrap_new_for_constexpr): New functions. (build_new_1): For c++2a and new[], add cast around the alloc call to help constexpr evaluation figure out the type of the heap storage. (build_vec_delete_1): Set DECL_INITIAL of tbase and emit a DECL_EXPR for it instead of initializing an uninitialized variable. * method.c: Include intl.h. (SFK_CTOR_P, SFK_DTOR_P, SFK_ASSIGN_P, SFK_COPY_P, SFK_MOVE_P): Move definitions earlier.
Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
On 10/4/19 1:50 PM, Jakub Jelinek wrote: On Thu, Oct 03, 2019 at 04:07:14PM -0400, Jason Merrill wrote: I believe it doesn't, because the heap VAR_DECLs are TREE_STATIC (things really don't work at all if they aren't). Ah, sure. I suppose you could clear TREE_STATIC from them before the verify_constant in this function? That works, but generates uglier diagnostics. Otherwise, I've tried to address all your comments. So, here is the updated patch without the find_heap_var_refs removal and TREE_STATIC clearing (tested so far just with make check-c++-all RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} dg.exp='constexpr-array* eval-order* is_literal* constexpr-new* constexpr-dtor* constexpr-delete* locations1.C feat-cxx*' ) plus attached incremental patch for the find_heap_var_refs removal and TREE_STATIC clearing. The difference can be seen in the incremental diff, the ugly part is error: '(int*)(& heap deleted)' is not a constant expression but it will show up only if the constant to be verified contains pointers to deallocated heap, if it still contains pointers to allocated heap, it will instead complain about allocated heap not being deallocated. 2019-10-04 Jakub Jelinek PR c++/91369 - Implement P0784R7: constexpr new c-family/ * c-cppbuiltin.c (c_cpp_builtins): Predefine __cpp_constexpr_dynamic_alloc=201907 for -std=c++2a. cp/ * cp-tree.h (enum cp_tree_index): Add CPTI_HEAP_UNINIT_IDENTIFIER, CPTI_HEAP_IDENTIFIER and CPTI_HEAP_DELETED_IDENTIFIER. (heap_uninit_identifier, heap_identifier, heap_deleted_identifier): Define. (type_has_constexpr_destructor, build_new_constexpr_heap_type, cxx_constant_dtor): Declare. * class.c (type_maybe_constexpr_default_constructor): Make static. (type_maybe_constexpr_destructor, type_has_constexpr_destructor): New functions. (finalize_literal_type_property): For c++2a, don't clear CLASSTYPE_LITERAL_P for types without trivial destructors unless they have non-constexpr destructors. (explain_non_literal_class): For c++2a, complain about non-constexpr destructors rather than about non-trivial destructors. * constexpr.c: Include stor-layout.h. (struct constexpr_global_ctx): New type. (struct constexpr_ctx): Add global field, remove values and constexpr_ops_count. (cxx_eval_call_expression): For c++2a allow calls to replaceable global allocation functions, for new return address of a heap uninit var, for delete record its deletion. Change ctx->values->{get,put} to ctx->global->values.{get,put}. (non_const_var_error): Add auto_diagnostic_group sentinel. Emit special diagnostics for heap variables. (cxx_eval_store_expression): Change ctx->values->{get,put} to ctx->global->values.{get,put}. (cxx_eval_loop_expr): Initialize jump_target if NULL. Change new_ctx.values->remove to ctx->global->values.remove. (cxx_eval_constant_expression): Change *ctx->constexpr_ops_count to ctx->global->constexpr_ops_count. Change ctx->values->{get,put} to ctx->global->values.{get,put}. : Formatting fix. On cast of replaceable global allocation function to some pointer type, adjust the type of the heap variable and change name from heap_uninit_identifier to heap_identifier. (find_heap_var_refs): New function. (cxx_eval_outermost_constant_expr): Add constexpr_dtor argument, handle evaluation of constexpr dtors and add tracking of heap variables. Use tf_no_cleanup for get_target_expr_with_sfinae. (cxx_constant_value): Adjust cxx_eval_outermost_constant_expr caller. (cxx_constant_dtor): New function. (maybe_constant_value, fold_non_dependent_expr_template, maybe_constant_init_1): Adjust cxx_eval_outermost_constant_expr callers. (potential_constant_expression_1): Ignore clobbers. Allow COND_EXPR_IS_VEC_DELETE for c++2a. * decl.c (initialize_predefined_identifiers): Add heap identifiers. (decl_maybe_constant_destruction): New function. (cp_finish_decl): Don't clear TREE_READONLY for constexpr variables with non-trivial, but constexpr destructors. (register_dtor_fn): For constexpr variables with constexpr non-trivial destructors call cxx_maybe_build_cleanup instead of adding destructor calls at runtime. (expand_static_init): For constexpr variables with constexpr non-trivial destructors call cxx_maybe_build_cleanup. (grokdeclarator): Allow constexpr destructors for c++2a. Formatting fix. (cxx_maybe_build_cleanup): For constexpr variables with constexpr non-trivial destructors call cxx_constant_dtor instead of adding destructor calls at runtime. * init.c: Include stor-layout.h.
Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
On Thu, Oct 03, 2019 at 04:07:14PM -0400, Jason Merrill wrote: > > I believe it doesn't, because the heap VAR_DECLs are TREE_STATIC (things > > really don't work at all if they aren't). > > Ah, sure. I suppose you could clear TREE_STATIC from them before the > verify_constant in this function? That works, but generates uglier diagnostics. Otherwise, I've tried to address all your comments. So, here is the updated patch without the find_heap_var_refs removal and TREE_STATIC clearing (tested so far just with make check-c++-all RUNTESTFLAGS="--target_board=unix\{-m32,-m64\} dg.exp='constexpr-array* eval-order* is_literal* constexpr-new* constexpr-dtor* constexpr-delete* locations1.C feat-cxx*' ) plus attached incremental patch for the find_heap_var_refs removal and TREE_STATIC clearing. The difference can be seen in the incremental diff, the ugly part is error: '(int*)(& heap deleted)' is not a constant expression but it will show up only if the constant to be verified contains pointers to deallocated heap, if it still contains pointers to allocated heap, it will instead complain about allocated heap not being deallocated. 2019-10-04 Jakub Jelinek PR c++/91369 - Implement P0784R7: constexpr new c-family/ * c-cppbuiltin.c (c_cpp_builtins): Predefine __cpp_constexpr_dynamic_alloc=201907 for -std=c++2a. cp/ * cp-tree.h (enum cp_tree_index): Add CPTI_HEAP_UNINIT_IDENTIFIER, CPTI_HEAP_IDENTIFIER and CPTI_HEAP_DELETED_IDENTIFIER. (heap_uninit_identifier, heap_identifier, heap_deleted_identifier): Define. (type_has_constexpr_destructor, build_new_constexpr_heap_type, cxx_constant_dtor): Declare. * class.c (type_maybe_constexpr_default_constructor): Make static. (type_maybe_constexpr_destructor, type_has_constexpr_destructor): New functions. (finalize_literal_type_property): For c++2a, don't clear CLASSTYPE_LITERAL_P for types without trivial destructors unless they have non-constexpr destructors. (explain_non_literal_class): For c++2a, complain about non-constexpr destructors rather than about non-trivial destructors. * constexpr.c: Include stor-layout.h. (struct constexpr_global_ctx): New type. (struct constexpr_ctx): Add global field, remove values and constexpr_ops_count. (cxx_eval_call_expression): For c++2a allow calls to replaceable global allocation functions, for new return address of a heap uninit var, for delete record its deletion. Change ctx->values->{get,put} to ctx->global->values.{get,put}. (non_const_var_error): Add auto_diagnostic_group sentinel. Emit special diagnostics for heap variables. (cxx_eval_store_expression): Change ctx->values->{get,put} to ctx->global->values.{get,put}. (cxx_eval_loop_expr): Initialize jump_target if NULL. Change new_ctx.values->remove to ctx->global->values.remove. (cxx_eval_constant_expression): Change *ctx->constexpr_ops_count to ctx->global->constexpr_ops_count. Change ctx->values->{get,put} to ctx->global->values.{get,put}. : Formatting fix. On cast of replaceable global allocation function to some pointer type, adjust the type of the heap variable and change name from heap_uninit_identifier to heap_identifier. (find_heap_var_refs): New function. (cxx_eval_outermost_constant_expr): Add constexpr_dtor argument, handle evaluation of constexpr dtors and add tracking of heap variables. Use tf_no_cleanup for get_target_expr_with_sfinae. (cxx_constant_value): Adjust cxx_eval_outermost_constant_expr caller. (cxx_constant_dtor): New function. (maybe_constant_value, fold_non_dependent_expr_template, maybe_constant_init_1): Adjust cxx_eval_outermost_constant_expr callers. (potential_constant_expression_1): Ignore clobbers. Allow COND_EXPR_IS_VEC_DELETE for c++2a. * decl.c (initialize_predefined_identifiers): Add heap identifiers. (decl_maybe_constant_destruction): New function. (cp_finish_decl): Don't clear TREE_READONLY for constexpr variables with non-trivial, but constexpr destructors. (register_dtor_fn): For constexpr variables with constexpr non-trivial destructors call cxx_maybe_build_cleanup instead of adding destructor calls at runtime. (expand_static_init): For constexpr variables with constexpr non-trivial destructors call cxx_maybe_build_cleanup. (grokdeclarator): Allow constexpr destructors for c++2a. Formatting fix. (cxx_maybe_build_cleanup): For constexpr variables with constexpr non-trivial destructors call cxx_constant_dtor instead of adding destructor calls at runtime. * init.c: Include stor-layout.h.
Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
On 10/3/19 2:38 PM, Jakub Jelinek wrote: On Tue, Oct 01, 2019 at 05:56:00PM -0400, Jason Merrill wrote: + else if (DECL_NAME (var) == heap_deleted_identifier) + { + if (!ctx->quiet) + error_at (loc, "deallocation of already deallocated " + "storage"); + *non_constant_p = true; + return t; + } Don't we need an error for trying to deallocate something that wasn't allocated within the constexpr evaluation? We don't need it, but it might be a good idea. Right now it will just fall through into the non-constexpr call in a constexpr context diagnostics, while a message that it isn't constexpr because it is trying to deallocate something that hasn't been allocated might be clearer. Will add. @@ -3011,8 +3067,10 @@ initialized_type (tree t) { /* A constructor call has void type, so we need to look deeper. */ tree fn = get_function_named_in_call (t); - if (fn && TREE_CODE (fn) == FUNCTION_DECL - && DECL_CXX_CONSTRUCTOR_P (fn)) + if (fn + && TREE_CODE (fn) == FUNCTION_DECL + && (DECL_CXX_CONSTRUCTOR_P (fn) + || (cxx_dialect >= cxx2a && DECL_CXX_DESTRUCTOR_P (fn type = DECL_CONTEXT (fn); Why is this needed? A destructor doesn't initialize anything, so returning void seems appropriate. This was needed initially, because the evaluate outermost expr if it sees void type just returns early, does nothing. But later I found out that while the above worked for the normal dtor case, for the array dtor case it didn't work anyway and I've added an extra argument. So this is not needed anymore and will delete. I think we want to factor this function more, so we don't have the same code in multiple places for handling an array, and an array member, and a pointer to array. Do you want to take a look at bug 71504 while you're touching this code? Patch posted separately. + if (!valp + && VAR_P (object) + && DECL_NAME (object) == heap_identifier) +{ + tree ctor = build_constructor (type, NULL); + CONSTRUCTOR_NO_CLEARING (ctor) = true; + ctx->values->put (object, ctor); + valp = ctx->values->get (object); +} Instead of this, how about giving the object NULL_TREE value when we create it in cxx_eval_call_expression? Will try. + if (tree fld1 = TYPE_FIELDS (var_type)) + if (TREE_CODE (fld1) == FIELD_DECL + && DECL_NAME (fld1) == NULL_TREE + && DECL_ARTIFICIAL (fld1) + && TREE_CODE (TREE_TYPE (fld1)) == ARRAY_TYPE + && COMPLETE_TYPE_P (TREE_TYPE (fld1))) + if (tree fld2 = DECL_CHAIN (fld1)) + if (TREE_CODE (fld2) == FIELD_DECL + && DECL_NAME (fld2) == NULL_TREE + && DECL_ARTIFICIAL (fld2) + && TREE_CODE (TREE_TYPE (fld2)) == ARRAY_TYPE + && TYPE_DOMAIN (TREE_TYPE (fld2)) == NULL_TREE + && DECL_CHAIN (fld2) == NULL_TREE) Maybe give the struct a magic name so you don't need to do as much checking of the FIELD_DECLs? Ok. So here you're completing the type of the array member of the struct. + TREE_TYPE (var) = var_type; + TREE_TYPE (TREE_OPERAND (op, 0)) + = build_pointer_type (var_type); + } + } Let's factor out all of this code, too. Yes, it is used twice, so factoring it out makes a lot of sense. @@ -5525,16 +5762,23 @@ cxx_eval_outermost_constant_expr (tree t bool non_constant_p = false; bool overflow_p = false; hash_map map; + auto_vec heap_vars; HOST_WIDE_INT constexpr_ctx_count = 0; constexpr_ctx ctx = { NULL, , NULL, NULL, NULL, NULL, - _ctx_count, allow_non_constant, strict, - manifestly_const_eval || !allow_non_constant }; + _ctx_count, _vars, allow_non_constant, + strict, manifestly_const_eval || !allow_non_constant }; As we add more stuff to constexpr_ctx, creating new ones on the stack becomes more and more expensive. We should really split off the parts that change frequently: Maybe just ctor/object, maybe also call/save_exprs/...? I was thinking about creating a constexpr_outermost_ctx that would include things that are global and don't change, at least the count and this vector. Not sure about the bool fields, we access them way too often that it might be too ugly to change all the ctx->quiet to ctx->outermost->quite or similar. @@ -5593,6 +5852,32 @@ cxx_eval_outermost_constant_expr (tree t non_constant_p = true; } + if (!heap_vars.is_empty ()) +{ + tree heap_var = cp_walk_tree_without_duplicates (, find_heap_var_refs, +
Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
On Tue, Oct 01, 2019 at 05:56:00PM -0400, Jason Merrill wrote: > > + else if (DECL_NAME (var) == heap_deleted_identifier) > > + { > > + if (!ctx->quiet) > > + error_at (loc, "deallocation of already deallocated " > > + "storage"); > > + *non_constant_p = true; > > + return t; > > + } > > Don't we need an error for trying to deallocate something that wasn't > allocated within the constexpr evaluation? We don't need it, but it might be a good idea. Right now it will just fall through into the non-constexpr call in a constexpr context diagnostics, while a message that it isn't constexpr because it is trying to deallocate something that hasn't been allocated might be clearer. Will add. > > @@ -3011,8 +3067,10 @@ initialized_type (tree t) > > { > > /* A constructor call has void type, so we need to look deeper. */ > > tree fn = get_function_named_in_call (t); > > - if (fn && TREE_CODE (fn) == FUNCTION_DECL > > - && DECL_CXX_CONSTRUCTOR_P (fn)) > > + if (fn > > + && TREE_CODE (fn) == FUNCTION_DECL > > + && (DECL_CXX_CONSTRUCTOR_P (fn) > > + || (cxx_dialect >= cxx2a && DECL_CXX_DESTRUCTOR_P (fn > > type = DECL_CONTEXT (fn); > > Why is this needed? A destructor doesn't initialize anything, so returning > void seems appropriate. This was needed initially, because the evaluate outermost expr if it sees void type just returns early, does nothing. But later I found out that while the above worked for the normal dtor case, for the array dtor case it didn't work anyway and I've added an extra argument. So this is not needed anymore and will delete. > I think we want to factor this function more, so we don't have the same code > in multiple places for handling an array, and an array member, and a pointer > to array. Do you want to take a look at bug 71504 while you're touching > this code? Patch posted separately. > > + if (!valp > > + && VAR_P (object) > > + && DECL_NAME (object) == heap_identifier) > > +{ > > + tree ctor = build_constructor (type, NULL); > > + CONSTRUCTOR_NO_CLEARING (ctor) = true; > > + ctx->values->put (object, ctor); > > + valp = ctx->values->get (object); > > +} > > Instead of this, how about giving the object NULL_TREE value when we create > it in cxx_eval_call_expression? Will try. > > + if (tree fld1 = TYPE_FIELDS (var_type)) > > + if (TREE_CODE (fld1) == FIELD_DECL > > + && DECL_NAME (fld1) == NULL_TREE > > + && DECL_ARTIFICIAL (fld1) > > + && TREE_CODE (TREE_TYPE (fld1)) == ARRAY_TYPE > > + && COMPLETE_TYPE_P (TREE_TYPE (fld1))) > > + if (tree fld2 = DECL_CHAIN (fld1)) > > + if (TREE_CODE (fld2) == FIELD_DECL > > + && DECL_NAME (fld2) == NULL_TREE > > + && DECL_ARTIFICIAL (fld2) > > + && TREE_CODE (TREE_TYPE (fld2)) == ARRAY_TYPE > > + && TYPE_DOMAIN (TREE_TYPE (fld2)) == NULL_TREE > > + && DECL_CHAIN (fld2) == NULL_TREE) > > Maybe give the struct a magic name so you don't need to do as much checking > of the FIELD_DECLs? Ok. > So here you're completing the type of the array member of the struct. > > > + TREE_TYPE (var) = var_type; > > + TREE_TYPE (TREE_OPERAND (op, 0)) > > + = build_pointer_type (var_type); > > + } > > + } > > Let's factor out all of this code, too. Yes, it is used twice, so factoring it out makes a lot of sense. > > @@ -5525,16 +5762,23 @@ cxx_eval_outermost_constant_expr (tree t > > bool non_constant_p = false; > > bool overflow_p = false; > > hash_map map; > > + auto_vec heap_vars; > > HOST_WIDE_INT constexpr_ctx_count = 0; > > constexpr_ctx ctx = { NULL, , NULL, NULL, NULL, NULL, > > - _ctx_count, allow_non_constant, strict, > > - manifestly_const_eval || !allow_non_constant }; > > + _ctx_count, _vars, allow_non_constant, > > + strict, manifestly_const_eval || !allow_non_constant }; > > As we add more stuff to constexpr_ctx, creating new ones on the stack > becomes more and more expensive. We should really split off the parts that > change frequently: Maybe just ctor/object, maybe also call/save_exprs/...? I was thinking about creating a constexpr_outermost_ctx that would include things that are global and don't change, at least the count and this vector. Not sure about the bool fields, we access them way too often that it might be too ugly to change all the ctx->quiet to ctx->outermost->quite or similar. > > @@ -5593,6 +5852,32 @@ cxx_eval_outermost_constant_expr (tree t > > non_constant_p = true; > > } > > + if (!heap_vars.is_empty ()) > > +{ > > + tree heap_var =
Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
On 10/2/19 7:36 AM, Jakub Jelinek wrote: Hi! Thanks for the review, let me start with the unrelated bugfixes then and deal with the rest later. On Tue, Oct 01, 2019 at 05:56:00PM -0400, Jason Merrill wrote: @@ -3905,7 +4033,7 @@ cxx_eval_store_expression (const constex bool no_zero_init = true; releasing_vec ctors; - while (!refs->is_empty()) + while (!refs->is_empty ()) { if (*valp == NULL_TREE) { @@ -4046,7 +4174,9 @@ cxx_eval_store_expression (const constex if (const_object_being_modified) { bool fail = false; - if (!CLASS_TYPE_P (TREE_TYPE (const_object_being_modified))) + tree const_objtype + = strip_array_types (TREE_TYPE (const_object_being_modified)); + if (!CLASS_TYPE_P (const_objtype)) This looks like an unrelated bugfix; you might commit it (and the others) separately if that's convenient. @@ -4907,14 +5043,21 @@ cxx_eval_constant_expression (const cons break; case CLEANUP_STMT: - r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval, + { + tree initial_jump_target = jump_target ? *jump_target : NULL_TREE; + r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval, + non_constant_p, overflow_p, + jump_target); + if (!CLEANUP_EH_ONLY (t) && !*non_constant_p) + /* Also evaluate the cleanup. If we weren't skipping at the +start of the CLEANUP_BODY, change jump_target temporarily +to _jump_target, so that even a return or break or +continue in the body doesn't skip the cleanup. */ This also looks like an unrelated bugfix. Both are only partially related, I think they can go in separately, but at least for the first one I haven't managed to come up with a testcase where it would matter and nothing in e.g. check-c++-all comes there with ARRAY_TYPE, is it ok for trunk without a testcase (first attached patch)? As for the second bugfix, I think it should be accompanied with the potential_constant_expression_1 change, and there I've actually managed to come up with a testcase where it matters, though it is using a GCC extension (generally, CLEANUP_STMTs won't appear in constexpr functions that often because of the requirement that variables in them have literal type and literal types have trivial destructors, so really no cleanups in that case). The testcase where it makes a difference is: constexpr void cleanup (int *x) { if (x) asm (""); } constexpr void cleanup2 (int *x) { } constexpr bool foo () { int a __attribute__((cleanup (cleanup))) = 1; return true; } constexpr bool bar () { int a __attribute__((cleanup (cleanup2))) = 1; return true; } constexpr auto x = foo (); constexpr auto y = bar (); With vanilla trunk, one gets a weird message: test.C:27:24: error: ‘constexpr bool foo()’ called in a constant expression 27 | constexpr auto x = foo (); |^~ test.C:28:24: error: ‘constexpr bool bar()’ called in a constant expression 28 | constexpr auto y = bar (); |^~ That is because we call potential_constant_expression_1 on the foo (or bar) body, see CLEANUP_STMT in there and punt. With just the potential_constant_expression_1 change to handle CLEANUP_STMT, we actually accept it, which is wrong. Finally, with the whole patch attached below, we reject foo () call and accept bar (): test.C:27:24: in ‘constexpr’ expansion of ‘foo()’ test.C:27:25: in ‘constexpr’ expansion of ‘cleanup((& a))’ test.C:5:5: error: inline assembly is not a constant expression 5 | asm (""); | ^~~ test.C:5:5: note: only unevaluated inline assembly is allowed in a ‘constexpr’ function in C++2a Is the testcase ok in the second patch? Are those patches ok for trunk? OK, thanks. Jason
Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
Hi! Thanks for the review, let me start with the unrelated bugfixes then and deal with the rest later. On Tue, Oct 01, 2019 at 05:56:00PM -0400, Jason Merrill wrote: > > @@ -3905,7 +4033,7 @@ cxx_eval_store_expression (const constex > > bool no_zero_init = true; > > releasing_vec ctors; > > - while (!refs->is_empty()) > > + while (!refs->is_empty ()) > > { > > if (*valp == NULL_TREE) > > { > > @@ -4046,7 +4174,9 @@ cxx_eval_store_expression (const constex > > if (const_object_being_modified) > > { > > bool fail = false; > > - if (!CLASS_TYPE_P (TREE_TYPE (const_object_being_modified))) > > + tree const_objtype > > + = strip_array_types (TREE_TYPE (const_object_being_modified)); > > + if (!CLASS_TYPE_P (const_objtype)) > > This looks like an unrelated bugfix; you might commit it (and the others) > separately if that's convenient. > > > @@ -4907,14 +5043,21 @@ cxx_eval_constant_expression (const cons > > break; > > case CLEANUP_STMT: > > - r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval, > > + { > > + tree initial_jump_target = jump_target ? *jump_target : NULL_TREE; > > + r = cxx_eval_constant_expression (ctx, CLEANUP_BODY (t), lval, > > + non_constant_p, overflow_p, > > + jump_target); > > + if (!CLEANUP_EH_ONLY (t) && !*non_constant_p) > > + /* Also evaluate the cleanup. If we weren't skipping at the > > +start of the CLEANUP_BODY, change jump_target temporarily > > +to _jump_target, so that even a return or break or > > +continue in the body doesn't skip the cleanup. */ > > This also looks like an unrelated bugfix. Both are only partially related, I think they can go in separately, but at least for the first one I haven't managed to come up with a testcase where it would matter and nothing in e.g. check-c++-all comes there with ARRAY_TYPE, is it ok for trunk without a testcase (first attached patch)? As for the second bugfix, I think it should be accompanied with the potential_constant_expression_1 change, and there I've actually managed to come up with a testcase where it matters, though it is using a GCC extension (generally, CLEANUP_STMTs won't appear in constexpr functions that often because of the requirement that variables in them have literal type and literal types have trivial destructors, so really no cleanups in that case). The testcase where it makes a difference is: constexpr void cleanup (int *x) { if (x) asm (""); } constexpr void cleanup2 (int *x) { } constexpr bool foo () { int a __attribute__((cleanup (cleanup))) = 1; return true; } constexpr bool bar () { int a __attribute__((cleanup (cleanup2))) = 1; return true; } constexpr auto x = foo (); constexpr auto y = bar (); With vanilla trunk, one gets a weird message: test.C:27:24: error: ‘constexpr bool foo()’ called in a constant expression 27 | constexpr auto x = foo (); |^~ test.C:28:24: error: ‘constexpr bool bar()’ called in a constant expression 28 | constexpr auto y = bar (); |^~ That is because we call potential_constant_expression_1 on the foo (or bar) body, see CLEANUP_STMT in there and punt. With just the potential_constant_expression_1 change to handle CLEANUP_STMT, we actually accept it, which is wrong. Finally, with the whole patch attached below, we reject foo () call and accept bar (): test.C:27:24: in ‘constexpr’ expansion of ‘foo()’ test.C:27:25: in ‘constexpr’ expansion of ‘cleanup((& a))’ test.C:5:5: error: inline assembly is not a constant expression 5 | asm (""); | ^~~ test.C:5:5: note: only unevaluated inline assembly is allowed in a ‘constexpr’ function in C++2a Is the testcase ok in the second patch? Are those patches ok for trunk? Jakub 2019-10-02 Jakub Jelinek * constexpr.c (cxx_eval_store_expression): Formatting fix. Handle const_object_being_modified with array type. --- gcc/cp/constexpr.c.jj 2019-09-27 20:33:37.600208356 +0200 +++ gcc/cp/constexpr.c 2019-09-27 20:38:38.203710246 +0200 @@ -3905,7 +4033,7 @@ cxx_eval_store_expression (const constex bool no_zero_init = true; releasing_vec ctors; - while (!refs->is_empty()) + while (!refs->is_empty ()) { if (*valp == NULL_TREE) { @@ -4046,7 +4174,9 @@ cxx_eval_store_expression (const constex if (const_object_being_modified) { bool fail = false; - if (!CLASS_TYPE_P (TREE_TYPE (const_object_being_modified))) + tree const_objtype + = strip_array_types (TREE_TYPE (const_object_being_modified)); + if (!CLASS_TYPE_P (const_objtype)) fail = true; else { 2019-10-02 Jakub Jelinek * constexpr.c (cxx_eval_constant_expression) : If not skipping upon entry to body, run cleanup with the same *jump_target
Re: [C++ PATCH] PR c++/91369 - Implement P0784R7: constexpr new
On 9/27/19 4:31 PM, Jakub Jelinek wrote: Hi! The following patch attempts to implement P0784R7, which includes constexpr destructors and constexpr new/delete/new[]/delete[]. ::operator new is allowed during constexpr evaluation and returns address of an artificial VAR_DECL with special name. At this point we don't really know the type of the heap storage, just size. Later on when we encounter cast to the corresponding pointer type, we change the name of the var and type to match the type from the new expression (for new[] we need to do further stuff as at the point where build_new_1 is called, we might not know the exact array size, but we shall know that during the constexpr evaluation, and cookie handling also complicates it a little bit). When we first store into such heap objects, a ctor is created for them on the fly. Finally, ::operator delete marks those heap VAR_DECLs as deleted and cxx_eval_outermost_constant_expr checks if everything that has been allocated has been also deallocated and verifies addresses of those heap vars aren't leaking into the return value. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2019-09-27 Jakub Jelinek PR c++/91369 - Implement P0784R7: constexpr new c-family/ * c-cppbuiltin.c (c_cpp_builtins): Predefine __cpp_constexpr_dynamic_alloc=201907 for -std=c++2a. cp/ * cp-tree.h (enum cp_tree_index): Add CPTI_HEAP_UNINIT_IDENTIFIER, CPTI_HEAP_IDENTIFIER and CPTI_HEAP_DELETED_IDENTIFIER. (heap_uninit_identifier, heap_identifier, heap_deleted_identifier): Define. (type_has_constexpr_destructor, cxx_constant_dtor): Declare. * class.c (type_maybe_constexpr_default_constructor): Make static. (type_maybe_constexpr_destructor, type_has_constexpr_destructor): New functions. (finalize_literal_type_property): For c++2a, don't clear CLASSTYPE_LITERAL_P for types without trivial destructors unless they have non-constexpr destructors. (explain_non_literal_class): For c++2a, complain about non-constexpr destructors rather than about non-trivial destructors. * constexpr.c: Include stor-layout.h. (struct constexpr_ctx): Add heap_vars field. (cxx_eval_call_expression): For c++2a allow calls to replaceable global allocation functions, for new return address of a heap uninit var, for delete record its deletion. (initialized_type): Handle destructors for c++2a. (cxx_fold_indirect_ref): Also handle array fields in structures. (non_const_var_error): Add auto_diagnostic_group sentinel. Emit special diagnostics for heap variables. (cxx_eval_store_expression): Create ctor for heap variables on the first write. Formatting fix. Handle const_object_being_modified with array type. (cxx_eval_loop_expr): Initialize jump_target if NULL. (cxx_eval_constant_expression) : If not skipping upon entry to body, run cleanup with the same *jump_target as it started to run the cleanup even if the body returns, breaks or continues. : Formatting fix. On cast of replaceable global allocation function to some pointer type, adjust the type of the heap variable and change name from heap_uninit_identifier to heap_identifier. (find_heap_var_refs): New function. (cxx_eval_outermost_constant_expr): Add constexpr_dtor argument, handle evaluation of constexpr dtors and add tracking of heap variables. Use tf_no_cleanup for get_target_expr_with_sfinae. (cxx_constant_value): Adjust cxx_eval_outermost_constant_expr caller. (cxx_constant_dtor): New function. (maybe_constant_value, fold_non_dependent_expr_template, maybe_constant_init_1): Adjust cxx_eval_outermost_constant_expr callers. (potential_constant_expression_1): Ignore clobbers. Allow COND_EXPR_IS_VEC_DELETE for c++2a. Allow CLEANUP_STMT. * decl.c (initialize_predefined_identifiers): Add heap identifiers. (cp_finish_decl): Don't clear TREE_READONLY for constexpr variables with non-trivial, but constexpr destructors. (register_dtor_fn): For constexpr variables with constexpr non-trivial destructors call cxx_constant_dtor instead of adding destructor calls at runtime. (expand_static_init): For constexpr variables with constexpr non-trivial destructors call cxx_maybe_build_cleanup. (grokdeclarator): Allow constexpr destructors for c++2a. Formatting fix. (cxx_maybe_build_cleanup): For constexpr variables with constexpr non-trivial destructors call cxx_constant_dtor instead of adding destructor calls at runtime. * init.c: Include stor-layout.h. (build_new_1): For c++2a and new[], add cast around the alloc call to help constexpr