diff mbox

[C++] Fix -Wunused-function (PR debug/66869)

Message ID 20160128201506.GG3017@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Jan. 28, 2016, 8:15 p.m. UTC 
On Wed, Jan 27, 2016 at 12:28:58PM -0700, Jeff Law wrote:
> On 01/27/2016 11:51 AM, Jakub Jelinek wrote:
> >2016-01-25  Jakub Jelinek  <jakub@redhat.com>
> >
> >	PR debug/66869
> >	* c-decl.c (c_write_global_declarations_1): Warn with
> >	warn_unused_function if static prototype without definition
> >	is not C_DECL_USED.
> >
> >	* gcc.dg/pr66869.c: New test.

And here is corresponding C++ FE patch.  Bootstrapped/regtested on
x86_64-linux and i686-linux, ok for trunk?

2016-01-28  Jakub Jelinek  <jakub@redhat.com>

	PR debug/66869
	* decl.c (wrapup_globals_for_namespace): Warn about unused static
	function declarations.

	* g++.dg/warn/Wunused-function2.C: New test.



	Jakub

Comments

Jason Merrill Jan. 29, 2016, 2:51 a.m. UTC | #1 
On 01/28/2016 03:15 PM, Jakub Jelinek wrote:
> +	if (TREE_CODE (decl) == FUNCTION_DECL
> +	    && DECL_INITIAL (decl) == 0
> +	    && DECL_EXTERNAL (decl)
> +	    && !TREE_PUBLIC (decl)
> +	    && !DECL_ARTIFICIAL (decl)
> +	    && !TREE_NO_WARNING (decl))

Do we need to check both DECL_INITIAL and DECL_EXTERNAL?

Jason
Jakub Jelinek Jan. 29, 2016, 10:35 a.m. UTC | #2 
On Thu, Jan 28, 2016 at 09:51:34PM -0500, Jason Merrill wrote:
> On 01/28/2016 03:15 PM, Jakub Jelinek wrote:
> >+	if (TREE_CODE (decl) == FUNCTION_DECL
> >+	    && DECL_INITIAL (decl) == 0
> >+	    && DECL_EXTERNAL (decl)
> >+	    && !TREE_PUBLIC (decl)
> >+	    && !DECL_ARTIFICIAL (decl)
> >+	    && !TREE_NO_WARNING (decl))
> 
> Do we need to check both DECL_INITIAL and DECL_EXTERNAL?

Dunno, but that is what cgraphunit.c does, c-decl.c too,
what the old toplev.c (check_global_declaration_1) did (back to at least
r26593 from ~ 1999), so I think we want some consistency.
Either it is needed, or if it is not needed, then all the spots should
change, not just this one.
I can try to stick there an assert whether for FUNCTION_DECL
(DECL_INITIAL (decl) == 0) == DECL_EXTERNAL (decl).

	Jakub
Jason Merrill Jan. 29, 2016, 10:47 a.m. UTC | #3 
On 01/29/2016 11:35 AM, Jakub Jelinek wrote:
> On Thu, Jan 28, 2016 at 09:51:34PM -0500, Jason Merrill wrote:
>> On 01/28/2016 03:15 PM, Jakub Jelinek wrote:
>>> +	if (TREE_CODE (decl) == FUNCTION_DECL
>>> +	    && DECL_INITIAL (decl) == 0
>>> +	    && DECL_EXTERNAL (decl)
>>> +	    && !TREE_PUBLIC (decl)
>>> +	    && !DECL_ARTIFICIAL (decl)
>>> +	    && !TREE_NO_WARNING (decl))
>>
>> Do we need to check both DECL_INITIAL and DECL_EXTERNAL?
>
> Dunno, but that is what cgraphunit.c does, c-decl.c too,
> what the old toplev.c (check_global_declaration_1) did (back to at least
> r26593 from ~ 1999), so I think we want some consistency.

OK.

Jason
Jakub Jelinek Jan. 29, 2016, 12:30 p.m. UTC | #4 
On Fri, Jan 29, 2016 at 11:35:07AM +0100, Jakub Jelinek wrote:
> I can try to stick there an assert whether for FUNCTION_DECL
> (DECL_INITIAL (decl) == 0) == DECL_EXTERNAL (decl).

Tried that, but cancelled that quickly, I see lots of cases where
DECL_INITIAL is non-NULL, but DECL_EXTERNAL is set, and some
where DECL_INITIAL is NULL, and DECL_EXTERNAL is not set,
at least in the other two spots (check_global_declaration in cgraphunit.c
and c-decl.c).  Haven't waited long enough to find out if the C++ FE is some
exception.

	Jakub
Jason Merrill Feb. 2, 2016, 6:17 p.m. UTC | #5 
On 01/29/2016 01:30 PM, Jakub Jelinek wrote:
> On Fri, Jan 29, 2016 at 11:35:07AM +0100, Jakub Jelinek wrote:
>> I can try to stick there an assert whether for FUNCTION_DECL
>> (DECL_INITIAL (decl) == 0) == DECL_EXTERNAL (decl).
>
> Tried that, but cancelled that quickly, I see lots of cases where
> DECL_INITIAL is non-NULL, but DECL_EXTERNAL is set, and some
> where DECL_INITIAL is NULL, and DECL_EXTERNAL is not set,
> at least in the other two spots (check_global_declaration in cgraphunit.c
> and c-decl.c).  Haven't waited long enough to find out if the C++ FE is some
> exception.

My thought was that if DECL_INITIAL is non-null, the function is 
defined, so it seems odd to warn about a lack of definition.

Jason
diff mbox

Patch

--- gcc/cp/decl.c.jj	2016-01-25 09:31:01.000000000 +0100
+++ gcc/cp/decl.c	2016-01-28 13:14:10.783286136 +0100
@@ -879,6 +879,24 @@  wrapup_globals_for_namespace (tree name_
   tree *vec = statics->address ();
   int len = statics->length ();
 
+  if (warn_unused_function)
+    {
+      tree decl;
+      unsigned int i;
+      FOR_EACH_VEC_SAFE_ELT (statics, i, decl)
+	if (TREE_CODE (decl) == FUNCTION_DECL
+	    && DECL_INITIAL (decl) == 0
+	    && DECL_EXTERNAL (decl)
+	    && !TREE_PUBLIC (decl)
+	    && !DECL_ARTIFICIAL (decl)
+	    && !TREE_NO_WARNING (decl))
+	  {
+	    warning (OPT_Wunused_function,
+		     "%q+F declared %<static%> but never defined", decl);
+	    TREE_NO_WARNING (decl) = 1;
+	  }
+    }
+
   /* Write out any globals that need to be output.  */
   return wrapup_global_declarations (vec, len);
 }
--- gcc/testsuite/g++.dg/warn/Wunused-function2.C.jj	2016-01-28 13:40:10.201053364 +0100
+++ gcc/testsuite/g++.dg/warn/Wunused-function2.C	2016-01-28 13:41:43.006788487 +0100
@@ -0,0 +1,6 @@ 
+// PR debug/66869
+// { dg-do compile }
+// { dg-options "-Wunused-function" }
+
+static void test (void); // { dg-warning "'void test..' declared 'static' but never defined" }
+int i;