This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C++ PATCH] -Wunused-but-set-parameter fix followup (PR c++/79782)
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 <jakub@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