From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85923 invoked by alias); 26 Jun 2017 12:28:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 85895 invoked by uid 89); 26 Jun 2017 12:28:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=famous X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Jun 2017 12:28:19 +0000 Received: by simark.ca (Postfix, from userid 33) id 35E7B1E561; Mon, 26 Jun 2017 08:28:17 -0400 (EDT) To: Pedro Alves Subject: Re: [PATCH 2/3] vec: Silence -Wunused-function warnings on clang X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Mon, 26 Jun 2017 12:28:00 -0000 From: Simon Marchi Cc: Simon Marchi , gdb-patches@sourceware.org In-Reply-To: References: <1498412703-24303-1-git-send-email-simon.marchi@ericsson.com> <1498412703-24303-3-git-send-email-simon.marchi@ericsson.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00697.txt.bz2 On 2017-06-26 11:28, Pedro Alves wrote: > On 06/25/2017 06:45 PM, Simon Marchi wrote: >> clang has a too aggressive (or broken, depends on how you want to see >> it) -Wunused-function warning, > > There's no way to avoid the warning in this use case, so I can't see > how to call it anything but the latter. > >> which is triggered by the functions >> defined by DEF_VEC_* but not used in the current source file. >> Normally, >> it won't warn about unused static inline functions defined in header >> files, because it's expected that a source file won't use all >> functions >> defined in a header file it includes. However, the DEF_VEC_* macros >> define those functions in source files, which leads clang to think >> that >> we should remove those functions. It is therefore missing a check to >> see whether those functions are resulting from macro expansion. A bug >> already exists for that: >> >> https://bugs.llvm.org//show_bug.cgi?id=22712 >> >> It's quite easy to silence this warning in a localized way, that is in >> the DEF_VEC_* macros. >> >> gdb/ChangeLog: >> >> * common/diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION): New >> macro. >> * common/vec.h: Include diagnostics.h. >> (DEF_VEC_I, DEF_VEC_P, DEF_VEC_O): Ignore -Wunused-function >> warning. >> --- >> gdb/common/diagnostics.h | 3 +++ >> gdb/common/vec.h | 11 +++++++++++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/gdb/common/diagnostics.h b/gdb/common/diagnostics.h >> index 35bacf2..ee824a3 100644 >> --- a/gdb/common/diagnostics.h >> +++ b/gdb/common/diagnostics.h >> @@ -35,9 +35,12 @@ >> # define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE >> ("-Wself-move") >> # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER \ >> DIAGNOSTIC_IGNORE ("-Wdeprecated-register") >> +# define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \ >> + DIAGNOSTIC_IGNORE ("-Wunused-function") > > GCC also understands this warning. So I think we should define > the ignore macro for GCC too. > > Now that raises the question of whether you want to suppress the > warning in vec.h for GCC too. But that's actually the point > that made me come here to comment: > > Imagine that we'll want to suppress -Wunused-function with GCC > in some other source file, for some reason. At that point we'll > naturally want to adjust diagnostics.h to define > > # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \ > DIAGNOSTIC_IGNORE ("-Wunused-function") > > for GCC too instead of leaving DIAGNOSTIC_IGNORE_UNUSED_FUNCTION empty > for GCC. But, that will make the existing (at that time) users > of DIAGNOSTIC_IGNORE_UNUSED_FUNCTION start suppressing the warning > on GCC too, and we may or not want that. > > This, to me indicates that: > > #1 - The common/diagnostics.h macros define the non-empty "ignore" > macro on all compilers that understand it. Then vec.h does: > > DIAGNOSTIC_PUSH > #ifdef __clang__ > DIAGNOSTIC_IGNORE_UNUSED_FUNCTION > #endif > > #2 - We name the macro something else more targeted for this > specific use case, and define it to empty for GCC, and to > -Wunused-function on clang. > > #2.1 - If put on common/diagnostics.h, name it something > generic, like: > > /* Suppress -Wunused-function for functions defined in source files as > result of expanding macros (that define the functions) defined > in headers. */ > #ifdef __lang > # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION_HEADER_MACRO_EXPANSION \ > DIAGNOSTIC_IGNORE_UNUSED_FUNCTION > #else > # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION_HEADER_MACRO_EXPANSION > #endif > > #2.2 - Otherwise, define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION > for for GCC and clang, and put something like this on > top of vec.h: > > /* Comment describing issue and pointing to clang bug report. */ > #ifdef __clang__ > #define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION \ > DIAGNOSTIC_IGNORE_UNUSED_FUNCTION > #else > #define DIAGNOSTIC_IGNORE_UNUSED_VEC_FUNCTION > #endif > > And use that instead of DIAGNOSTIC_IGNORE_FUNCTION. > > I think I prefer #2.2, then #2.1, then #1. > > Thanks, > Pedro Alves I wanted to keep it simple and easy to understand, so I didn't want to add to many layers of definitions. I thought that even if we ignored -Wunused-function in the vector macro expansions when compiling with GCC, it wasn't a big deal. But 2.2 is fine with me, I'll try it. It's all temporary anyway (famous last words), since the days of vec.h are counted :).