Re: [PATCH] V2: Fix compile warning building testcase

[Sam Thursfield <sam dot thursfield at codethink dot co dot uk>]

Hi Ian

Thanks a lot for reviewing the patch. I've attached a new version with 
some comments below.

On 08/03/17 04:03, Ian Lance Taylor via gcc-patches wrote:
> Thanks.  The patch submission is fine but 1) you didn't see which
> version of GCC you are using; 2) I don't understand why it works.  If
> BACKTRACE_SUPPORTED is 0, then I would expect that you would see a
> warning for test1, test2, test3, and test4.  Your patch doesn't fix
> those, so it sounds like you are only seeing the warning for test5, so
> BACKTRACE_SUPPORTED must be 1..  test5 is not compiled if
> BACKTRACE_SUPPORTS_DATA is 0, so BACKTRACE_SUPPORTS_DATA must be 1.
> So both BACKTRACE_SUPPORTED and BACKTRACE_SUPPORTS_DATA must be 1, so
> I don't understand what your patch fixes.

I'm using GCC 6.3.0 here to compile.

I see your point about the fix not making sense. The values of the
defines are this:

    #define BACKTRACE_SUPPORTED 0
    #define BACKTRACE_SUPPORTS_DATA 1

I noticed that test1(), test2(), test3() and test4() all have the
"unused" attribute:

    static int test1 (void) __attribute__ ((noinline, unused));
    static inline int test2 (void) __attribute__ ((always_inline, unused));
    static int test3 (void) __attribute__ ((noinline, unused));
    static inline int test4 (void) __attribute__ ((always_inline, unused));

So that will be why they don't trigger -Wunused-function :-)

It seems to me now that it'd be neater to add that same attribute to 
test5(), instead of using an #ifdef guard. Attached is a new patch that 
does so.

Again, I've tested this using GCC 6.3.0 on powerpc-ibm-aix7.2.0.0.

Sam

--
Sam Thursfield, Codethink Ltd.
Office telephone: +44 161 236 5575

>From 87b52dee5ca3b8feca379877c873c22b673f1b7b Mon Sep 17 00:00:00 2001
From: Sam Thursfield <sam.thursfield@codethink.co.uk>
Date: Wed, 8 Mar 2017 06:48:30 -0600
Subject: [PATCH] libbacktrace: Fix compile warning building testcase

This warning occurred when the BACKTRACE_SUPPORTED flag is defined to 0.

    ../../../gcc/libbacktrace/btest.c:624:1: error: 'test5' defined but not used [-Werror=unused-function]
     test5 (void)
     ^~~~~
    cc1: all warnings being treated as errors

The other test functions have the 'unused' attribute, presumably to
avoid similar errors, so let's handle test5() the same way. instead
of using a #ifdef guard.

libbacktrace/:

2017-03-08  Sam Thursfield  <sam.thursfield@codethink.co.uk>

       * btest.c (test5): Replace #ifdef guard with 'unused' attribute
       to fix compile warning when BACKTRACE_SUPPORTED isn't defined.
---
 libbacktrace/btest.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/libbacktrace/btest.c b/libbacktrace/btest.c
index e28a3d8..92cb325 100644
--- a/libbacktrace/btest.c
+++ b/libbacktrace/btest.c
@@ -616,7 +616,7 @@ f33 (int f1line, int f2line)
   return failures;
 }
 
-#if BACKTRACE_SUPPORTS_DATA
+static int test5 (void) __attribute__ ((unused));
 
 int global = 1;
 
@@ -686,8 +686,6 @@ test5 (void)
   return failures;
 }
 
-#endif /* BACKTRACE_SUPPORTS_DATA  */
-
 static void
 error_callback_create (void *data ATTRIBUTE_UNUSED, const char *msg,
 		       int errnum)
-- 
2.2.2