[PATCH] Attribute for unused warning for variables of non-trivial types
Hello,
could somebody please review
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55203 (patch also attached)?
The patch implements an attribute for marking types for which gcc cannot on
its own issue warnings about unused variables (e.g. because the ctor is
external), but for which such a warning might be useful anyway (e.g.
std::string).
Thank you.
--
Lubos Lunak
l.lu...@suse.cz
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 34cfb98..d9e6725 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -375,6 +375,7 @@ static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
static tree ignore_attribute (tree *, tree, tree, int, bool *);
static tree handle_no_split_stack_attribute (tree *, tree, tree, int, bool *);
static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *);
+static tree handle_warn_unused_attribute (tree *, tree, tree, int, bool *);
static void check_function_nonnull (tree, int, tree *);
static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
@@ -738,6 +739,8 @@ const struct attribute_spec c_common_attribute_table[] =
The name contains space to prevent its usage in source code. */
{ "fn spec", 1, 1, false, true, true,
handle_fnspec_attribute, false },
+ { "warn_unused",0, 0, false, false, false,
+ handle_warn_unused_attribute, false },
{ NULL, 0, 0, false, false, false, NULL, false }
};
@@ -7908,6 +7911,27 @@ handle_fnspec_attribute (tree *node ATTRIBUTE_UNUSED, tree ARG_UNUSED (name),
return NULL_TREE;
}
+/* Handle a "warn_unused" attribute; arguments as in
+ struct attribute_spec.handler. */
+
+static tree
+handle_warn_unused_attribute (tree *node, tree name,
+ tree args ATTRIBUTE_UNUSED,
+ int flags ATTRIBUTE_UNUSED, bool *no_add_attrs)
+{
+ if (TYPE_P(*node))
+/* Do nothing else, just set the attribute. We'll get at
+ it later with lookup_attribute. */
+;
+ else
+{
+ warning (OPT_Wattributes, "%qE attribute ignored", name);
+ *no_add_attrs = true;
+}
+
+ return NULL_TREE;
+}
+
/* Handle a "returns_twice" attribute; arguments as in
struct attribute_spec.handler. */
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index e21049b..0f5fb63 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6894,7 +6894,12 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
/* FIXME handle trivial default constructor, too. */
if (!already_used)
-mark_used (fn);
+{
+ /* Constructors and destructor of warn_unused types do not make them used. */
+ if (( !DECL_CONSTRUCTOR_P(fn) && !DECL_DESTRUCTOR_P(fn))
+ || !lookup_attribute ("warn_unused", TYPE_ATTRIBUTES (TYPE_METHOD_BASETYPE (TREE_TYPE (fn)
+mark_used (fn);
+}
if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0)
{
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index d25aa80..e0548e5 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -632,7 +632,8 @@ poplevel (int keep, int reverse, int functionbody)
&& DECL_NAME (decl) && ! DECL_ARTIFICIAL (decl)
&& TREE_TYPE (decl) != error_mark_node
&& (!CLASS_TYPE_P (TREE_TYPE (decl))
- || !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl
+ || !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl))
+ || lookup_attribute ("warn_unused", TYPE_ATTRIBUTES (TREE_TYPE (decl)
{
if (! TREE_USED (decl))
warning (OPT_Wunused_variable, "unused variable %q+D", decl);
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 0446038..38b15a7 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1512,8 +1512,12 @@ build_aggr_init (tree exp, tree init, int flags, tsubst_flags_t complain)
}
if (TREE_CODE (exp) == VAR_DECL || TREE_CODE (exp) == PARM_DECL)
-/* Just know that we've seen something for this node. */
-TREE_USED (exp) = 1;
+{
+ /* Just know that we've seen something for this node.
+ Merely creating a warn_unused aggregate doesn't make it used though. */
+ if( !lookup_attribute ("warn_unused", TYPE_ATTRIBUTES (type)))
+TREE_USED (exp) = 1;
+}
is_global = begin_init_stmts (&stmt_expr, &compound_stmt);
destroy_temps = stmts_are_full_exprs_p ();
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 6bf929a..177b3ee 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -15631,6 +15631,19 @@ only be applied to classes declared within an @code{extern "Java"} block.
Calls to methods declared in this interface will be dispatched using GCJ's
interface table mechanism, instead of regular virtual table dispatch.
+@item warn_unused
+@cindex @code{warn_unused} attribute
+
+For C++ types with non-trivial constructors and/or destru
Re: Fwd: [PATCH] Attribute for unused warning for variables of non-trivial types
Sorry, this has disappeared off my radar for a while.
On Wednesday 21 of November 2012, Jason Merrill wrote:
> On 11/20/2012 10:39 AM, Lubos Lunak wrote:
> >if (TREE_CODE (exp) == VAR_DECL || TREE_CODE (exp) == PARM_DECL)
> > -/* Just know that we've seen something for this node. */
> > -TREE_USED (exp) = 1;
> > +{
> > + /* Just know that we've seen something for this node.
> > + Merely creating a warn_unused aggregate doesn't make it used
> > though. */ + if( !lookup_attribute ("warn_unused", TYPE_ATTRIBUTES
> > (type))) +TREE_USED (exp) = 1;
> > +}
>
> I don't think we need this either; without this hunk, we should warn
> with -Wunused-but-set-variable, which seems like the behavior we want,
> since a variable with a non-trivial default constructor is set.
I have fixed the other two pointed out places (updated version attached), but
if I do also this, then the warning doesn't work. This is because TREE_USED
is used for detection, and it triggers the first warning in the relevant part
in gcc/cp/decl.c . The but-set warning would require using DECL_READ_P being
unset instead, but that one is always set by that point (it's set at least
twice during building the ctor call).
Are you sure this should be covered by -Wunused-but-set-variable rather than
plain -Wunused-variable? While strictly technically speaking the variable is
set by the ctor, it's conceptually confusing (is "string s;" really set from
the developer's point of view?), and it's inconsistent with basic types:
$ echo "void f() { int a = 1; } " | g++ -x c++ -fsyntax-only -Wall -
: In function ‘void f()’:
:1:16: warning: unused variable ‘a’ [-Wunused-variable]
So while I could try to patch all the places that mark the variable
DECL_READ_P during ctor/dtor calls, the only difference I see is that it will
make the handling more complicated, so I myself do not find that worth it.
--
Lubos Lunak
l.lu...@suse.cz
From 704a0ae2906e090ad08834c78d096af0eff9d1f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?=
Date: Sun, 30 Jun 2013 13:18:28 +0200
Subject: [PATCH] implement warn_unused attribute (gcc#55203)
---
gcc/c-family/c-common.c | 24
gcc/cp/decl.c | 3 ++-
gcc/cp/init.c | 8 ++--
gcc/doc/extend.texi | 13 +
gcc/testsuite/g++.dg/warn/warn_unused.C | 22 ++
5 files changed, 67 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/warn_unused.C
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 8b780c2..e4c7f1b 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -368,6 +368,7 @@ static tree handle_optimize_attribute (tree *, tree, tree, int, bool *);
static tree ignore_attribute (tree *, tree, tree, int, bool *);
static tree handle_no_split_stack_attribute (tree *, tree, tree, int, bool *);
static tree handle_fnspec_attribute (tree *, tree, tree, int, bool *);
+static tree handle_warn_unused_attribute (tree *, tree, tree, int, bool *);
static void check_function_nonnull (tree, int, tree *);
static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
@@ -738,6 +739,8 @@ const struct attribute_spec c_common_attribute_table[] =
The name contains space to prevent its usage in source code. */
{ "fn spec", 1, 1, false, true, true,
handle_fnspec_attribute, false },
+ { "warn_unused",0, 0, false, false, false,
+ handle_warn_unused_attribute, false },
{ NULL, 0, 0, false, false, false, NULL, false }
};
@@ -7942,6 +7945,27 @@ handle_fnspec_attribute (tree *node ATTRIBUTE_UNUSED, tree ARG_UNUSED (name),
return NULL_TREE;
}
+/* Handle a "warn_unused" attribute; arguments as in
+ struct attribute_spec.handler. */
+
+static tree
+handle_warn_unused_attribute (tree *node, tree name,
+ tree args ATTRIBUTE_UNUSED,
+ int flags ATTRIBUTE_UNUSED, bool *no_add_attrs)
+{
+ if (TYPE_P (*node))
+/* Do nothing else, just set the attribute. We'll get at
+ it later with lookup_attribute. */
+;
+ else
+{
+ warning (OPT_Wattributes, "%qE attribute ignored", name);
+ *no_add_attrs = true;
+}
+
+ return NULL_TREE;
+}
+
/* Handle a "returns_twice" attribute; arguments as in
struct attribute_spec.handler. */
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index f562546..ae8ef27 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -630,7 +630,8 @@ poplevel (int keep, int reverse, int functionbody)
&& DECL_NAME (decl) && ! DECL_ARTIFICIAL (decl)
&& type != error_mark_node
&& (!CLASS_TYP
Re: Fwd: [PATCH] Attribute for unused warning for variables of non-trivial types
Sorry, I haven't found time for this until now. On Sunday 14 of July 2013, Jason Merrill wrote: > On 07/08/2013 10:32 AM, Jason Merrill wrote: > > Was it a deliberate decision to put this in the c-common attributes > > rather than the C++-specific ones? I'm not saying it's wrong, just > > interested in your thinking. > > I think it makes sense to leave it in c-common so that types shared > between C and C++ can use it without a warning. Given the types need to have a ctor/dtor in order for this warning to make sense, this should not be needed. I actually didn't put it in c-common attributes on purpuse, I just based it on another attribute without realizing there was place elsewhere for C++-specific ones. I can write a patch to move this if wanted. > I've fixed up the other issues and am applying this patch: Thanks a lot. As for the copyright assignment, AFAIK there is a generic SUSE one that should cover this as well. -- Lubos Lunak l.lu...@suse.cz
