From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44905 invoked by alias); 26 Jun 2017 09:28:13 -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 44875 invoked by uid 89); 26 Jun 2017 09:28:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Jun 2017 09:28:10 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 06DE880474; Mon, 26 Jun 2017 09:28:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 06DE880474 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 06DE880474 Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 23BEB7E0C1; Mon, 26 Jun 2017 09:28:07 +0000 (UTC) Subject: Re: [PATCH 2/3] vec: Silence -Wunused-function warnings on clang To: Simon Marchi , gdb-patches@sourceware.org References: <1498412703-24303-1-git-send-email-simon.marchi@ericsson.com> <1498412703-24303-3-git-send-email-simon.marchi@ericsson.com> From: Pedro Alves Message-ID: Date: Mon, 26 Jun 2017 09:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1498412703-24303-3-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg00693.txt.bz2 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