------- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-18 05:38 ------- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types
On Mar 17, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote: > On Thu, Mar 17, 2005 at 05:11:08AM -0300, Alexandre Oliva wrote: >> * gimplify.c (gimplify_decl_expr): Add temp variable to binding >> before gimplifying its initializer. > Ok. >> +cp_gimplify_compound_literal_expr (tree *expr_p, tree *pre_p, tree *post_p) >> + return gimplify_expr (expr_p, pre_p, post_p, is_gimple_val, >> fb_rvalue); > You don't need to recurse here. Just return GS_OK. Neat! >> + /* If no cleanups are needed, we can do things in a simpler way. */ >> + gimplify_and_add (decl_s, pre_p); >> + *expr_p = decl; >> + return GS_OK; > If you've simplified to a decl, you can return GS_ALL_DONE. Cool! I've now made the same change to the function in c-gimplify.c that handles compound literal exprs, from which I'd copied this code. >> + case COMPOUND_LITERAL_EXPR: >> + cp_gimplify_compound_literal_expr (expr_p, pre_p, post_p); >> + ret = GS_OK; > Should be "ret = cp_gimplify_compound_literal_expr (...)". Doh. One shouldn't change the return type of a function from void to something else without checking existing calls, even ones introduced in the same patch :-/ >> +// { dg-final { scan-assembler "_ZN1sD1Ev.*_ZN1sD1Ev.*_ZN1sD1Ev" } } >> + >> +// Make sure we issue 3 dtor calls: one for the t::x, one for the s >> +// temporary in normal execution flow and one for the same s temporary >> +// in the EH cleanup should the dtor of t::x throw. > Wouldn't it be a lot safer to make this a run test instead? Indeed. Done. Here's the latest revision of the patch, yet to be bootstrapped and tested on x86_64-linux-gnu. Hopefully my box won't hang overnight again :-( It's giving me a lot of grief, but I still haven't figured out what's wrong with it. Could be the FCdevel kernel built with gcc4 exposing bugs in usb2 or firewire disk drivers, or the internal ide HD that a few weeks ago looked like it was going to die but then apparently changed its mind :-( Index: gcc/ChangeLog from Alexandre Oliva <[EMAIL PROTECTED]> PR c++/20103 * gimplify.c (gimplify_decl_expr): Add temp variable to binding before gimplifying its initializer. * c-gimplify.c (gimplify_compound_literal_expr): After replacing the expr with a decl, we're all done. Index: gcc/gimplify.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v retrieving revision 2.118 diff -u -p -r2.118 gimplify.c --- gcc/gimplify.c 14 Mar 2005 20:01:40 -0000 2.118 +++ gcc/gimplify.c 18 Mar 2005 05:30:03 -0000 @@ -990,6 +990,15 @@ gimplify_decl_expr (tree *stmt_p) { tree init = DECL_INITIAL (decl); + /* This decl isn't mentioned in the enclosing block, so add it + to the list of temps. FIXME it seems a bit of a kludge to + say that anonymous artificial vars aren't pushed, but + everything else is. c_gimplify_compound_literal_expr may + have already added this tmp var, so don't do it again in this + case. */ + if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE) + gimple_add_tmp_var (decl); + if (!TREE_CONSTANT (DECL_SIZE (decl))) { /* This is a variable-sized decl. Simplify its size and mark it @@ -1043,12 +1052,6 @@ gimplify_decl_expr (tree *stmt_p) as they may contain a label address. */ walk_tree (&init, force_labels_r, NULL, NULL); } - - /* This decl isn't mentioned in the enclosing block, so add it to the - list of temps. FIXME it seems a bit of a kludge to say that - anonymous artificial vars aren't pushed, but everything else is. */ - if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE) - gimple_add_tmp_var (decl); } return GS_ALL_DONE; Index: gcc/c-gimplify.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/c-gimplify.c,v retrieving revision 2.26 diff -u -p -r2.26 c-gimplify.c --- gcc/c-gimplify.c 27 Jan 2005 18:22:19 -0000 2.26 +++ gcc/c-gimplify.c 18 Mar 2005 05:29:53 -0000 @@ -487,7 +487,7 @@ gimplify_compound_literal_expr (tree *ex gimplify_and_add (decl_s, pre_p); *expr_p = decl; - return GS_OK; + return GS_ALL_DONE; } /* Do C-specific gimplification. Args are as for gimplify_expr. */ Index: gcc/cp/ChangeLog from Alexandre Oliva <[EMAIL PROTECTED]> PR c++/20103 * cp-tree.h (build_compound_literal): Declare. * semantics.c (finish_compound_literal): Move most of the code... * tree.c (build_compound_literal): ... here. New function. (lvalue_p_1): Handle COMPOUND_LITERAL_EXPR. (stabilize_init): Likewise. * pt.c (tsubst_copy_and_build): Likewise. * call.c (build_over_call): Likewise. * class.c (fixed_type_or_null): Likewise. * cp-gimplify.c (cp_gimplify_init_expr): Likewise. (cp_gimplify_compound_literal_expr): New. (cp_gimplify_expr): Use it. * cvt.c (force_rvalue, ocp_convert): Handle COMPOUND_LITERAL_EXPR. * typeck.c (build_x_unary_op): Likewise. (cxx_mark_addressable): Likewise. (maybe_warn_about_returning_address_of_local): Likewise. Index: gcc/cp/cp-tree.h =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v retrieving revision 1.1111 diff -u -p -r1.1111 cp-tree.h --- gcc/cp/cp-tree.h 17 Mar 2005 14:37:04 -0000 1.1111 +++ gcc/cp/cp-tree.h 18 Mar 2005 05:30:12 -0000 @@ -4219,6 +4219,7 @@ extern tree build_min_nt (enum tree_co extern tree build_min_non_dep (enum tree_code, tree, ...); extern tree build_cplus_new (tree, tree); extern tree get_target_expr (tree); +extern tree build_compound_literal (tree, tree); extern tree build_cplus_array_type (tree, tree); extern tree hash_tree_cons (tree, tree, tree); extern tree hash_tree_chain (tree, tree); Index: gcc/cp/semantics.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v retrieving revision 1.464 diff -u -p -r1.464 semantics.c --- gcc/cp/semantics.c 14 Mar 2005 14:33:34 -0000 1.464 +++ gcc/cp/semantics.c 18 Mar 2005 05:30:14 -0000 @@ -1980,23 +1980,16 @@ finish_compound_literal (tree type, tree compound_literal = build_constructor (NULL_TREE, initializer_list); /* Mark it as a compound-literal. */ TREE_HAS_CONSTRUCTOR (compound_literal) = 1; + if (processing_template_decl) + /* This causes template substitution to run digest_init on the + CONSTRUCTOR. */ TREE_TYPE (compound_literal) = type; else - { - /* Check the initialization. */ - compound_literal = digest_init (type, compound_literal, NULL); - /* If the TYPE was an array type with an unknown bound, then we can - figure out the dimension now. For example, something like: - - `(int []) { 2, 3 }' - - implies that the array has two elements. */ - if (TREE_CODE (type) == ARRAY_TYPE && !COMPLETE_TYPE_P (type)) - complete_array_type (type, compound_literal, 1); - } + /* Check the initialization. */ + compound_literal = digest_init (type, compound_literal, NULL); - return compound_literal; + return build_compound_literal (type, compound_literal); } /* Return the declaration for the function-name variable indicated by Index: gcc/cp/tree.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/tree.c,v retrieving revision 1.427 diff -u -p -r1.427 tree.c --- gcc/cp/tree.c 21 Feb 2005 23:12:27 -0000 1.427 +++ gcc/cp/tree.c 18 Mar 2005 05:30:16 -0000 @@ -154,6 +154,7 @@ lvalue_p_1 (tree ref, treat_class_rvalues_as_lvalues); case TARGET_EXPR: + case COMPOUND_LITERAL_EXPR: return treat_class_rvalues_as_lvalues ? clk_class : clk_none; case CALL_EXPR: @@ -365,6 +366,43 @@ get_target_expr (tree init) return build_target_expr_with_type (init, TREE_TYPE (init)); } +/* Build a COMPOUND_LITERAL_EXPR. TYPE is the type given in the + compound literal, which may be an incomplete array type completed + by the initializer; COMPOUND_LITERAL is a CONSTRUCTOR that + initializes the compound literal. */ + +tree +build_compound_literal (tree type, tree compound_literal) +{ + /* We do not use start_decl here because we have a type, not a declarator; + and do not use finish_decl because the decl should be stored inside + the COMPOUND_LITERAL_EXPR rather than added elsewhere as a DECL_EXPR. */ + tree decl; + tree complit; + tree stmt; + + if (type == error_mark_node || compound_literal == error_mark_node) + return error_mark_node; + + /* If the TYPE was an array type with an unknown bound, then we can + figure out the dimension now. For example, something like: + + `(int []) { 2, 3 }' + + implies that the array has two elements. */ + if (!processing_template_decl + && TREE_CODE (type) == ARRAY_TYPE && !COMPLETE_TYPE_P (type)) + complete_array_type (type, compound_literal, 1); + + decl = build_local_temp (type); + DECL_INITIAL (decl) = compound_literal; + + stmt = build_stmt (DECL_EXPR, decl); + complit = build1 (COMPOUND_LITERAL_EXPR, TREE_TYPE (decl), stmt); + TREE_SIDE_EFFECTS (complit) = 1; + + return complit; +} static tree build_cplus_array_type_1 (tree elt_type, tree index_type) @@ -2241,6 +2279,8 @@ stabilize_init (tree init, tree *initp) t = TREE_OPERAND (t, 1); if (TREE_CODE (t) == TARGET_EXPR) t = TARGET_EXPR_INITIAL (t); + if (TREE_CODE (t) == COMPOUND_LITERAL_EXPR) + t = DECL_INITIAL (COMPOUND_LITERAL_EXPR_DECL (t)); if (TREE_CODE (t) == COMPOUND_EXPR) t = expr_last (t); if (TREE_CODE (t) == CONSTRUCTOR Index: gcc/cp/pt.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/pt.c,v retrieving revision 1.985 diff -u -p -r1.985 pt.c --- gcc/cp/pt.c 17 Mar 2005 13:26:39 -0000 1.985 +++ gcc/cp/pt.c 18 Mar 2005 05:30:23 -0000 @@ -8785,6 +8785,11 @@ tsubst_copy_and_build (tree t, return build_throw (RECUR (TREE_OPERAND (t, 0))); + case COMPOUND_LITERAL_EXPR: + return build_compound_literal + (RECUR (TREE_TYPE (COMPOUND_LITERAL_EXPR_DECL (t))), + RECUR (DECL_INITIAL (COMPOUND_LITERAL_EXPR_DECL (t)))); + case CONSTRUCTOR: { tree r; Index: gcc/cp/call.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v retrieving revision 1.531 diff -u -p -r1.531 call.c --- gcc/cp/call.c 24 Feb 2005 21:55:10 -0000 1.531 +++ gcc/cp/call.c 18 Mar 2005 05:30:27 -0000 @@ -4850,12 +4898,14 @@ build_over_call (struct z_candidate *can temp or an INIT_EXPR otherwise. */ if (integer_zerop (TREE_VALUE (args))) { - if (TREE_CODE (arg) == TARGET_EXPR) + if (TREE_CODE (arg) == TARGET_EXPR + || TREE_CODE (arg) == COMPOUND_LITERAL_EXPR) return arg; else if (TYPE_HAS_TRIVIAL_INIT_REF (DECL_CONTEXT (fn))) return build_target_expr_with_type (arg, DECL_CONTEXT (fn)); } else if (TREE_CODE (arg) == TARGET_EXPR + || TREE_CODE (arg) == COMPOUND_LITERAL_EXPR || TYPE_HAS_TRIVIAL_INIT_REF (DECL_CONTEXT (fn))) { tree to = stabilize_reference Index: gcc/cp/class.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v retrieving revision 1.709 diff -u -p -r1.709 class.c --- gcc/cp/class.c 7 Mar 2005 23:08:57 -0000 1.709 +++ gcc/cp/class.c 18 Mar 2005 05:30:32 -0000 @@ -5234,6 +5234,7 @@ fixed_type_or_null (tree instance, int* } /* fall through... */ case TARGET_EXPR: + case COMPOUND_LITERAL_EXPR: case PARM_DECL: case RESULT_DECL: if (IS_AGGR_TYPE (TREE_TYPE (instance))) Index: gcc/cp/cp-gimplify.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/cp-gimplify.c,v retrieving revision 1.17 diff -u -p -r1.17 cp-gimplify.c --- gcc/cp/cp-gimplify.c 22 Dec 2004 18:00:38 -0000 1.17 +++ gcc/cp/cp-gimplify.c 18 Mar 2005 05:30:32 -0000 @@ -1,6 +1,6 @@ /* C++-specific tree lowering bits; see also c-gimplify.c and tree-gimple.c. - Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc. + Copyright (C) 2002, 2003, 2004, 2005 Free Software Foundation, Inc. Contributed by Jason Merrill <[EMAIL PROTECTED]> This file is part of GCC. @@ -125,6 +125,8 @@ cp_gimplify_init_expr (tree *expr_p, tre from = TARGET_EXPR_INITIAL (from); if (TREE_CODE (from) == CLEANUP_POINT_EXPR) from = TREE_OPERAND (from, 0); + if (TREE_CODE (from) == COMPOUND_LITERAL_EXPR) + from = DECL_INITIAL (COMPOUND_LITERAL_EXPR_DECL (from)); /* Look through any COMPOUND_EXPRs. */ sub = expr_last (from); @@ -170,6 +172,32 @@ gimplify_must_not_throw_expr (tree *expr *expr_p = stmt; } +/* Gimplify a compound literal expression. This just means adding the + DECL_EXPR before the current EXPR_STMT and using its anonymous decl + instead, unless cleanups are needed. */ + +static enum gimplify_status +cp_gimplify_compound_literal_expr (tree *expr_p, tree *pre_p, tree *post_p) +{ + tree decl_s = COMPOUND_LITERAL_EXPR_DECL_STMT (*expr_p); + tree decl = DECL_EXPR_DECL (decl_s); + tree init = DECL_INITIAL (decl); + tree cleanup = cxx_maybe_build_cleanup (decl); + + if (cleanup) + { + DECL_INITIAL (decl) = NULL_TREE; + *expr_p = build4 (TARGET_EXPR, TREE_TYPE (decl), decl, + init, cleanup, NULL_TREE); + return GS_OK; + } + + /* If no cleanups are needed, we can do things in a simpler way. */ + gimplify_and_add (decl_s, pre_p); + *expr_p = decl; + return GS_ALL_DONE; +} + /* Do C++-specific gimplification. Args are as for gimplify_expr. */ int @@ -254,6 +282,10 @@ cp_gimplify_expr (tree *expr_p, tree *pr ret = GS_OK; break; + case COMPOUND_LITERAL_EXPR: + ret = cp_gimplify_compound_literal_expr (expr_p, pre_p, post_p); + break; + default: ret = c_gimplify_expr (expr_p, pre_p, post_p); break; Index: gcc/cp/cvt.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/cvt.c,v retrieving revision 1.180 diff -u -p -r1.180 cvt.c --- gcc/cp/cvt.c 9 Feb 2005 02:53:37 -0000 1.180 +++ gcc/cp/cvt.c 18 Mar 2005 05:30:33 -0000 @@ -575,7 +575,8 @@ convert_from_reference (tree val) tree force_rvalue (tree expr) { - if (IS_AGGR_TYPE (TREE_TYPE (expr)) && TREE_CODE (expr) != TARGET_EXPR) + if (IS_AGGR_TYPE (TREE_TYPE (expr)) && TREE_CODE (expr) != TARGET_EXPR + && TREE_CODE (expr) != COMPOUND_LITERAL_EXPR) expr = ocp_convert (TREE_TYPE (expr), expr, CONV_IMPLICIT|CONV_FORCE_TEMP, LOOKUP_NORMAL); else @@ -640,6 +641,16 @@ ocp_convert (tree type, tree expr, int c TREE_TYPE (e) = TREE_TYPE (TARGET_EXPR_SLOT (e)) = type; return e; } + else if (TREE_CODE (e) == COMPOUND_LITERAL_EXPR) + { + /* Don't build a NOP_EXPR of class type. Instead, change the + type of the temporary. Only allow this for cv-qual changes, + though. */ + gcc_assert (same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (e)), + TYPE_MAIN_VARIANT (type))); + TREE_TYPE (e) = TREE_TYPE (COMPOUND_LITERAL_EXPR_DECL (e)) = type; + return e; + } else { /* We shouldn't be treating objects of ADDRESSABLE type as Index: gcc/cp/typeck.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/cp/typeck.c,v retrieving revision 1.618 diff -u -p -r1.618 typeck.c --- gcc/cp/typeck.c 9 Mar 2005 07:28:10 -0000 1.618 +++ gcc/cp/typeck.c 18 Mar 2005 05:30:37 -0000 @@ -3574,7 +3574,8 @@ build_x_unary_op (enum tree_code code, t PTRMEM_OK_P (xarg) = ptrmem; } } - else if (TREE_CODE (xarg) == TARGET_EXPR) + else if (TREE_CODE (xarg) == TARGET_EXPR + || TREE_CODE (xarg) == COMPOUND_LITERAL_EXPR) warning ("taking address of temporary"); exp = build_unary_op (ADDR_EXPR, xarg, 0); } @@ -4307,7 +4308,12 @@ cxx_mark_addressable (tree exp) case TARGET_EXPR: TREE_ADDRESSABLE (x) = 1; - cxx_mark_addressable (TREE_OPERAND (x, 0)); + cxx_mark_addressable (TARGET_EXPR_SLOT (x)); + return true; + + case COMPOUND_LITERAL_EXPR: + TREE_ADDRESSABLE (x) = 1; + cxx_mark_addressable (COMPOUND_LITERAL_EXPR_DECL (x)); return true; default: @@ -5970,7 +5976,8 @@ maybe_warn_about_returning_address_of_lo if (TREE_CODE (valtype) == REFERENCE_TYPE) { if (TREE_CODE (whats_returned) == AGGR_INIT_EXPR - || TREE_CODE (whats_returned) == TARGET_EXPR) + || TREE_CODE (whats_returned) == TARGET_EXPR + || TREE_CODE (whats_returned) == COMPOUND_LITERAL_EXPR) { warning ("returning reference to temporary"); return; Index: gcc/testsuite/ChangeLog from Alexandre Oliva <[EMAIL PROTECTED]> PR c++/20103 * g++.dg/tree-ssa/pr20103.C: New test. * g++.dg/template/complit1.C: Expect error like ext/complit1.C. * g++.dg/ext/complit4.C: New test. Index: gcc/testsuite/g++.dg/tree-ssa/pr20103.C =================================================================== RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20103.C diff -N gcc/testsuite/g++.dg/tree-ssa/pr20103.C --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gcc/testsuite/g++.dg/tree-ssa/pr20103.C 18 Mar 2005 05:30:51 -0000 @@ -0,0 +1,59 @@ +// PR c++/20103 + +// { dg-do compile } + +// { dg-options "" } + +// Gimplification used to fail for (B){x}, because create_tmp_var +// required a non-addressable type, and we didn't wrap the constructor +// in a target_expr, ensuring it's moved to a separate decl. + +// Whether it is an lvalue, like in C, or an rvalue, is up to the ISO +// C++ Commitete to decide in the second version of the C++ Standard. +// We're going with rvalues for now. + +struct A +{ + A(const A&); +}; + +struct B +{ + A a; +}; + +void foo(B); +void bar(B&); // { dg-error "in passing argument" } +void bac(const B&); +void bap(const B*); + +void baz(A &x) +{ + foo ((B){x}); + bar ((B){x}); // { dg-error "non-const reference" } + bac ((B){x}); + bap (&(B){x}); // { dg-error "address of temporary" } + + foo ((const B&)(B){x}); + bar ((B&)(B){x}); // { dg-error "invalid cast" } + bac ((const B&)(B){x}); + bap (&(const B&)(B){x}); +} + +template <typename T, typename U> +void baz(T &x) +{ + foo ((U){x}); + bar ((U){x}); // { dg-error "non-const reference" } + bac ((U){x}); + bap (&(U){x}); // { dg-error "address of temporary" } + + foo ((const U&)(U){x}); + bar ((U&)(U){x}); // { dg-error "invalid cast" } + bac ((const U&)(U){x}); + bap (&(const U&)(U){x}); +} + +void bazT(A &x) { + baz<A,B>(x); +} Index: gcc/testsuite/g++.dg/template/complit1.C =================================================================== RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.dg/template/complit1.C,v retrieving revision 1.3 diff -u -p -r1.3 complit1.C --- gcc/testsuite/g++.dg/template/complit1.C 28 Dec 2002 07:48:08 -0000 1.3 +++ gcc/testsuite/g++.dg/template/complit1.C 18 Mar 2005 05:30:51 -0000 @@ -6,6 +6,6 @@ template <int D> struct C { }; template<int D> -C<D>::C() : d((int[]){1,2,3}) {} +C<D>::C() : d((int[]){1,2,3}) {} // { dg-error "assignment of arrays" } -template class C<1>; +template class C<1>; // { dg-error "instantiated from" } Index: gcc/testsuite/g++.dg/ext/complit4.C =================================================================== RCS file: gcc/testsuite/g++.dg/ext/complit4.C diff -N gcc/testsuite/g++.dg/ext/complit4.C --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gcc/testsuite/g++.dg/ext/complit4.C 18 Mar 2005 05:30:51 -0000 @@ -0,0 +1,44 @@ +// { dg-do run } +// { dg-options "" } + +// Make sure we issue dtor calls for t::x, and for the s temporary +// used to construct it, in normal and eh-cleanup control flows. + +#include <stdlib.h> + +static int s_dtor_count = 0; +static bool t_dtor_throws = false; + +struct s { + int i; + ~s() { + s_dtor_count++; + } +}; + +struct t { + s x; + ~t() { + if (t_dtor_throws) + throw s_dtor_count; + } +}; + +int main () { + s_dtor_count = 0; + (t){(s){1}}; + if (s_dtor_count != 2) + abort (); + + try { + t_dtor_throws = true; + s_dtor_count = 0; + (t){(s){1}}; + abort (); + } catch (int saved_s_dtor_count) { + if (s_dtor_count != 2) + abort (); + if (saved_s_dtor_count != 0) + abort (); + } +} -- Alexandre Oliva http://www.ic.unicamp.br/~oliva/ Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org} Free Software Evangelist [EMAIL PROTECTED], gnu.org} -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103