Hi!
On Tue, Feb 28, 2017 at 02:47:31PM -1000, Nathan Sidwell wrote:
> On 02/28/2017 02:41 PM, Jason Merrill wrote:
> > On Tue, Feb 28, 2017 at 12:48 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> > > The DR1659/DR1611 changes result in construct_virtual_base not being
> > > called,
> > > but unfortunately the call generated in there was the spot that caused
> > > mark_exp_read on the arguments passed to the vbase construction (TREE_USED
> > > is set on these earlier already during parsing them). That results
> > > in false positive -Wunused-but-set-parameter warnings.
> > >
> > > The following patch tries to avoid the warning in that case by marking
> > > the arguments as read (essentially pretending they were read in the
> > > omitted
> > > call). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > > trunk?
> >
> > I believe there's some question still about whether the DR1659/11
> > changes are what we want; I'll defer to Nathan on this patch.
>
> Jakub's patch is OK. The defect I've reported is how 1658 interacts with
> virtual destructors. (bug 79393)
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).
Plus, as the following testcase shows, mark_exp_read really expects to be
called on quite narrow sets of expressions that are being emitted, while
in arguments because it is not really emitted we can have e.g. nested
CONSTRUCTORs etc. and mark_exp_read doesn't handle those.
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).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-03-01 Jakub Jelinek <ja...@redhat.com>
PR c++/79782
* init.c (mark_exp_read_r): New function.
(emit_mem_initializers): Don't walk arguments if it is void_type_node.
Use cp_walk_tree with mark_exp_read_r instead of plain mark_exp_read.
* g++.dg/warn/Wunused-parm-10.C: New test.
--- gcc/cp/init.c.jj 2017-03-01 09:35:19.000000000 +0100
+++ gcc/cp/init.c 2017-03-01 16:59:09.014981469 +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
@@ -1217,12 +1228,12 @@ emit_mem_initializers (tree mem_inits)
/* C++14 DR1658 Means we do not have to construct vbases of
abstract classes. */
construct_virtual_base (subobject, arguments);
- else
+ else if (arguments != void_type_node)
/* 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 (&TREE_VALUE (arg), 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-01
17:02:41.811195793 +0100
+++ gcc/testsuite/g++.dg/warn/Wunused-parm-10.C 2017-03-01 17:01:31.000000000
+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
