Patchwork Attribute for unused warning for variables of non-trivial types

login
register
mail settings
Submitter Lubos Lunak
Date Nov. 8, 2012, 7:43 p.m.
Message ID <201211082043.51048.l.lunak@suse.cz>
Download mbox | patch
Permalink /patch/197871/
State New
Headers show

Comments

Lubos Lunak - Nov. 8, 2012, 7:43 p.m.
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.
Florian Weimer - Nov. 9, 2012, 11:08 a.m.
On 11/08/2012 08:43 PM, Lubos Lunak wrote:

>   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).

I'm not sure if the default shouldn't be "warn".  RAII-only classes 
which are used purely for constructor/destructor side effects are pretty 
rare, AFAICT.

To make this useful with containers, we'd need further annotations to 
tell read and write accesses apart.  (A vector might be append-only, and 
the elements might be ignored.)
Gabriel Dos Reis - Nov. 9, 2012, 3:56 p.m.
On Fri, Nov 9, 2012 at 5:08 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 11/08/2012 08:43 PM, Lubos Lunak wrote:
>
>>   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).
>
>
> I'm not sure if the default shouldn't be "warn".  RAII-only classes which
> are used purely for constructor/destructor side effects are pretty rare,
> AFAICT.
>
> To make this useful with containers, we'd need further annotations to tell
> read and write accesses apart.  (A vector might be append-only, and the
> elements might be ignored.)
>
> --
> Florian Weimer / Red Hat Product Security Team

I am also concerned that an idiomatic standard C++ program would need
obscure annotations to silence GCC.

-- Gaby

Patch

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 destructors it may be
+difficult or impossible for the compiler to find out whether a variable
+of this type is truly unused if it is not referenced. This type attribute
+informs the compiler that variables of this type should be warned about
+if they appear to be unused, just like variables of basic types would be
+warned about.
+
+Types which would benefit from this type attribute are for example various
+container classes such as std::list or std::string.
+
 @end table
 
 See also @ref{Namespace Association}.
diff --git a/gcc/testsuite/g++.dg/warn/warn_unused.C b/gcc/testsuite/g++.dg/warn/warn_unused.C
new file mode 100644
index 0000000..af687fc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/warn_unused.C
@@ -0,0 +1,22 @@ 
+// { dg-do compile }
+// { dg-options -Wunused }
+
+struct __attribute__((warn_unused)) Test
+{
+    Test();
+    ~Test();
+    void use();
+};
+
+struct TestNormal
+{
+    TestNormal();
+};
+
+int main()
+{
+   Test unused;         // { dg-warning "unused variable" }
+   Test used;           // { dg-bogus "unused variable" }
+   TestNormal normal;   // { dg-bogus "unused variable" }
+   used.use();
+}