On Wed, Mar 8, 2017 at 4:55 AM, Sam Thursfield <sam.thursfi...@codethink.co.uk> wrote: > > 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.
Makes sense. Thanks. Committed to mainline. Ian
