On Wed, Mar 01, 2017 at 03:10:15PM -1000, Nathan Sidwell wrote: > On 03/01/2017 09:40 AM, Jakub Jelinek wrote: > > > Unfortunately my patch apparently broke the case where such ctor in virtual > > class has no arguments (void_type_node is used in that case instead of a > > TREE_LIST, it is a little bit weird (I'd have expected perhaps > > void_list_node instead), but it is what it does). > > Seems funky (but not your problem), which of the testcase(s) does it > correspond to?
That is ICE on the last line of: struct E { virtual E *foo () const = 0; }; struct F : virtual public E { }; struct G : public virtual F { G (int x) : F () { } }; where arguments == void_type_node: cp_parser_mem_initializer has: if (!expression_list) expression_list = void_type_node; but it is documented: Returns a TREE_LIST. The TREE_PURPOSE is the TYPE (for a base class) or FIELD_DECL (for a non-static data member) to initialize; the TREE_VALUE is the expression-list. An empty initialization list is represented by void_list_node. */ and e.g. pt.c has: if (TREE_VALUE (t) == void_type_node) /* VOID_TYPE_NODE is used to indicate value-initialization. */ { for (i = 0; i < len; i++) TREE_VEC_ELT (expanded_arguments, i) = void_type_node; } > > So this patch in addition to not walking anything for > > arguments == void_type_node > > just walks the arguments and calls mark_exp_read on all PARM_DECLs in there > > (I think that is all we care about, we can't have there VAR_DECLs or > > RESULT_DECLs). > > I suppose someone could pass in a global VAR_DECL, but if the ctor's the > only use of that decl, it's rather stupid. We don't track -Wunused-but-set-* for global VAR_DECLs, it is just for automatic vars and parameters. > Do you actually need to iterate over the arg list -- can't you just pass > ARGUMENTS straight into cp_walk_tree? You're right, that works too, and actually doesn't mind void_type_node either. So I'll retest: 2017-03-02 Jakub Jelinek <ja...@redhat.com> PR c++/79782 * init.c (mark_exp_read_r): New function. (emit_mem_initializers): Use cp_walk_tree with mark_exp_read_r on whole arguments instead of plain mark_exp_read on TREE_LIST values. * g++.dg/warn/Wunused-parm-10.C: New test. --- gcc/cp/init.c.jj 2017-03-02 08:08:42.736368162 +0100 +++ gcc/cp/init.c 2017-03-02 08:15:51.805770171 +0100 @@ -1127,6 +1127,17 @@ sort_mem_initializers (tree t, tree mem_ return sorted_inits; } +/* Callback for cp_walk_tree to mark all PARM_DECLs in a tree as read. */ + +static tree +mark_exp_read_r (tree *tp, int *, void *) +{ + tree t = *tp; + if (TREE_CODE (t) == PARM_DECL) + mark_exp_read (t); + return NULL_TREE; +} + /* Initialize all bases and members of CURRENT_CLASS_TYPE. MEM_INITS is a TREE_LIST giving the explicit mem-initializer-list for the constructor. The TREE_PURPOSE of each entry is a subobject (a @@ -1221,8 +1232,7 @@ emit_mem_initializers (tree mem_inits) /* When not constructing vbases of abstract classes, at least mark the arguments expressions as read to avoid -Wunused-but-set-parameter false positives. */ - for (tree arg = arguments; arg; arg = TREE_CHAIN (arg)) - mark_exp_read (TREE_VALUE (arg)); + cp_walk_tree (&arguments, mark_exp_read_r, NULL, NULL); if (inherited_base) pop_deferring_access_checks (); --- gcc/testsuite/g++.dg/warn/Wunused-parm-10.C.jj 2017-03-02 08:13:25.365661085 +0100 +++ gcc/testsuite/g++.dg/warn/Wunused-parm-10.C 2017-03-02 08:13:25.365661085 +0100 @@ -0,0 +1,12 @@ +// PR c++/79782 +// { dg-do compile { target c++11 } } +// { dg-options "-Wunused-but-set-parameter -Wunused-parameter" } + +struct E { virtual E *foo () const = 0; }; +struct F : virtual public E { }; +struct G : public virtual F { G (int x) : F () { } }; // { dg-warning "unused parameter" } +struct H : virtual public E { H (int x, int y); }; +struct I : public virtual H { I (int x, int y) : H (x, y) { } }; // { dg-bogus "set but not used" } +struct J : public virtual H { J (int x, int y) : H { x, y } { } }; // { dg-bogus "set but not used" } +struct K : public virtual H { K (int x, int y) : H (x * 0, y + 1) { } }; // { dg-bogus "set but not used" } +struct L : public virtual H { L (int x, int y) : H { x & 0, y | 1 } { } }; // { dg-bogus "set but not used" } Jakub