* C++ PATCH to -Wunused with C++17 structured bindings
@ 2017-05-24 2:23 Jason Merrill
2017-05-24 6:52 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2017-05-24 2:23 UTC (permalink / raw)
To: gcc-patches List; +Cc: Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 628 bytes --]
Someone on IRC complained that there was no way to suppress -Wunused
on structured bindings. It seemed to me that the way the feature
works, we shouldn't warn about the bindings individually; users need
to give each of the subobjects a name even if they're only interested
in using one of them.
So this patch switches to tracking whether the underlying aggregate
object as a whole is used; using one of the bindings will avoid any
warning.
This doesn't apply to tuple structured bindings, since in that case
the bindings are actual variables rather than aliases to subobjects.
Tested x86_64-pc-linux-gnu, applying to trunk.
[-- Attachment #2: sbind-unused.diff --]
[-- Type: text/plain, Size: 2648 bytes --]
commit a10b737bee6f269c6d6cf2a668d03fb322e1c45e
Author: Jason Merrill <jason@redhat.com>
Date: Thu May 11 13:30:24 2017 -0400
-Wunused and C++17 structured bindings
* decl.c (poplevel): Don't warn about unused structured bindings,
only real variables.
* error.c (dump_simple_decl): Handle structured bindings.
* expr.c (mark_exp_read): Look through DECL_VALUE_EXPR.
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 5877f37..afd47bb 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -656,7 +656,10 @@ poplevel (int keep, int reverse, int functionbody)
if (VAR_P (decl)
&& (! TREE_USED (decl) || !DECL_READ_P (decl))
&& ! DECL_IN_SYSTEM_HEADER (decl)
- && DECL_NAME (decl) && ! DECL_ARTIFICIAL (decl)
+ /* For structured bindings, consider only real variables, not
+ subobjects. */
+ && (DECL_DECOMPOSITION_P (decl) ? !DECL_VALUE_EXPR (decl)
+ : (DECL_NAME (decl) && !DECL_ARTIFICIAL (decl)))
&& type != error_mark_node
&& (!CLASS_TYPE_P (type)
|| !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index b65cee4..66a4b60 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -992,6 +992,8 @@ dump_simple_decl (cxx_pretty_printer *pp, tree t, tree type, int flags)
else
dump_decl (pp, DECL_NAME (t), flags);
}
+ else if (DECL_DECOMPOSITION_P (t))
+ pp_string (pp, M_("<structured bindings>"));
else
pp_string (pp, M_("<anonymous>"));
if (flags & TFF_DECL_SPECIFIERS)
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 77af54e..75e99e5 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -133,6 +133,9 @@ mark_exp_read (tree exp)
switch (TREE_CODE (exp))
{
case VAR_DECL:
+ if (DECL_VALUE_EXPR (exp))
+ mark_exp_read (DECL_VALUE_EXPR (exp));
+ gcc_fallthrough ();
case PARM_DECL:
DECL_READ_P (exp) = 1;
break;
diff --git a/gcc/testsuite/g++.dg/cpp1z/decomp29.C b/gcc/testsuite/g++.dg/cpp1z/decomp29.C
new file mode 100644
index 0000000..daf07a0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/decomp29.C
@@ -0,0 +1,26 @@
+// { dg-options "-std=c++17 -Wunused" }
+
+#include <tuple>
+
+struct A { int i,j,k; };
+
+A f();
+
+int z;
+
+int main()
+{
+ {
+ auto [i,j,k] = f(); // { dg-warning "unused" }
+ }
+ {
+ auto [i,j,k] = f();
+ z = i;
+ }
+ {
+ auto [i,j] = std::tuple{1,2}; // { dg-warning "unused" }
+ }
+ // No parallel second test, because in this case i and j are variables rather
+ // than mere bindings, so there isn't a link between them and using i will
+ // not prevent a warning about unused j.
+}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: C++ PATCH to -Wunused with C++17 structured bindings
2017-05-24 2:23 C++ PATCH to -Wunused with C++17 structured bindings Jason Merrill
@ 2017-05-24 6:52 ` Jakub Jelinek
2017-05-24 16:19 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2017-05-24 6:52 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Tue, May 23, 2017 at 09:45:10PM -0400, Jason Merrill wrote:
> Someone on IRC complained that there was no way to suppress -Wunused
> on structured bindings. It seemed to me that the way the feature
> works, we shouldn't warn about the bindings individually; users need
> to give each of the subobjects a name even if they're only interested
> in using one of them.
>
> So this patch switches to tracking whether the underlying aggregate
> object as a whole is used; using one of the bindings will avoid any
> warning.
>
> This doesn't apply to tuple structured bindings, since in that case
> the bindings are actual variables rather than aliases to subobjects.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
So, shall we have even for the tuple structured bindings somewhere (in
lang_decl, such as adding struct lang_decl_decomp that would include
lang_decl_min?) the tree for the underlying artificial var decl?
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: C++ PATCH to -Wunused with C++17 structured bindings
2017-05-24 6:52 ` Jakub Jelinek
@ 2017-05-24 16:19 ` Jason Merrill
2017-05-25 14:23 ` [C++ PATCH] Fix " Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2017-05-24 16:19 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
On Wed, May 24, 2017 at 2:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, May 23, 2017 at 09:45:10PM -0400, Jason Merrill wrote:
>> Someone on IRC complained that there was no way to suppress -Wunused
>> on structured bindings. It seemed to me that the way the feature
>> works, we shouldn't warn about the bindings individually; users need
>> to give each of the subobjects a name even if they're only interested
>> in using one of them.
>>
>> So this patch switches to tracking whether the underlying aggregate
>> object as a whole is used; using one of the bindings will avoid any
>> warning.
>>
>> This doesn't apply to tuple structured bindings, since in that case
>> the bindings are actual variables rather than aliases to subobjects.
>
> So, shall we have even for the tuple structured bindings somewhere (in
> lang_decl, such as adding struct lang_decl_decomp that would include
> lang_decl_min?) the tree for the underlying artificial var decl?
That would make sense.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* [C++ PATCH] Fix -Wunused with C++17 structured bindings
2017-05-24 16:19 ` Jason Merrill
@ 2017-05-25 14:23 ` Jakub Jelinek
2017-05-25 19:08 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2017-05-25 14:23 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Wed, May 24, 2017 at 12:13:40PM -0400, Jason Merrill wrote:
> On Wed, May 24, 2017 at 2:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Tue, May 23, 2017 at 09:45:10PM -0400, Jason Merrill wrote:
> >> Someone on IRC complained that there was no way to suppress -Wunused
> >> on structured bindings. It seemed to me that the way the feature
> >> works, we shouldn't warn about the bindings individually; users need
> >> to give each of the subobjects a name even if they're only interested
> >> in using one of them.
> >>
> >> So this patch switches to tracking whether the underlying aggregate
> >> object as a whole is used; using one of the bindings will avoid any
> >> warning.
> >>
> >> This doesn't apply to tuple structured bindings, since in that case
> >> the bindings are actual variables rather than aliases to subobjects.
> >
> > So, shall we have even for the tuple structured bindings somewhere (in
> > lang_decl, such as adding struct lang_decl_decomp that would include
> > lang_decl_min?) the tree for the underlying artificial var decl?
>
> That would make sense.
So like this (so far lightly tested)? As DECL_DECOMPOSITION_P bit
sits in DECL_LANG_SPECIFIC, it is not possible to set that bit first and
then retrofit_lang_decl, so I had to play some games with that.
There is one issue I've noticed in your earlier patch (fixed in this patch),
we have separate DECL_HAS_VALUE_EXPR_P bit and DECL_VALUE_EXPR, testing
the former is cheap, the latter is an expensive function call.
So
if (DECL_VALUE_EXPR (exp))
mark_exp_read (DECL_VALUE_EXPR (exp));
is something unnecessarily expensive for all vars, while
if (DECL_HAS_VALUE_EXPR_P (exp))
mark_exp_read (DECL_VALUE_EXPR (exp));
or
if (DECL_HAS_VALUE_EXPR_P (exp))
{
tree t = DECL_VALUE_EXPR (exp);
if (t)
mark_exp_read (t);
}
is much cheaper. I believe usually DECL_VALUE_EXPR should be non-NULL
if DECL_HAS_VALUE_EXPR_P, but am not 100% sure if it is always guaranteed
(but pretty sure lots of code would break if it is not).
2017-05-25 Jakub Jelinek <jakub@redhat.com>
* cp-tree.h (struct lang_decl_decomp): New type.
(struct lang_decl): Add u.decomp.
(LANG_DECL_DECOMP_CHECK): Define.
(DECL_DECOMPOSITION_P): Note it is set also on the vars
for user identifiers.
(DECL_DECOMP_BASE): Define.
(retrofit_lang_decl): Add extra int = 0 argument.
* lex.c (retrofit_lang_decl): Add SEL argument, if non-zero
use it to influence the selector choices and for selector
0 to non-zero transition copy old content.
(cxx_dup_lang_specific_decl): Handle DECL_DECOMPOSITION_P.
* decl.c (poplevel): For DECL_DECOMPOSITION_P, check
!DECL_DECOMP_BASE instead of !DECL_VALUE_EXPR. Adjust warning
wording if decl is a structured binding.
(cp_finish_decomp): Pass 4 as the new argument to retrofit_lang_decl.
Set DECL_DECOMP_BASE. Ignore DECL_READ_P sets from initialization
of individual variables for tuple structured bindings.
(grokdeclarator): Pass 4 as the new argument to retrofit_lang_decl.
Clear DECL_DECOMP_BASE.
* decl2.c (mark_used): Mark DECL_DECOMP_BASE TREE_USED as well.
* pt.c (tsubst_decomp_names): Assert DECL_DECOMP_BASE matches what
is expected.
* expr.c (mark_exp_read): Recurse on DECL_DECOMP_BASE instead of
DECL_VALUE_EXPR.
* g++.dg/cpp1z/decomp29.C (p): New variable.
(main): Add further tests.
--- gcc/cp/cp-tree.h.jj 2017-05-25 10:37:00.000000000 +0200
+++ gcc/cp/cp-tree.h 2017-05-25 12:48:34.828167911 +0200
@@ -2516,6 +2516,15 @@ struct GTY(()) lang_decl_parm {
int index;
};
+/* Additional DECL_LANG_SPECIFIC information for structured bindings. */
+
+struct GTY(()) lang_decl_decomp {
+ struct lang_decl_min min;
+ /* The artificial underlying "e" variable of the structured binding
+ variable. */
+ tree base;
+};
+
/* DECL_LANG_SPECIFIC for all types. It would be nice to just make this a
union rather than a struct containing a union as its only field, but
tree.h declares it as a struct. */
@@ -2527,6 +2536,7 @@ struct GTY(()) lang_decl {
struct lang_decl_fn GTY ((tag ("1"))) fn;
struct lang_decl_ns GTY((tag ("2"))) ns;
struct lang_decl_parm GTY((tag ("3"))) parm;
+ struct lang_decl_decomp GTY((tag ("4"))) decomp;
} u;
};
@@ -2563,6 +2573,13 @@ struct GTY(()) lang_decl {
lang_check_failed (__FILE__, __LINE__, __FUNCTION__); \
<->u.parm; })
+#define LANG_DECL_DECOMP_CHECK(NODE) __extension__ \
+({ struct lang_decl *lt = DECL_LANG_SPECIFIC (NODE); \
+ if (!VAR_P (NODE) \
+ || lt->u.base.selector != 4) \
+ lang_check_failed (__FILE__, __LINE__, __FUNCTION__); \
+ <->u.decomp; })
+
#define LANG_DECL_U2_CHECK(NODE, TF) __extension__ \
({ struct lang_decl *lt = DECL_LANG_SPECIFIC (NODE); \
if (!LANG_DECL_HAS_MIN (NODE) || lt->u.base.u2sel != TF) \
@@ -2583,6 +2600,9 @@ struct GTY(()) lang_decl {
#define LANG_DECL_PARM_CHECK(NODE) \
(&DECL_LANG_SPECIFIC (NODE)->u.parm)
+#define LANG_DECL_DECOMP_CHECK(NODE) \
+ (&DECL_LANG_SPECIFIC (NODE)->u.decomp)
+
#define LANG_DECL_U2_CHECK(NODE, TF) \
(&DECL_LANG_SPECIFIC (NODE)->u.min.u2)
@@ -3829,8 +3849,8 @@ more_aggr_init_expr_args_p (const aggr_i
(DECL_LANG_SPECIFIC (VAR_DECL_CHECK (NODE))->u.base.var_declared_inline_p \
= true)
-/* Nonzero if NODE is an artificial VAR_DECL for a C++17 decomposition
- declaration. */
+/* Nonzero if NODE is an artificial VAR_DECL for a C++17 structured binding
+ declaration or one of VAR_DECLs for the user identifiers in it. */
#define DECL_DECOMPOSITION_P(NODE) \
(VAR_P (NODE) && DECL_LANG_SPECIFIC (NODE) \
? DECL_LANG_SPECIFIC (NODE)->u.base.decomposition_p \
@@ -3839,6 +3859,10 @@ more_aggr_init_expr_args_p (const aggr_i
(DECL_LANG_SPECIFIC (VAR_DECL_CHECK (NODE))->u.base.decomposition_p \
= true)
+/* The underlying artificial VAR_DECL for structured binding. */
+#define DECL_DECOMP_BASE(NODE) \
+ (LANG_DECL_DECOMP_CHECK (NODE)->base)
+
/* Nonzero if NODE is an inline VAR_DECL. In C++17, static data members
declared with constexpr specifier are implicitly inline variables. */
#define DECL_INLINE_VAR_P(NODE) \
@@ -6274,7 +6298,7 @@ extern tree unqualified_name_lookup_erro
extern tree unqualified_fn_lookup_error (cp_expr);
extern tree build_lang_decl (enum tree_code, tree, tree);
extern tree build_lang_decl_loc (location_t, enum tree_code, tree, tree);
-extern void retrofit_lang_decl (tree);
+extern void retrofit_lang_decl (tree, int = 0);
extern tree copy_decl (tree CXX_MEM_STAT_INFO);
extern tree copy_type (tree CXX_MEM_STAT_INFO);
extern tree cxx_make_type (enum tree_code);
--- gcc/cp/lex.c.jj 2017-05-11 09:42:27.000000000 +0200
+++ gcc/cp/lex.c 2017-05-25 15:01:01.362356364 +0200
@@ -529,16 +529,28 @@ build_lang_decl_loc (location_t loc, enu
and pushdecl (for functions generated by the back end). */
void
-retrofit_lang_decl (tree t)
+retrofit_lang_decl (tree t, int sel)
{
struct lang_decl *ld;
size_t size;
- int sel;
+ size_t oldsize = 0;
if (DECL_LANG_SPECIFIC (t))
- return;
+ {
+ if (sel)
+ {
+ if (DECL_LANG_SPECIFIC (t)->u.base.selector == sel)
+ return;
+ gcc_assert (DECL_LANG_SPECIFIC (t)->u.base.selector == 0);
+ oldsize = sizeof (struct lang_decl_min);
+ }
+ else
+ return;
+ }
- if (TREE_CODE (t) == FUNCTION_DECL)
+ if (sel == 4)
+ size = sizeof (struct lang_decl_decomp);
+ else if (TREE_CODE (t) == FUNCTION_DECL)
sel = 1, size = sizeof (struct lang_decl_fn);
else if (TREE_CODE (t) == NAMESPACE_DECL)
sel = 2, size = sizeof (struct lang_decl_ns);
@@ -550,6 +562,8 @@ retrofit_lang_decl (tree t)
gcc_unreachable ();
ld = (struct lang_decl *) ggc_internal_cleared_alloc (size);
+ if (oldsize)
+ memcpy (ld, DECL_LANG_SPECIFIC (t), oldsize);
ld->u.base.selector = sel;
@@ -584,6 +598,8 @@ cxx_dup_lang_specific_decl (tree node)
size = sizeof (struct lang_decl_ns);
else if (TREE_CODE (node) == PARM_DECL)
size = sizeof (struct lang_decl_parm);
+ else if (DECL_DECOMPOSITION_P (node))
+ size = sizeof (struct lang_decl_decomp);
else if (LANG_DECL_HAS_MIN (node))
size = sizeof (struct lang_decl_min);
else
--- gcc/cp/decl.c.jj 2017-05-25 10:37:00.000000000 +0200
+++ gcc/cp/decl.c 2017-05-25 14:35:54.600274460 +0200
@@ -658,7 +658,7 @@ poplevel (int keep, int reverse, int fun
&& ! DECL_IN_SYSTEM_HEADER (decl)
/* For structured bindings, consider only real variables, not
subobjects. */
- && (DECL_DECOMPOSITION_P (decl) ? !DECL_VALUE_EXPR (decl)
+ && (DECL_DECOMPOSITION_P (decl) ? !DECL_DECOMP_BASE (decl)
: (DECL_NAME (decl) && !DECL_ARTIFICIAL (decl)))
&& type != error_mark_node
&& (!CLASS_TYPE_P (type)
@@ -667,16 +667,28 @@ poplevel (int keep, int reverse, int fun
TYPE_ATTRIBUTES (TREE_TYPE (decl)))))
{
if (! TREE_USED (decl))
- warning_at (DECL_SOURCE_LOCATION (decl),
- OPT_Wunused_variable, "unused variable %qD", decl);
+ {
+ if (!DECL_NAME (decl) && DECL_DECOMPOSITION_P (decl))
+ warning_at (DECL_SOURCE_LOCATION (decl),
+ OPT_Wunused_variable,
+ "unused structured binding declaration");
+ else
+ warning_at (DECL_SOURCE_LOCATION (decl),
+ OPT_Wunused_variable, "unused variable %qD", decl);
+ }
else if (DECL_CONTEXT (decl) == current_function_decl
// For -Wunused-but-set-variable leave references alone.
&& TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE
&& errorcount == unused_but_set_errorcount)
{
- warning_at (DECL_SOURCE_LOCATION (decl),
- OPT_Wunused_but_set_variable,
- "variable %qD set but not used", decl);
+ if (!DECL_NAME (decl) && DECL_DECOMPOSITION_P (decl))
+ warning_at (DECL_SOURCE_LOCATION (decl),
+ OPT_Wunused_but_set_variable, "structured "
+ "binding declaration set but not used");
+ else
+ warning_at (DECL_SOURCE_LOCATION (decl),
+ OPT_Wunused_but_set_variable,
+ "variable %qD set but not used", decl);
unused_but_set_errorcount = errorcount;
}
}
@@ -7361,8 +7373,9 @@ cp_finish_decomp (tree decl, tree first,
}
if (processing_template_decl)
{
- retrofit_lang_decl (first);
+ retrofit_lang_decl (first, 4);
SET_DECL_DECOMPOSITION_P (first);
+ DECL_DECOMP_BASE (first) = decl;
}
first = DECL_CHAIN (first);
}
@@ -7375,8 +7388,9 @@ cp_finish_decomp (tree decl, tree first,
for (unsigned int i = 0; i < count; i++, d = DECL_CHAIN (d))
{
v[count - i - 1] = d;
- retrofit_lang_decl (d);
+ retrofit_lang_decl (d, 4);
SET_DECL_DECOMPOSITION_P (d);
+ DECL_DECOMP_BASE (d) = decl;
}
tree type = TREE_TYPE (decl);
@@ -7482,6 +7496,7 @@ cp_finish_decomp (tree decl, tree first,
eltscnt = tree_to_uhwi (tsize);
if (count != eltscnt)
goto cnt_mismatch;
+ int save_read = DECL_READ_P (decl);
for (unsigned i = 0; i < count; ++i)
{
location_t sloc = input_location;
@@ -7514,6 +7529,10 @@ cp_finish_decomp (tree decl, tree first,
cp_finish_decl (v[i], init, /*constexpr*/false,
/*asm*/NULL_TREE, LOOKUP_NORMAL);
}
+ /* Ignore reads from the underlying decl performed during initialization
+ of the individual variables. If those will be read, we'll mark
+ the underlying decl as read at that point. */
+ DECL_READ_P (decl) = save_read;
}
else if (TREE_CODE (type) == UNION_TYPE)
{
@@ -12295,9 +12314,10 @@ grokdeclarator (const cp_declarator *dec
{
gcc_assert (declarator && declarator->kind == cdk_decomp);
DECL_SOURCE_LOCATION (decl) = declarator->id_loc;
- retrofit_lang_decl (decl);
+ retrofit_lang_decl (decl, 4);
DECL_ARTIFICIAL (decl) = 1;
SET_DECL_DECOMPOSITION_P (decl);
+ DECL_DECOMP_BASE (decl) = NULL_TREE;
}
}
--- gcc/cp/decl2.c.jj 2017-05-24 11:59:02.000000000 +0200
+++ gcc/cp/decl2.c 2017-05-25 14:43:01.216917214 +0200
@@ -5027,6 +5027,9 @@ mark_used (tree decl, tsubst_flags_t com
/* Set TREE_USED for the benefit of -Wunused. */
TREE_USED (decl) = 1;
+ /* And for structured bindings also the underlying decl. */
+ if (DECL_DECOMPOSITION_P (decl) && DECL_DECOMP_BASE (decl))
+ TREE_USED (DECL_DECOMP_BASE (decl)) = 1;
if (TREE_CODE (decl) == TEMPLATE_DECL)
return true;
--- gcc/cp/pt.c.jj 2017-05-24 11:59:02.000000000 +0200
+++ gcc/cp/pt.c 2017-05-25 13:11:03.326234338 +0200
@@ -15705,6 +15705,7 @@ tsubst_decomp_names (tree decl, tree pat
return error_mark_node;
}
(*cnt)++;
+ gcc_assert (DECL_DECOMP_BASE (decl2) == pattern_decl);
gcc_assert (DECL_HAS_VALUE_EXPR_P (decl2));
tree v = DECL_VALUE_EXPR (decl2);
DECL_HAS_VALUE_EXPR_P (decl2) = 0;
--- gcc/cp/expr.c.jj 2017-05-24 11:59:02.000000000 +0200
+++ gcc/cp/expr.c 2017-05-25 13:07:37.304821510 +0200
@@ -133,8 +133,8 @@ mark_exp_read (tree exp)
switch (TREE_CODE (exp))
{
case VAR_DECL:
- if (DECL_VALUE_EXPR (exp))
- mark_exp_read (DECL_VALUE_EXPR (exp));
+ if (DECL_DECOMPOSITION_P (exp))
+ mark_exp_read (DECL_DECOMP_BASE (exp));
gcc_fallthrough ();
case PARM_DECL:
DECL_READ_P (exp) = 1;
--- gcc/testsuite/g++.dg/cpp1z/decomp29.C.jj 2017-05-24 11:59:01.000000000 +0200
+++ gcc/testsuite/g++.dg/cpp1z/decomp29.C 2017-05-25 14:47:59.000000000 +0200
@@ -5,6 +5,7 @@
struct A { int i,j,k; };
A f();
+int p[3];
int z;
@@ -14,13 +15,42 @@ int main()
auto [i,j,k] = f(); // { dg-warning "unused" }
}
{
+ [[maybe_unused]] auto [i,j,k] = f();
+ }
+ {
auto [i,j,k] = f();
z = i;
}
{
+ auto [i,j,k] = f(); // { dg-warning "unused" }
+ i = 5;
+ }
+ {
auto [i,j] = std::tuple{1,2}; // { dg-warning "unused" }
}
- // No parallel second test, because in this case i and j are variables rather
- // than mere bindings, so there isn't a link between them and using i will
- // not prevent a warning about unused j.
+ {
+ [[maybe_unused]] auto [i,j] = std::tuple{1,2};
+ }
+ {
+ auto [i,j] = std::tuple{1,2};
+ z = i;
+ }
+ {
+ auto [i,j] = std::tuple{1,2};
+ i = 5;
+ }
+ {
+ auto [i,j,k] = p; // { dg-warning "unused" }
+ }
+ {
+ [[maybe_unused]] auto [i,j,k] = p;
+ }
+ {
+ auto [i,j,k] = p;
+ z = i;
+ }
+ {
+ auto [i,j,k] = p; // { dg-warning "unused" }
+ i = 5;
+ }
}
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ PATCH] Fix -Wunused with C++17 structured bindings
2017-05-25 14:23 ` [C++ PATCH] Fix " Jakub Jelinek
@ 2017-05-25 19:08 ` Jason Merrill
0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2017-05-25 19:08 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
OK, thanks.
On Thu, May 25, 2017 at 10:22 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, May 24, 2017 at 12:13:40PM -0400, Jason Merrill wrote:
>> On Wed, May 24, 2017 at 2:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Tue, May 23, 2017 at 09:45:10PM -0400, Jason Merrill wrote:
>> >> Someone on IRC complained that there was no way to suppress -Wunused
>> >> on structured bindings. It seemed to me that the way the feature
>> >> works, we shouldn't warn about the bindings individually; users need
>> >> to give each of the subobjects a name even if they're only interested
>> >> in using one of them.
>> >>
>> >> So this patch switches to tracking whether the underlying aggregate
>> >> object as a whole is used; using one of the bindings will avoid any
>> >> warning.
>> >>
>> >> This doesn't apply to tuple structured bindings, since in that case
>> >> the bindings are actual variables rather than aliases to subobjects.
>> >
>> > So, shall we have even for the tuple structured bindings somewhere (in
>> > lang_decl, such as adding struct lang_decl_decomp that would include
>> > lang_decl_min?) the tree for the underlying artificial var decl?
>>
>> That would make sense.
>
> So like this (so far lightly tested)? As DECL_DECOMPOSITION_P bit
> sits in DECL_LANG_SPECIFIC, it is not possible to set that bit first and
> then retrofit_lang_decl, so I had to play some games with that.
>
> There is one issue I've noticed in your earlier patch (fixed in this patch),
> we have separate DECL_HAS_VALUE_EXPR_P bit and DECL_VALUE_EXPR, testing
> the former is cheap, the latter is an expensive function call.
> So
> if (DECL_VALUE_EXPR (exp))
> mark_exp_read (DECL_VALUE_EXPR (exp));
> is something unnecessarily expensive for all vars, while
> if (DECL_HAS_VALUE_EXPR_P (exp))
> mark_exp_read (DECL_VALUE_EXPR (exp));
> or
> if (DECL_HAS_VALUE_EXPR_P (exp))
> {
> tree t = DECL_VALUE_EXPR (exp);
> if (t)
> mark_exp_read (t);
> }
> is much cheaper. I believe usually DECL_VALUE_EXPR should be non-NULL
> if DECL_HAS_VALUE_EXPR_P, but am not 100% sure if it is always guaranteed
> (but pretty sure lots of code would break if it is not).
>
> 2017-05-25 Jakub Jelinek <jakub@redhat.com>
>
> * cp-tree.h (struct lang_decl_decomp): New type.
> (struct lang_decl): Add u.decomp.
> (LANG_DECL_DECOMP_CHECK): Define.
> (DECL_DECOMPOSITION_P): Note it is set also on the vars
> for user identifiers.
> (DECL_DECOMP_BASE): Define.
> (retrofit_lang_decl): Add extra int = 0 argument.
> * lex.c (retrofit_lang_decl): Add SEL argument, if non-zero
> use it to influence the selector choices and for selector
> 0 to non-zero transition copy old content.
> (cxx_dup_lang_specific_decl): Handle DECL_DECOMPOSITION_P.
> * decl.c (poplevel): For DECL_DECOMPOSITION_P, check
> !DECL_DECOMP_BASE instead of !DECL_VALUE_EXPR. Adjust warning
> wording if decl is a structured binding.
> (cp_finish_decomp): Pass 4 as the new argument to retrofit_lang_decl.
> Set DECL_DECOMP_BASE. Ignore DECL_READ_P sets from initialization
> of individual variables for tuple structured bindings.
> (grokdeclarator): Pass 4 as the new argument to retrofit_lang_decl.
> Clear DECL_DECOMP_BASE.
> * decl2.c (mark_used): Mark DECL_DECOMP_BASE TREE_USED as well.
> * pt.c (tsubst_decomp_names): Assert DECL_DECOMP_BASE matches what
> is expected.
> * expr.c (mark_exp_read): Recurse on DECL_DECOMP_BASE instead of
> DECL_VALUE_EXPR.
>
> * g++.dg/cpp1z/decomp29.C (p): New variable.
> (main): Add further tests.
>
> --- gcc/cp/cp-tree.h.jj 2017-05-25 10:37:00.000000000 +0200
> +++ gcc/cp/cp-tree.h 2017-05-25 12:48:34.828167911 +0200
> @@ -2516,6 +2516,15 @@ struct GTY(()) lang_decl_parm {
> int index;
> };
>
> +/* Additional DECL_LANG_SPECIFIC information for structured bindings. */
> +
> +struct GTY(()) lang_decl_decomp {
> + struct lang_decl_min min;
> + /* The artificial underlying "e" variable of the structured binding
> + variable. */
> + tree base;
> +};
> +
> /* DECL_LANG_SPECIFIC for all types. It would be nice to just make this a
> union rather than a struct containing a union as its only field, but
> tree.h declares it as a struct. */
> @@ -2527,6 +2536,7 @@ struct GTY(()) lang_decl {
> struct lang_decl_fn GTY ((tag ("1"))) fn;
> struct lang_decl_ns GTY((tag ("2"))) ns;
> struct lang_decl_parm GTY((tag ("3"))) parm;
> + struct lang_decl_decomp GTY((tag ("4"))) decomp;
> } u;
> };
>
> @@ -2563,6 +2573,13 @@ struct GTY(()) lang_decl {
> lang_check_failed (__FILE__, __LINE__, __FUNCTION__); \
> <->u.parm; })
>
> +#define LANG_DECL_DECOMP_CHECK(NODE) __extension__ \
> +({ struct lang_decl *lt = DECL_LANG_SPECIFIC (NODE); \
> + if (!VAR_P (NODE) \
> + || lt->u.base.selector != 4) \
> + lang_check_failed (__FILE__, __LINE__, __FUNCTION__); \
> + <->u.decomp; })
> +
> #define LANG_DECL_U2_CHECK(NODE, TF) __extension__ \
> ({ struct lang_decl *lt = DECL_LANG_SPECIFIC (NODE); \
> if (!LANG_DECL_HAS_MIN (NODE) || lt->u.base.u2sel != TF) \
> @@ -2583,6 +2600,9 @@ struct GTY(()) lang_decl {
> #define LANG_DECL_PARM_CHECK(NODE) \
> (&DECL_LANG_SPECIFIC (NODE)->u.parm)
>
> +#define LANG_DECL_DECOMP_CHECK(NODE) \
> + (&DECL_LANG_SPECIFIC (NODE)->u.decomp)
> +
> #define LANG_DECL_U2_CHECK(NODE, TF) \
> (&DECL_LANG_SPECIFIC (NODE)->u.min.u2)
>
> @@ -3829,8 +3849,8 @@ more_aggr_init_expr_args_p (const aggr_i
> (DECL_LANG_SPECIFIC (VAR_DECL_CHECK (NODE))->u.base.var_declared_inline_p \
> = true)
>
> -/* Nonzero if NODE is an artificial VAR_DECL for a C++17 decomposition
> - declaration. */
> +/* Nonzero if NODE is an artificial VAR_DECL for a C++17 structured binding
> + declaration or one of VAR_DECLs for the user identifiers in it. */
> #define DECL_DECOMPOSITION_P(NODE) \
> (VAR_P (NODE) && DECL_LANG_SPECIFIC (NODE) \
> ? DECL_LANG_SPECIFIC (NODE)->u.base.decomposition_p \
> @@ -3839,6 +3859,10 @@ more_aggr_init_expr_args_p (const aggr_i
> (DECL_LANG_SPECIFIC (VAR_DECL_CHECK (NODE))->u.base.decomposition_p \
> = true)
>
> +/* The underlying artificial VAR_DECL for structured binding. */
> +#define DECL_DECOMP_BASE(NODE) \
> + (LANG_DECL_DECOMP_CHECK (NODE)->base)
> +
> /* Nonzero if NODE is an inline VAR_DECL. In C++17, static data members
> declared with constexpr specifier are implicitly inline variables. */
> #define DECL_INLINE_VAR_P(NODE) \
> @@ -6274,7 +6298,7 @@ extern tree unqualified_name_lookup_erro
> extern tree unqualified_fn_lookup_error (cp_expr);
> extern tree build_lang_decl (enum tree_code, tree, tree);
> extern tree build_lang_decl_loc (location_t, enum tree_code, tree, tree);
> -extern void retrofit_lang_decl (tree);
> +extern void retrofit_lang_decl (tree, int = 0);
> extern tree copy_decl (tree CXX_MEM_STAT_INFO);
> extern tree copy_type (tree CXX_MEM_STAT_INFO);
> extern tree cxx_make_type (enum tree_code);
> --- gcc/cp/lex.c.jj 2017-05-11 09:42:27.000000000 +0200
> +++ gcc/cp/lex.c 2017-05-25 15:01:01.362356364 +0200
> @@ -529,16 +529,28 @@ build_lang_decl_loc (location_t loc, enu
> and pushdecl (for functions generated by the back end). */
>
> void
> -retrofit_lang_decl (tree t)
> +retrofit_lang_decl (tree t, int sel)
> {
> struct lang_decl *ld;
> size_t size;
> - int sel;
> + size_t oldsize = 0;
>
> if (DECL_LANG_SPECIFIC (t))
> - return;
> + {
> + if (sel)
> + {
> + if (DECL_LANG_SPECIFIC (t)->u.base.selector == sel)
> + return;
> + gcc_assert (DECL_LANG_SPECIFIC (t)->u.base.selector == 0);
> + oldsize = sizeof (struct lang_decl_min);
> + }
> + else
> + return;
> + }
>
> - if (TREE_CODE (t) == FUNCTION_DECL)
> + if (sel == 4)
> + size = sizeof (struct lang_decl_decomp);
> + else if (TREE_CODE (t) == FUNCTION_DECL)
> sel = 1, size = sizeof (struct lang_decl_fn);
> else if (TREE_CODE (t) == NAMESPACE_DECL)
> sel = 2, size = sizeof (struct lang_decl_ns);
> @@ -550,6 +562,8 @@ retrofit_lang_decl (tree t)
> gcc_unreachable ();
>
> ld = (struct lang_decl *) ggc_internal_cleared_alloc (size);
> + if (oldsize)
> + memcpy (ld, DECL_LANG_SPECIFIC (t), oldsize);
>
> ld->u.base.selector = sel;
>
> @@ -584,6 +598,8 @@ cxx_dup_lang_specific_decl (tree node)
> size = sizeof (struct lang_decl_ns);
> else if (TREE_CODE (node) == PARM_DECL)
> size = sizeof (struct lang_decl_parm);
> + else if (DECL_DECOMPOSITION_P (node))
> + size = sizeof (struct lang_decl_decomp);
> else if (LANG_DECL_HAS_MIN (node))
> size = sizeof (struct lang_decl_min);
> else
> --- gcc/cp/decl.c.jj 2017-05-25 10:37:00.000000000 +0200
> +++ gcc/cp/decl.c 2017-05-25 14:35:54.600274460 +0200
> @@ -658,7 +658,7 @@ poplevel (int keep, int reverse, int fun
> && ! DECL_IN_SYSTEM_HEADER (decl)
> /* For structured bindings, consider only real variables, not
> subobjects. */
> - && (DECL_DECOMPOSITION_P (decl) ? !DECL_VALUE_EXPR (decl)
> + && (DECL_DECOMPOSITION_P (decl) ? !DECL_DECOMP_BASE (decl)
> : (DECL_NAME (decl) && !DECL_ARTIFICIAL (decl)))
> && type != error_mark_node
> && (!CLASS_TYPE_P (type)
> @@ -667,16 +667,28 @@ poplevel (int keep, int reverse, int fun
> TYPE_ATTRIBUTES (TREE_TYPE (decl)))))
> {
> if (! TREE_USED (decl))
> - warning_at (DECL_SOURCE_LOCATION (decl),
> - OPT_Wunused_variable, "unused variable %qD", decl);
> + {
> + if (!DECL_NAME (decl) && DECL_DECOMPOSITION_P (decl))
> + warning_at (DECL_SOURCE_LOCATION (decl),
> + OPT_Wunused_variable,
> + "unused structured binding declaration");
> + else
> + warning_at (DECL_SOURCE_LOCATION (decl),
> + OPT_Wunused_variable, "unused variable %qD", decl);
> + }
> else if (DECL_CONTEXT (decl) == current_function_decl
> // For -Wunused-but-set-variable leave references alone.
> && TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE
> && errorcount == unused_but_set_errorcount)
> {
> - warning_at (DECL_SOURCE_LOCATION (decl),
> - OPT_Wunused_but_set_variable,
> - "variable %qD set but not used", decl);
> + if (!DECL_NAME (decl) && DECL_DECOMPOSITION_P (decl))
> + warning_at (DECL_SOURCE_LOCATION (decl),
> + OPT_Wunused_but_set_variable, "structured "
> + "binding declaration set but not used");
> + else
> + warning_at (DECL_SOURCE_LOCATION (decl),
> + OPT_Wunused_but_set_variable,
> + "variable %qD set but not used", decl);
> unused_but_set_errorcount = errorcount;
> }
> }
> @@ -7361,8 +7373,9 @@ cp_finish_decomp (tree decl, tree first,
> }
> if (processing_template_decl)
> {
> - retrofit_lang_decl (first);
> + retrofit_lang_decl (first, 4);
> SET_DECL_DECOMPOSITION_P (first);
> + DECL_DECOMP_BASE (first) = decl;
> }
> first = DECL_CHAIN (first);
> }
> @@ -7375,8 +7388,9 @@ cp_finish_decomp (tree decl, tree first,
> for (unsigned int i = 0; i < count; i++, d = DECL_CHAIN (d))
> {
> v[count - i - 1] = d;
> - retrofit_lang_decl (d);
> + retrofit_lang_decl (d, 4);
> SET_DECL_DECOMPOSITION_P (d);
> + DECL_DECOMP_BASE (d) = decl;
> }
>
> tree type = TREE_TYPE (decl);
> @@ -7482,6 +7496,7 @@ cp_finish_decomp (tree decl, tree first,
> eltscnt = tree_to_uhwi (tsize);
> if (count != eltscnt)
> goto cnt_mismatch;
> + int save_read = DECL_READ_P (decl);
> for (unsigned i = 0; i < count; ++i)
> {
> location_t sloc = input_location;
> @@ -7514,6 +7529,10 @@ cp_finish_decomp (tree decl, tree first,
> cp_finish_decl (v[i], init, /*constexpr*/false,
> /*asm*/NULL_TREE, LOOKUP_NORMAL);
> }
> + /* Ignore reads from the underlying decl performed during initialization
> + of the individual variables. If those will be read, we'll mark
> + the underlying decl as read at that point. */
> + DECL_READ_P (decl) = save_read;
> }
> else if (TREE_CODE (type) == UNION_TYPE)
> {
> @@ -12295,9 +12314,10 @@ grokdeclarator (const cp_declarator *dec
> {
> gcc_assert (declarator && declarator->kind == cdk_decomp);
> DECL_SOURCE_LOCATION (decl) = declarator->id_loc;
> - retrofit_lang_decl (decl);
> + retrofit_lang_decl (decl, 4);
> DECL_ARTIFICIAL (decl) = 1;
> SET_DECL_DECOMPOSITION_P (decl);
> + DECL_DECOMP_BASE (decl) = NULL_TREE;
> }
> }
>
> --- gcc/cp/decl2.c.jj 2017-05-24 11:59:02.000000000 +0200
> +++ gcc/cp/decl2.c 2017-05-25 14:43:01.216917214 +0200
> @@ -5027,6 +5027,9 @@ mark_used (tree decl, tsubst_flags_t com
>
> /* Set TREE_USED for the benefit of -Wunused. */
> TREE_USED (decl) = 1;
> + /* And for structured bindings also the underlying decl. */
> + if (DECL_DECOMPOSITION_P (decl) && DECL_DECOMP_BASE (decl))
> + TREE_USED (DECL_DECOMP_BASE (decl)) = 1;
>
> if (TREE_CODE (decl) == TEMPLATE_DECL)
> return true;
> --- gcc/cp/pt.c.jj 2017-05-24 11:59:02.000000000 +0200
> +++ gcc/cp/pt.c 2017-05-25 13:11:03.326234338 +0200
> @@ -15705,6 +15705,7 @@ tsubst_decomp_names (tree decl, tree pat
> return error_mark_node;
> }
> (*cnt)++;
> + gcc_assert (DECL_DECOMP_BASE (decl2) == pattern_decl);
> gcc_assert (DECL_HAS_VALUE_EXPR_P (decl2));
> tree v = DECL_VALUE_EXPR (decl2);
> DECL_HAS_VALUE_EXPR_P (decl2) = 0;
> --- gcc/cp/expr.c.jj 2017-05-24 11:59:02.000000000 +0200
> +++ gcc/cp/expr.c 2017-05-25 13:07:37.304821510 +0200
> @@ -133,8 +133,8 @@ mark_exp_read (tree exp)
> switch (TREE_CODE (exp))
> {
> case VAR_DECL:
> - if (DECL_VALUE_EXPR (exp))
> - mark_exp_read (DECL_VALUE_EXPR (exp));
> + if (DECL_DECOMPOSITION_P (exp))
> + mark_exp_read (DECL_DECOMP_BASE (exp));
> gcc_fallthrough ();
> case PARM_DECL:
> DECL_READ_P (exp) = 1;
> --- gcc/testsuite/g++.dg/cpp1z/decomp29.C.jj 2017-05-24 11:59:01.000000000 +0200
> +++ gcc/testsuite/g++.dg/cpp1z/decomp29.C 2017-05-25 14:47:59.000000000 +0200
> @@ -5,6 +5,7 @@
> struct A { int i,j,k; };
>
> A f();
> +int p[3];
>
> int z;
>
> @@ -14,13 +15,42 @@ int main()
> auto [i,j,k] = f(); // { dg-warning "unused" }
> }
> {
> + [[maybe_unused]] auto [i,j,k] = f();
> + }
> + {
> auto [i,j,k] = f();
> z = i;
> }
> {
> + auto [i,j,k] = f(); // { dg-warning "unused" }
> + i = 5;
> + }
> + {
> auto [i,j] = std::tuple{1,2}; // { dg-warning "unused" }
> }
> - // No parallel second test, because in this case i and j are variables rather
> - // than mere bindings, so there isn't a link between them and using i will
> - // not prevent a warning about unused j.
> + {
> + [[maybe_unused]] auto [i,j] = std::tuple{1,2};
> + }
> + {
> + auto [i,j] = std::tuple{1,2};
> + z = i;
> + }
> + {
> + auto [i,j] = std::tuple{1,2};
> + i = 5;
> + }
> + {
> + auto [i,j,k] = p; // { dg-warning "unused" }
> + }
> + {
> + [[maybe_unused]] auto [i,j,k] = p;
> + }
> + {
> + auto [i,j,k] = p;
> + z = i;
> + }
> + {
> + auto [i,j,k] = p; // { dg-warning "unused" }
> + i = 5;
> + }
> }
>
>
> Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-25 19:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-24 2:23 C++ PATCH to -Wunused with C++17 structured bindings Jason Merrill
2017-05-24 6:52 ` Jakub Jelinek
2017-05-24 16:19 ` Jason Merrill
2017-05-25 14:23 ` [C++ PATCH] Fix " Jakub Jelinek
2017-05-25 19:08 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).