From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id 746B63858D32 for ; Tue, 18 Oct 2022 16:15:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 746B63858D32 Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id D0BEE300089; Tue, 18 Oct 2022 16:14:57 +0000 (UTC) Message-ID: <3415b2ae-bcf4-b995-15a3-e8f173461c5a@irq.a4lg.com> Date: Wed, 19 Oct 2022 01:14:55 +0900 Mime-Version: 1.0 Subject: Re: [PATCH v2 3/4] gdb/unittests: PR28413, suppress warnings generated by Gnulib Content-Language: en-US To: Enze Li Cc: gdb-patches@sourceware.org References: <667cafec3594a5eb180aee68f8c3adf1531addd3.1663835104.git.research_trasio@irq.a4lg.com> From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, GIT_PATCH_0, KAM_ASCII_DIVIDERS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Oct 2022 16:15:02 -0000 On 2022/10/18 22:44, Enze Li wrote: > Hi Tsukasa, > > Thanks for doing this. I tested this patch on macOS and found one nit. > Please see below. > > On Thu, Sep 22 2022 at 08:25:46 AM +0000, Tsukasa OI wrote: > >> Gnulib generates a warning if the system version of certain functions >> are used (to redirect the developer to use Gnulib version). It caused a >> compiler error when... >> >> - Compiled with Clang >> - -Werror is specified (by default) >> - C++ standard used by Clang is before C++17 (by default as of 15.0.0) >> when this unit test is activated. >> >> This issue is raised as PR28413. >> >> However, previous proposal to fix this issue (a "fix" to Gnulib): >> >> was rejected because it ruins the intent of Gnulib warnings. >> >> So, we need a Binutils/GDB-side solution. >> >> This commit tries to deal with this issue on the GDB side. We have >> "include/diagnostics.h" to disable certain warnings only when necessary. >> >> This commit suppresses the Gnulib warnings by adding >> DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including "defs.h". >> >> gdb/ChangeLog: >> >> pr 28413 >> * unittests/string_view-selftests.c: Suppress Gnulib-generated >> warnings when "defs.h" is included. >> --- >> gdb/unittests/string_view-selftests.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c >> index 2d7261d18d3..c1f7799d94c 100644 >> --- a/gdb/unittests/string_view-selftests.c >> +++ b/gdb/unittests/string_view-selftests.c >> @@ -23,7 +23,12 @@ >> >> #define GNULIB_NAMESPACE gnulib >> >> +#include "diagnostics.h" >> + >> +DIAGNOSTIC_PUSH >> +DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS >> #include "defs.h" >> +DIAGNOSTIC_POP >> #include "gdbsupport/selftest.h" >> #include "gdbsupport/gdb_string_view.h" > > With your patch applied on macOS 15.10.7, it still says, > > ========================================================================================== > CXX unittests/string_view-selftests.o > In file included from unittests/string_view-selftests.c:39: > /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/fstream:579:17: error: The symbol > ::fdopen refers to the system function. Use gnulib::fdopen instead. > [-Werror,-Wuser-defined-warnings] > __file_ = fdopen(__fd, __mdstr); > ^ > ./../gnulib/import/stdio.h:826:1: note: from 'diagnose_if' attribute on 'fdopen': > _GL_CXXALIASWARN (fdopen); > ^~~~~~~~~~~~~~~~~~~~~~~~~ > ./../gnulib/import/stdio.h:461:4: note: expanded from macro '_GL_CXXALIASWARN' > _GL_CXXALIASWARN_1 (func, GNULIB_NAMESPACE) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./../gnulib/import/stdio.h:463:4: note: expanded from macro '_GL_CXXALIASWARN_1' > _GL_CXXALIASWARN_2 (func, namespace) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ./../gnulib/import/stdio.h:468:5: note: expanded from macro '_GL_CXXALIASWARN_2' > _GL_WARN_ON_USE (func, \ > ^~~~~~~~~~~~~~~~~~~~~~~~ > ./../gnulib/import/stdio.h:632:19: note: expanded from macro '_GL_WARN_ON_USE' > __attribute__ ((__diagnose_if__ (1, message, "warning"))) > ^ ~ > 1 error generated. > make[2]: *** [unittests/string_view-selftests.o] Error 1 > ========================================================================================== > > I think this can be solved by just adding > DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS before including > according to your method. > > WDYT? Thanks for testing this on real Mac (since only Mac machine I own is very old so that support is dropped). Thinking of possible differences in the standard C++ library (as you tested), it'd be better to surround entire #include block with DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS (rather than just "defs.h" and ). I will submit PATCH v3 consisting only with this patch (because 2 of 4 patches [1-2/4] are merged already and Simon Marchi made a change better than my PATCH v2 4/4). Regards, Tsukasa > ========================================================================================== > diff --git a/gdb/unittests/string_view-selftests.c b/gdb/unittests/string_view-selftests.c > index 2d7261d18d3..b779b9b765a 100644 > --- a/gdb/unittests/string_view-selftests.c > +++ b/gdb/unittests/string_view-selftests.c > @@ -23,7 +23,12 @@ > > #define GNULIB_NAMESPACE gnulib > > +#include "diagnostics.h" > + > +DIAGNOSTIC_PUSH > +DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS > #include "defs.h" > +DIAGNOSTIC_POP > #include "gdbsupport/selftest.h" > #include "gdbsupport/gdb_string_view.h" > > @@ -31,7 +36,10 @@ > included test files are wrapped in a namespace. */ > #include > #include > +DIAGNOSTIC_PUSH > +DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS > #include > +DIAGNOSTIC_POP > #include > > /* libstdc++'s testsuite uses VERIFY. */ > ========================================================================================== > > Other than that, looks good to me. > > Reviewed-by: Enze Li > > Thanks, > Enze >